On Thu, 24 Aug 2023 18:44:14 GMT, Mandy Chung <mch...@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 incrementally with one additional 
> commit since the last revision:
> 
>   review feedback and javadoc clean up

> StackWalkers would be configured along two different axes (Kind and Options). 

This is intentional.   `Kind` is not the best name but it describes what 
information a stack walker collects from each stack frame.  

> It changes the mental model, in that all StackWalkers would now be divided 
> into two Kinds. I feel like this bleeds the implementation into the API a bit.

Although this RFE is motivated by an optimized way to walk the stack, each 
method has a declaring class.

In the current implementation, it has an internal way to collect the live 
locals and operands on the stack (`LOCALS_AND_OPERANDS`).   That fits into the 
third type to be collected from each frame.   If that were supported, it would 
belong to the new enum type. 

> 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?

In fact it's not so much constrained by the default behavior.  I wasn't 
completely happy with adding `NO_METHOD_INFO` to the options but could live 
with it.   When I reconsidered, I like separating this from `Option` as it 
specifies what information to be collected from each frame whereas `Option` 
controls everything else such as what frames to be filtered for better 
categorization.

I updated the javadoc.  Maybe it helps?

https://cr.openjdk.org/~mchung/api/java.base/java/lang/StackWalker.html
https://cr.openjdk.org/~mchung/jdk22/specdiff/overview-summary.html

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

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

Reply via email to