-
Enhancement
-
Resolution: Done
-
Major
-
6.0.0.Final
For every registerIntConstraintMatch() call, the ScoreHolders create an UnMatchListener (anonymous) instance and a Runnable (anonymous) instance. Although 1 instance is needed to hold the state, the other is overhead, so remove it.
Old explanation:
Investigating a performance problem in my solver caused me to take a look at the internals of ScoreHolder and I think there are some improvements to be made there. Namely here:
https://github.com/droolsjbpm/optaplanner/blob/master/optaplanner-core/src/main/java/org/optaplanner/core/api/score/buildin/hardsoftbigdecimal/HardSoftBigDecimalScoreHolder.java#L61
The Runnable. New instance is created every time - in my case, that is thousands, if not millions of short-lived instances. Wouldn't it make sense to make the matching reuse one shared instance?
- You obviously don't worry about synchronization, since you're modifying a shared state (hardScore) without any write locks. (Ditto for softScore.)
- The Runnable itself doesn't store any state.
- And the fact that you're instantiating it all the time only to have it GC'd most likely just a few more iterations later seems to me a waste of memory and CPU cycles, be it for GC or for the actual constant instantiation.
It won't be as easy as reusing te runnable, though - the runnable carries the "weight" value with it, and that is score-specific. So, instead of a shared Runnable, we need to provide a shared callback that the underlying AbstractScoreHolder would then fill with the proper "weight" value.
I am attaching a JMH micro-benchmark, which demonstrates the solution. In this synthetic environment, it achieves 10 % speedups. In my use case, it provided ~ 5 % speedup. Anyway, pretty good, as it has literally no drawbacks I could see.