RFR: 8303150: DCmd framework unnecessarily creates a DCmd instance on registration

[Frederic Parain]

On Mon, 13 Mar 2023 01:19:50 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.

The concern during the initial implementation was that the value returned by num_arguments() and the real number of arguments could become out of sync, but your solution to check that they are consistent only on debug builds addresses this concern.
Thank you for fixing this!

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

Marked as reviewed by fparain (Committer).

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