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

[Mat Carter]

On Tue, 12 Nov 2024 21:12:21 GMT, Ashutosh Mehra <asmehra at openjdk.org> wrote:

>> Mat Carter has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 43 additional commits since the last revision:
>> 
>>  - Enhanced upcall system testing options (based on PR feedback)
>>  - Removing unused includes
>>  - Remove cds include, no longer required after refactor
>>  - merge issue
>>  - fix merge issues
>>  - Update other platforms
>>  - Merge branch 'premain' into macarte-endtraining
>>  - Merge pull request #2 from macarte/macarte-endtraining-refactor
>>    
>>    Macarte endtraining refactor
>>  - minor edits
>>  - small changes based on PR review
>>  - ... and 33 more: https://git.openjdk.org/leyden/compare/4af49dbe...9c3cdf78
>
> src/hotspot/share/runtime/runtimeUpcallNop.cpp line 45:
> 
>> 43: bool RuntimeUpcallNop::filter_method_callback(MethodDetails& methodDetails)
>> 44: {
>> 45:   return false;
> 
> Shouldn't this be returning true to enable it for all methods as per the documentation for AddRuntimeUpcallsNOP in globals.hpp?

Thanks for pointing that out; I've updated the AddRuntimeUpcallsNOP to a string that is of the format "=[onMethodEntry|onMethodExit]:[all|none]" allowing us to test (a) mulitple upcalls ,(b) test onMethodExit (that it currently asserts), (c) have the NOP upcall do nothing, but either install an upcall on no methods or all methods

> src/hotspot/share/runtime/runtimeUpcalls.hpp line 39:
> 
>> 37: };
>> 38: 
>> 39: typedef void (*RuntimeUpcall)(JavaThread* current);
> 
> Not needed for the AOTEndTrainingOnMethodEntry option, but I think passing the method as the argument for which upcall is being done could be beneficial.

I can see where that might be useful, but it's complicated as we have a single call back but the 'method' comes back from the interpreter as a ciMethod instance and from c1/c2 as a Method instance, so we'd need to either have a level of indirection so we could wrap the method (like I did with MethodDetails) or have multiple callbacks

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

PR Review Comment: https://git.openjdk.org/leyden/pull/21#discussion_r1841260379
PR Review Comment: https://git.openjdk.org/leyden/pull/21#discussion_r1841263686