Proposal to remove `limitOption` from OMR::Options

[KavinSatheeskumar]

An overview of the existing system

In OMROptions.cpp, there is an array OMR::Options::_jitOptions which holds a struct representing each valid OMR cli option. Each of these structs contains an OptionFunctionPtr which indicates the call back used to parse and process the given option.

TR::Options::limitOption is one of these functions (technically, it is only used in J9Options.cpp, but that’s just a technical detail.)
TR::Options::limitOption is essentially a wrapper around TR_Debug::limitOption(…, bool). The Boolean is used to select between one of two global instances of TR::CompilationFilters to get passed to TR_Debug::limitOption(…, TR::CompilationFilters*). TR_Debug::limitOption(…, TR::CompilationFilters*) essentially adds an extra filter to specified global object and mutates some of its flags.

TR::CompilationFilters is a large class containing a number of binary search trees stored in a hash table which can be queried by TR_Debug::methodSigCanBeFound(const char*methodSig,…). The only method which makes use of this system is TR_Debug::methodSigCanBeCompiledOrRelocated(…). In turn, this method is only called by two methods which are thin wrappers around it, and those two methods are only called a total of 3 times.

Issues with the existing system

• It is quite convoluted, many layers of abstraction that are hard to navigate through
• Over engineered
  • Since only a few methods make use of this system, there is no reason it has to be this complicated in comparison to the other option types
• Not accessible via the regular OMR::Options::getCmdLineOptions() API, (instead relying on the TR_Debug object).
• Potential performance overhead
  • Because of how much memory allocation, hashing and tree traversal is required, I strongly suspect that this has a minor impact on performance, especially when you consider that this happens during every compilation. Especially since a regex match must be perf

Proposed Fix

The main functionality required of limit options is to be able to specify multiple methods to be either included or excluded from either

• Compilation
• Relocation
• Potentially more

As a result, I propose we either

• Extend setRegex
• Create a new setMultiRegex

Which allows the user to pass in multiple regular expressions via the command line as follows and have the combined into one regex which matches any of the ones passed in rather than the second instance of the cli argument overwriting the first as is done currently.

Java -Xjit:myRegex={regex1},myRegex={regex2}
# internally, this sets myRegex equal to regex1|regex2

Then the few places in the code that make use of limit options can be re-written to match these multi regexes rather than querying this hash table of binary search trees.

[dsouzai]

fyi @jdmpapin @0xdaryl

[mpirvu]

While the implementation changes it may useful to retain the old names of the option, names that service people are familiar to (-Xjit:exclude={regex} and -Xjit:limit={regex})