Cannot Reproduce
Details
Details
Assignee
Carlos Ruiz
Carlos RuizReporter
Jeremy Krieg
Jeremy KriegOriginal estimate
Time tracking
No time logged1w remaining
Components
Affects versions
Priority
Created May 10, 2018 at 6:44 AM
Updated July 2, 2018 at 11:41 AM
Resolved May 15, 2018 at 10:00 AM
Having been working through the core code a bit recently, I've noticed that there is quite a bit of it that uses long chains of simple if-else statements. Here is an example from
org.compiere.util.DisplayType
:This would be much more efficient if implemented using a switch() statement, eg:
Not only is the latter a bit more readable and less verbose, it also yields a performance improvement because the selection is O( 1 ) instead of O( n ) (n being the number of conditions being tested). In a core class like DisplayType, which is called often, this could yield a performance boost.
The potential performance boost is even greater if one considers selections based on string values. From Java 7, strings can be used in a switch() statement (see Oracle specs) . The Java compiler can generally optimise these more efficiently using something like a hashtable implementation or a trie, which turns the O( mn ) (m = length of strings, n = number of conditions) into something closer to an O( m ) operation. If we need Java 6 compatibility (though surely it's time to move on?), we could achieve a similar result using a static HashMap<String,Runnable> (though it is still less readable):
A further advantage of the hashmap implementation is that you can use it with any type of object that correctly implements the hashCode() and equals() methods.
This kind of optimisation could potentially have a large payoff in code like
DisplayType
, which is called frequently.Making this change through the entire codebase would be a time consuming exercise, so I propose something like the following:
Try to look for this in any new code/change proposals for trunk and get the contributors to fix them before merging.
Identify any frequently-called core classes like
DisplayType
where this pattern appears, and prioritise optimising those cases first. Probably worthwhile ensuring that they have unit tests first so as not to inadvertantly break anything while making the change.Fix any remaining instances as the opportunity arises.
Thoughts?