RFR: 8319850: PrintInlining should print which methods are late inlines [v14]

[theoweidmannoracle]

On Fri, 22 Nov 2024 14:34:43 GMT, theoweidmannoracle <duke at openjdk.org> wrote:

>> In https://github.com/openjdk/jdk/pull/16595 @caojoshua previously suggested changes to indicate which calls were inlined late, when printing inlines. This PR re-introduces the changes from the previously closed PR and fixes a minor issue where asserts were triggered.
>> 
>> Concerns were raised by @rwestrel in the previous PR:
>> 
>>> When InlineTree::ok_to_inline() is called, some diagnostic message is recorded for the call site. Do I understand right that with this patch, if the call is inlined late, then that message is dropped and replaced by a new "late inline.." message? If that's the case, isn't it the case that sometimes the InlineTree::ok_to_inline() has some useful information that's lost when late inlining happens?
>> 
>> As already pointed out in the PR by @caojoshua, this does not matter for string/methodhandle/vector/boxing late inlines, as they are [only performed if ok_to_inline() returns true](https://github.com/openjdk/jdk/blob/master/src/hotspot/share/opto/doCall.cpp#L189). This is also the only call to ok_to_inline().  
>> 
>> The only other location, where late inline call generators are created, are calls to CallGenerator::for_late_inline_virtual(), which creates a LateInlineVirtualCallGenerator. LateInlineVirtualCallGenerator (introduced in https://github.com/openjdk/jdk/pull/1550) does not really perform inlining but rather performs strength reduction from virtual to static calls. As can be verified by running the according test `test/hotspot/jtreg/compiler/c2/irTests/TestPostParseCallDevirtualization.java`, this does not affect the printing for inlining:
>> 
>> 
>> 5022 1026       3       compiler.c2.irTests.TestPostParseCallDevirtualization::callHelper (7 bytes)   made not entrant
>>                               @ 1   compiler.c2.irTests.TestPostParseCallDevirtualization$I::method (0 bytes)   failed to inline: virtual call
>> 
>> 
>> Thus, as far as I can tell, the proposed changes by @caojoshua do not lose any useful information about why no inlining prior to the late inlining occurred.
>
> theoweidmannoracle has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Fix TestDuplicatedLateInliningOutput

> I pulled your latest changes, and I am seeing missing newlines in the output, just by running `java -XX:+PrintInlining`. With -XX:+PrintIntrinsics, there is no additional output, so I'm wondering how -XX:+PrintIntrinsics tests are passing. Maybe we are missing test coverage for that flag.



> I'm also seeing missing method names:
> 
> ```
>                               @ 10   java.lang.StringBuilder::                              @ 7   jdk.internal.classfile.impl.SplitConstantPool::utf8Entry (45 bytes)   failed to inline: callee is too large
> ```
> 
> and weird indentation:
> 
> ```
>                                 @ 1   java.lang.Object::<init> (1 bytes)   inline
>                                                             @ 1   sun.invoke.util.Wrapper::basicTypeChar (18 bytes)   inline
> ```

@dean-long The reason for this is that multiple compile threads are trying to print at the same time. The odd formatting goes away with `-Xbatch`, preventing concurrent compilation. I didn't remove any explicit locking or synchronizing mechanism during refactoring. I think there was never any explicit mechanism to make this work without -Xbatch but it rather worked because the entire printinlining for one method was first dumped into a stringStream, which was then dumped onto tty in one go. With my refactoring though, InlinePrinter::IPInlineSite::dump will directly print individual segments of the output to tty, opening the door widely for bad interleavings with multiple compile threads. 

Do you think I should introduce an explicit synchronization mechanism to ensure the formatting is still correct with multiple compile threads?

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

PR Comment: https://git.openjdk.org/jdk/pull/21899#issuecomment-2497189139