RFR: 8303150: DCmd framework unnecessarily creates a DCmd instance on registration [v2]

[Kevin Walls]

On Wed, 15 Mar 2023 02:36:03 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> When DCmd factories are registered, the factory is passed the number of arguments taken by the DCmd - using a template method `get_num_arguments`. For DCmds that don't extend DCmdWithParser there has to be a static `num_arguments()` method in that class. For DCmds that do extend DCmdWithParser the logic instantiates an instance of the DCmd, extracts its parser and calls its `num_arguments` method which dynamically counts the number of defined arguments.
>> 
>> Creating an instance of each DCmd and dynamically counting arguments is inefficient and unnecessary, the number of arguments is statically known and easily expressed (in fact many of the JFR DCmds already statically define this). So we add the static `num_arguments()` method to each class that needs it and return the statically counted number of arguments. To ensure the static number and actual number don't get out-of-sync, we keep the original dynamic logic for use in debug builds to assert that the static and dynamic counts are the same. The assert will trigger during a debug build if something does get out of sync, for example if a new DCmd (extending DCmdWithParser) were added but didn't define the static `num_arguments()` method.
>> 
>> A number of DCmd classes were unnecessarily defining their own dynamic version of `num_arguments` and these are now removed.
>> 
>> In the template method I use `ENABLE_IF(std::is_convertible<T, DCmd>::value)` to check we only call this on DCmd classes. This may be unnecessary but it seemed consistent with the other template methods. Note that `std::is_base_of` only works for immediate super types.
>> 
>> Testing: tiers 1-4
>> 
>> Performance: in theory we should see some improvement in startup; in practice it is barely noticeable.
>> 
>> Thanks.
>
> David Holmes has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Comment update - Kevin's feedback

Marked as reviewed by kevinw (Committer).

Took me a bit of time, as I was thinking if we compare a hard-coded value with a parsed argument count, how does that cope with optional arguments?  Maybe "get_parsed_num_arguments()" suggested to me it was a live command being parsed, but DCmdParser's _arguments_list is a list of known possible arguments. Should have read your "number of arguments taken by the DCmd" at the top more carefully.

-------------

PR: https://git.openjdk.org/jdk/pull/12994