Fix container inefficiency in HeuristicSearch.java

[cinsttool]

Hi,

We find that there exist container inefficiency in HeuristicSearch.java.

The method getChildStates is never invoked in current repo.
And the List object childStates has no use in all the subclasses in current repo.
Therefore, the objects in List object childStates is are never accessed.
We add an attribute useChildStates to control the allocation of childStates when needed.

We discovered the above container inefficiency using our tool cinst. The patch is submitted.
Could you please check and accept it? We have tested the patch on our PC. The patched program works well.

[cyrille-artho]

Hi,
Thank you for your patch. It is indeed true that the search heuristics are not heavily tested or used in this repo. However, it could be that extensions use it and rely on childStates being updated.
Updating the behavior would at least require additional tests to check that it works as intended, and updates in the documentation. Furthermore, we would have to see if there are popular extensions depending on the current functionality.

Can you please provide a link to your "cinst" tool? Is the overhead of storing the child states really such a problem that it is worth this change (which results in an incompability against the current API of JPF)?

[cinsttool]

Hi, Thank you for your patch. It is indeed true that the search heuristics are not heavily tested or used in this repo. However, it could be that extensions use it and rely on childStates being updated. Updating the behavior would at least require additional tests to check that it works as intended, and updates in the documentation. Furthermore, we would have to see if there are popular extensions depending on the current functionality.

Can you please provide a link to your "cinst" tool? Is the overhead of storing the child states really such a problem that it is worth this change (which results in an incompability against the current API of JPF)?

We haven't release cinst for now. We will provide the link once we release cinst.

We use DiningPhil example to show the optimization effects:

Benchmark 1: java -jar jpf-core/build/RunJPF.jar jpf-core/src/examples/DiningPhil.jpf 5
  Time (mean ± σ):      6.838 s ±  0.176 s    [User: 14.855 s, System: 0.890 s]
  Range (min … max):    6.629 s …  7.140 s    10 runs
 
Benchmark 2: java -jar jpf-opt/build/RunJPF.jar jpf-opt/src/examples/DiningPhil.jpf 5
  Time (mean ± σ):      6.790 s ±  0.166 s    [User: 14.981 s, System: 0.800 s]
  Range (min … max):    6.593 s …  7.015 s    10 runs
 
Summary
   java -jar jpf-opt/build/RunJPF.jar jpf-opt/src/examples/DiningPhil.jpf 5 ran
    1.01 ± 0.04 times faster than numactl -c 1 java -jar jpf-core/build/RunJPF.jar jpf-core/src/examples/DiningPhil.jpf 5

The optimization effects are not significant in this example.
Actually, avoiding the allocations of the childStates can reduce the memory consumption.
But, if the heap is large enough, the optimization may not bring so much improvement on runtime.

[cyrille-artho]

Thanks for the explanation. As far as I know, the built-in tests are barely using heuristics, so the 1 % performance increase might be due to chance, as the total test time is likely dominated by the 99 % of other tests.
I'll keep this PR open so others can comment on it.

[cinsttool]

Thanks for your comment.

We agree that the built-in tests are barely using heuristics.
That's why we use the DiningPhil example as the test case.

We use this repo for detecting true container inefficiencies to validate the capabilities of our tools.
Therefore, the API changes that the optimization will introduce are not considered.
If it will cause a large number of modifications to other projects, we also agree that there is no need to merge.