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

[Mandy Chung]

On Mon, 21 Aug 2023 21:10:36 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 `Option.NO_METHOD_INFO` new stack walking option.
>> If no method information is needed, this option can be used such that the
>> 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
>> 
>> Adding `NO_METHOD_TIME` option 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 implementation in traversing
>> `StackFrame`s.
>> 
>> For example: to find the first caller filtering a known list of implementation class,
>> existing code can just add `NO_METHOD_INFO` in the call to `StackWalker::getInstance`
>> to create a stack walker instance:
>> 
>> 
>>      StackWalker walker = StackWalker.getInstance(Option.RETAIN_CLASS_REFERENCE, NO_METHOD_INFO);
>>      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.
>> 
>> The alternative considered 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.
>> 
>> ### Implementation Details
>> 
>> If `NO_METHOD_NAME` is set, the implementation creates `ClassFrameInfo[]`
>> buffer that is filled by the VM during stack walking.   `ClassFrameInfo` holds the 
>> Class instance plus `flag...
>
> Mandy Chung has updated the pull request incrementally with one additional commit since the last revision:
> 
>   clean up

@vidmik gave the feedback about the name of the option `NO_METHOD_INFO`, generally preferred a positive word.   I do agree.   One option is to introduce  `enum Kind { CLASS_INFO, METHOD_INFO}` instead of `NO_METHOD_INFO` and new factory methods `getInstance(Kind kind, Option... option)`.  
`getInstance()` is equivalent to `getInstance(METHOD_INFO)`.   To collect class only, do

getInstance(CLASS_INFO);
getInstance(CLASS_INFO, RETAIN_CLASS_REFERENCE);


I think a new enum is a cleaner way to specify what information to collect.    Any feedback/thought before I update this?

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

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