On Tue, 29 Aug 2023 00:16:40 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 `Option::DROP_METHOD_INFO` enum that requests to >> drop the method information. If no method information is needed, a >> `StackWalker` with `DROP_METHOD_INFO` >> can be used instead and such stack walker will save the overhead of >> extracting the method information >> and the memory used for the stack walking. >> >> 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 create a stack walker instance with `DROP_METHOD_INFO` >> option: >> >> >> StackWalker walker = StackWalker.getInstance(Option.DROP_METHOD_INFO, >> Option.RETAIN_CLASS_REFERENCE); >> Optional<Class<?>> callerClass = walker.walk(s -> >> s.map(StackFrame::getDeclaringClass) >> .filter(Predicate.not(implClasses::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 have to >> modify calls to the `walk` method to `walkClass` and the function body. >> >> ### Implementation Details >> >> A `StackWalker` configured with `DROP_METHOD_INFO` ... > > Mandy Chung has updated the pull request incrementally with two additional > commits since the last revision: > > - Revised the API change. Add Option::DROP_METHOD_INFO > - Review feedback from Remi src/java.base/share/classes/java/lang/StackWalker.java line 53: > 51: * > 52: * <p> The {@linkplain Option <em>stack walking option</em>} specifies > 53: * what the information a stack walker collects from the stack frames. I think this needs to use "Stack walking options" or "A stack walking option". Alternatively, just copy the first sentence from the Option javadoc so it changes to "Stack walker options configure the stack frame information obtained by a StackWalker". src/java.base/share/classes/java/lang/StackWalker.java line 54: > 52: * <p> The {@linkplain Option <em>stack walking option</em>} specifies > 53: * what the information a stack walker collects from the stack frames. > 54: * By default, the class name and method information are available but What would you think about changing "available" to "collected"? src/java.base/share/classes/java/lang/StackWalker.java line 58: > 56: * The method information can be dropped via {@link > Option#DROP_METHOD_INFO > 57: * DROP_METHOD_INFO} option. The {@code Class} object can be retained for > 58: * access via {@link Option#RETAIN_CLASS_REFERENCE > RETAIN_CLASS_REFERENCE} option. Probably should be "via the" rather than "via" in all cases. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/15370#discussion_r1309066013 PR Review Comment: https://git.openjdk.org/jdk/pull/15370#discussion_r1309066916 PR Review Comment: https://git.openjdk.org/jdk/pull/15370#discussion_r1309067884
