On Tue, 11 Jul 2023 15:47:35 GMT, Volker Simonis <simo...@openjdk.org> wrote:

>> As the included jtreg test demonstrates, `StackWalker.getCallerClass()` can 
>> throw an `UnsupportedOperationException` if called reflectively. Currently 
>> this only happens if we invoke `StackWalker.getCallerClass()` recursively 
>> reflectively, but this issue will become more prominent once we fix 
>> [JDK-8285447](https://bugs.openjdk.org/browse/JDK-8285447). The gory details 
>> follow below:
>> 
>> The protocol between the Java API and the JVM for 
>> `StackWalker.getCallerClass()/walk()` is as follows:
>> - On the Java side, `StackWalker` calls into `StackStreamFactory` for the 
>> real work.
>> - For `StackWalker.getCallerClass()` `StackStreamFactory` basically creates 
>> a `Class[]` which will be passed down and filled in the JVM. For 
>> `StackWalker.walk()` it will normally be a `StackFrameInfo[]` (or a 
>> `LiveStackFrameInfo[]` if the internal `ExtendedOption.LOCALS_AND_OPERANDS` 
>> option was used).
>> - The default size of this arrays is currently 
>> `StackStreamFactory.SMALL_BATCH` which is 8 (but see 
>> [JDK-8285447](https://bugs.openjdk.org/browse/JDK-8285447)).
>> - `StackStreamFactory` than calls `AbstractStackWalker.callStackWalk()` 
>> which is a natively implemented in the VM by `JVM_CallStackWalk()`.
>> -  `JVM_CallStackWalk()` calls `StackWalk::walk()` which calls 
>> `StackWalk::fetchFirstBatch()` which calls `StackWalk::fill_in_frames()` 
>> which walks the stack and fills in the available class/stackframe slots in 
>> the passed in array until the array is full or there are no more stack 
>> frames,
>> - Once  `StackWalk::fill_in_frames()` returns, 
>> `StackWalk::fetchFirstBatch()` calls back to Java by invoking 
>> `AbstractStackWalker::doStackWalk()` to consume the result.
>> - `AbstractStackWalker::doStackWalk()` calls `consumeFrames()` (which is 
>> overridden depending on whether we initially called `getCallerClass()` or 
>> `walk()`) which consumes the frames until it either finishes (e.g. finds the 
>> caller class) or until there are no more frames.
>> - In the latter case `consumeFrames()` will call into the the VM again by 
>> calling `AbstractStackWalker.fetchStackFrames()` to fetch additional frames 
>> from the stack.
>> - `AbstractStackWalker.fetchStackFrames()` is implemented by 
>> `JVM_MoreStackWalk()` which calls `StackWalk::fetchNextBatch()` which calls 
>> `StackWalk::fill_in_frames()` (the same method that already fetched the 
>> initial batch of frames).
>> 
>> Following is a stacktrace of what I've explained so far:
>> 
>> Native frames: (J=compiled Java code, j=interpreted, Vv=VM code, C=native 
>> code)
>> V  [libjvm.so+0x1...
>
> Volker Simonis has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Fixed case when calling getCallerClass() from a @CallerSensitive method 
> reflectively

I wonder if we should move the check to throw UOE if called by caller-sensitive 
method in Java as a general guidance to implement the runtime in Java if 
desirable.   That means it requires the VM to fill not only the class in the 
buffer but also need a flag to indicate the method is caller-sensitive or not.  
 It's a tradeoff of the buffer size.    The common case for `getCallerClass` is 
being invoked statically and should find the class from the first batch.   Only 
if it's invoked reflectively and if filtered in the Java,  it'll fetch more 
batches and hence the performance would not be as fast as filtered in VM but I 
think that's okay since it's uncommon.

Would you have cycle to implement this alternative and determine any 
performance impact to common cases?  Then evaluate this further.

The benchmark is at 
`test/micro/org/openjdk/bench/java/lang/StackWalkBench.java`.

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

PR Comment: https://git.openjdk.org/jdk/pull/14773#issuecomment-1634783438

Reply via email to