RFR: 8268829: Provide an optimized way to walk the stack with Class object only [v3]

[Brent Christian]

On Tue, 22 Aug 2023 19:01:53 GMT, Mandy Chung <mchung at openjdk.org> wrote:

>> 8268829: Provide an optimized way to walk the stack with Class object only
>> 
>> `StackWalker::walk` creates one `StackFrame` per frame and the current implementation
>> allocates one `StackFrameInfo` and one `MemberName` objects per frame. Some frameworks
>> like logging may only interest in the Class object but not the method name nor the BCI,
>> for example, filters out its implementation classes to find the caller class.  It's
>> similar to `StackWalker::getCallerClass` but allows a predicate to filter out the element.
>> 
>> This PR proposes to add `StackWalker.Kind` enum to specify the information that a stack walker
>> collects.  If no method information is needed, a `StackWalker` of `CLASS_INFO` can be used
>> instead and such stack walker will save the overhead (1) to extract the method information
>> and (2) the memory used for the stack walking.   In addition, this can also fix
>> 
>> - [8311500](https://bugs.openjdk.org/browse/JDK-8311500): StackWalker.getCallerClass() throws UOE if invoked reflectively
>> 
>> New factory methods to take a parameter to specify the kind of stack walker to be created are defined.
>> This provides a simple way for existing code, for example logging frameworks, to take advantage of
>> this enhancement with the least change as it can keep the existing function for traversing
>> `StackFrame`s.
>> 
>> For example: to find the first caller filtering a known list of implementation class,
>> existing code can call `StackWalker::getInstance(CLASS_INFO, ...)` to create a stack walker instance:
>> 
>> 
>>      StackWalker walker = StackWalker.getInstance(Kind.CLASS_INFO, Option.RETAIN_CLASS_REFERENCE);
>>      Optional<Class<?>> callerClass = walker.walk(s ->
>>              s.map(StackFrame::getDeclaringClass)
>>               .filter(interestingClasses::contains)
>>               .findFirst());
>> 
>> 
>> If method information is accessed on the `StackFrame`s produced by this stack walker such as
>> `StackFrame::getMethodName`, then `UnsupportedOperationException` will be thrown.
>> 
>> #### Alternatives Considered
>> One alternative is to provide a new API:
>> `<T> T walkClass(Function<? super Stream<Class<?>, ? extends T> function)`
>> 
>> In this case, the caller would need to pass a function that takes a stream
>> of `Class` object instead of `StackFrame`.  Existing code would have to
>> modify calls to the `walk` method to `walkClass` and the function body.
>> 
>> Another alternative is to add a new `NO_METHOD_INFO` option.  Similar to the proposed API,
>>...
>
> Mandy Chung has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 16 commits:
> 
>  - Replace NO_METHOD_INFO option with StackWalker.Kind enums
>  - Merge branch 'master' of https://github.com/openjdk/jdk into stackwalker-class
>  - clean up
>  - fix trailing whitespace
>  - Merge branch 'master' of https://github.com/openjdk/jdk into stackwalker-class
>  - Update LocalLongHelper.java
>  - clean up
>  - Merge branch 'master' of https://github.com/openjdk/jdk into stackwalker-class
>  - Add StackWalker.Option.NO_METHOD_INFO
>  - Merge branch 'master' of https://github.com/openjdk/jdk into stackwalker-class
>  - ... and 6 more: https://git.openjdk.org/jdk/compare/ce1ded1a...434d90cb

Sorry for not getting a look at this earlier.

I don't think adding a new `Kind` enum is the best way to do this.

StackWalkers would be configured along two different axes (`Kind` _and_ `Options`). It changes the mental model, in that all StackWalkers would now be divided into two `Kind`s. I feel like this bleeds the implementation into the API a bit.

The existing `Option` enum already provides a way to configure which frames are walked, and what information to include in those frames.  I think adding a new `Option` value fits better.

It's true that compatibility dictates that the default behavior be to _include_ method info, so the new option must _omit_ method info. If the `NO_METHOD_INFO` is disliked, perhaps a better name can be found - `SKIP_METHOD_INFO` or `OMIT_METHOD_INFO`?

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

PR Comment: https://git.openjdk.org/jdk/pull/15370#issuecomment-1690187998