On Thu, 2 Mar 2023 19:56:08 GMT, Paul Sandoz <psan...@openjdk.org> wrote:
>> Adam Sotona has updated the pull request incrementally with one additional >> commit since the last revision: >> >> StackMapFrameInfo extracted to top level from StackMapTableAttribute > > src/java.base/share/classes/jdk/internal/classfile/components/snippet-files/PackageSnippets.java > line 149: > >> 147: var instrumentorCodeMap = instrumentor.methods().stream() >> 148: >> .filter(instrumentedMethodsFilter) >> 149: >> .collect(Collectors.toMap(mm -> mm.methodName().stringValue() + >> mm.methodType().stringValue(), mm -> mm.code().orElse(null))); > > Should we be filtering out abstract methods before the `collect` and/or > replace with: > > mm -> mm.code().orElseThrow() > > ? Abstract methods should not be selected for instrumentation, so `orElseThrow()` is a good idea. Fixed, thanks. > src/java.base/share/classes/jdk/internal/classfile/components/snippet-files/PackageSnippets.java > line 171: > >> 169: >> 170: //store stacked method >> parameters into locals >> 171: var storeStack = new >> LinkedList<StoreInstruction>(); > > FWIW you can use an ArrayDeque + push + forEach, in the spirit of > highlighting Deque over LinkedList for stack-based usage (since this is an > example with visibility). fixed, thanks. ------------- PR: https://git.openjdk.org/jdk/pull/10982
