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

[Mat Carter]

On Tue, 5 Nov 2024 19:33:36 GMT, Mat Carter <macarte at openjdk.org> wrote:

>> 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...
>
>> 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 par...

> @macarte The RuntimeUpcalls has support for method exit upcalls, but the changes in interpreter and compilers are missing. I guess this is intentional as there is no use-case for the method exit upcalls as of now.

That's correct, there's no use-case but I added the option so that a future implementation doesn't require a change to all existing upcalls.  I also added an assert that will trigger if an attempt to register an onMethodExit upcall is made.

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

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