On Fri, 25 Aug 2023 22:22:43 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.
>> 
>> #### Javadoc & specdiff
>> 
>> https://cr.openjdk.org/~mchung/api/java.base/java/lang/StackWalker.html
>> https://cr.openjdk.org/~mchung/jdk22/specdiff/overview-summary.html
>> 
>> #### 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 hav...
>
> Mandy Chung has updated the pull request incrementally with two additional 
> commits since the last revision:
> 
>  - fixup javadoc
>  - Review feedback: move JLIA to ClassFrameInfo

As far as I understand the point of this change is to provide a new API that 
will allow a caller to request class information only, thus improving 
performance when only class information is required. Whether we use a new enum 
or a new option constant, we will need changes to the API documentation. If we 
use a new option constant we won't need any new static factory methods though.
Whether filtering is performed in java or in native is orthogonal to that. The 
trend these days is to do as much as possible in java, and the patch goes in 
that direction. There is a small price to pay if the frame to be filtered is a 
continuation, as this will cause the member name to be resolved. Is that 
significant enough to push back towards doing the filtering in native: I don't 
know. It may be worth investigating.

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

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

Reply via email to