RFR: 8350983: JShell LocalExecutionControl only needs stopCheck() on backward branches [v2]

[Jan Lahoda]

On Mon, 3 Mar 2025 15:19:22 GMT, Archie Cobbs <acobbs at openjdk.org> wrote:

>> JShell's `LocalExecutionControl` ensures a running eval thread can be stopped by `JShell.stop()` by inserting an invocation of `stopCheck()` at every branch point in the bytecode of loaded classfiles.
>> 
>> However, this same guarantee can be provided by invoking `stopCheck()` only at backward branches and not at forward branches. By doing this, the performance and size impact on the executing code can be reduced.
>> 
>> Fortunately the new ClassFile API makes this kind of modification easy 👍
>
> Archie Cobbs has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Niceify using pattern matching switch with guard per review suggestion.

Some comments inlined.

src/jdk.jshell/share/classes/jdk/jshell/execution/LocalExecutionControl.java line 99:

> 97:                         ClassTransform.transformingMethodBodies(
> 98:                             CodeTransform.ofStateful(() -> {
> 99:                                 HashSet<Label> priorLabels = new HashSet<>();

Nit: could be `Set<Label> priorLabels = new HashSet<>();`.

src/jdk.jshell/share/classes/jdk/jshell/execution/LocalExecutionControl.java line 102:

> 100:                                 return (builder, element) -> {
> 101:                                     switch (element) {
> 102:                                     case LabelTarget target -> priorLabels.add(target.label());

I think we usually indentat for `case`/`default`, right? (The old way of putting `case`/`default` at the same indentation level as was the switch is not really used in new code, or is it?)

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

PR Review: https://git.openjdk.org/jdk/pull/23850#pullrequestreview-2783802231
PR Review Comment: https://git.openjdk.org/jdk/pull/23850#discussion_r2053930429
PR Review Comment: https://git.openjdk.org/jdk/pull/23850#discussion_r2053932201