RFR: 8335358: [premain] Explore alternative ways to trigger the end of training run [v9]

[John R Rose]

On Thu, 19 Sep 2024 02:24:42 GMT, Mat Carter <macarte at openjdk.org> wrote:

>> AOT training can be ended using either
>> 
>> - -XX: AOTEndTrainingOnMethodEntry =Hello.someMethod [same syntax as CompileOnly]
>> - -XX: AOTEndTrainingOnMethodEntry =Hello.someMethod,Hello.someOtherMethod,count=42
>> - jcmd <pid> AOT.end_training
>> 
>> supports arm64 and x64
>> 
>> note: the AOTEndTrainingOnMethodEntry is ignored when CDSPreImage is specified; this is needed as the phase2 forked java process is passed all phase1 flags along with the -XX:CDSPreImage, but we don't want to run the trigger code in this phase (there may be a better way to handle this state or simply remove the flag from the forked process)
>> 
>> JBS Issue: https://bugs.openjdk.org/browse/JDK-8335358
>
> Mat Carter has updated the pull request incrementally with one additional commit since the last revision:
> 
>   jtreg tests for AOTEndTrainingOnMethodEntry

Suggestion:  Rename end_training_check in the interpreter to method_entry_upcall.  That way, the very wide but shallow changes to the assembly code will support a variety of features, not just this particular one.

Probably the same thing would work for the JITs as well.  So when C1 says


CDSConfig::is_dumping_preimage_static_archive_with_triggers() && callee->is_end_training_trigger()


Basically, the assembler and interpreter and JITs should not be thinking about CDS, just this newfangled "upcall".

it might as well say `callee->has_entry_upcall()` and the CDS-specific logic can be moved nearer to the method classes.  The name `is_end_training_trigger` can be retired or used in fewer places, when the only question at hand is whether to stick in an upcall.  Perhaps methodFlags continues to have `is_end_training_trigger` but `Method` exports `has_upcall` which delegates (internally) to `is_end_training_trigger`.

When you get to the C++ code for the upcall, just proceed as you do already, with a comment that the upcall has been installed if and only if the method was marked for training end.  Later on, that logic (again in C++) can be extended if we have other entry methods we wish to track.  Or not; maybe we'll never have a second use for this mechanism.  But even then, a more general looking name won't be harmful.  And there is some value in keeping the CDS-specific logic more localized, in a handful of functions that handle "upcalls" whatever those might be.

There are other lightweight versions of upcalls floating around too, low-level method entry notifications.  Some of them have been prototyped for Leyden (to trigger slow-burning recompilations from optimized code).  Factoring this feature into an upcall pattern might make it easier to merge the various kinds of upcalls into a common facility, eventually.

The name `_end_training_predicate` promises more than it is; shouldn't it be `_end_training_count_limit`?

I suggest passing the `CompileCommandEnum` into `parse_method_pattern`.  This will be a more user-facing instance of method patterns, so there will probably be special rules written for them.  Probably more restrictive than the present rules for method patterns in other compile commands.  For starters, we should choose between the dot-based and double-colon-based syntaxes.  I can't predict which way this conversation will go, but it will happen and we will be glad to be able to contextualize the parse to this particular command.

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

PR Comment: https://git.openjdk.org/leyden/pull/21#issuecomment-2360024827