On Fri, 3 Nov 2023 15:50:43 GMT, Rémi Forax <fo...@openjdk.org> wrote:
>> Jan Lahoda has updated the pull request incrementally with two additional >> commits since the last revision: >> >> - Some more get->orElseThrow >> - Reflecting review feedback. > > src/java.base/share/classes/java/lang/runtime/SwitchBootstraps.java line 429: > >> 427: Label next = element.next(); >> 428: cb.labelBinding(element.target()); >> 429: if (element.caseLabel() instanceof Class<?> >> classLabel && > > I think you can do a if else of isPresent inside instanceof Class<?> to avoid > to reapeat the instanceof and store `classLabel.describeConstable()` into a > local variable. Done, thanks. > src/java.base/share/classes/java/lang/runtime/SwitchBootstraps.java line 437: > >> 435: cb.aload(3); >> 436: >> cb.constantInstruction(extraClassLabels.size()); >> 437: cb.aaload(); > > Arrays are mutable in Java, so the VM can not know if the array of non > denotable classes (`extraClassLabels`) will be changed or not so the result > of aaload is not a constant so the call to isInstance can not be optimized. > Using a immutable list (`List.of()`) instead of an array should work, because > all the implementation of List.of() are using @Stable. In that case aaload > becomes invokevirtual List.get(). Done, thanks. > src/java.base/share/classes/java/lang/runtime/SwitchBootstraps.java line 517: > >> 515: >> BiPredicate.class, >> 516: >> Class[].class)); >> 517: return MethodHandles.insertArguments(typeSwitch, 2, new >> ResolvedEnumLabels(caller, enumDescs.toArray(s -> new EnumDesc<?>[s])), > > you can use the method reference `EnumDesc[]::new`instead of `s -> new > EnumDesc<?>[s]` and same below Class[]::new (the wirldcard should not be > necessary) Done, thanks. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/16489#discussion_r1381963834 PR Review Comment: https://git.openjdk.org/jdk/pull/16489#discussion_r1381963986 PR Review Comment: https://git.openjdk.org/jdk/pull/16489#discussion_r1381963719
