From 755b93e4f50b45e1b8bada3cd0f275914f70f383 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Wed, 9 Sep 2026 17:42:08 +0300 Subject: [PATCH 01/11] JavaScript port: resolve suspension by receiver type, not by bare signature The JavaScript backend compiles a method that can block into a JS generator and every call to it into ``yield*``. JavascriptSuspensionAnalysis decides which methods those are, and it decided by ``name + descriptor`` with no owner class -- so one blocking ``run()V`` anywhere in the program made every ``run()V`` call site in the program a suspension point, and every caller of those suspending in turn. Measured on hellocodenameone before this change: 21,467 live methods, 13,011 of them suspending, and 51% of those became suspending purely through that signature-wide propagation. The top entries were ``toString()`` (1,682 call sites), ``equals(Object)`` (1,192), ``getHeight()I`` (1,145), ``getWidth()I`` (1,126) and ``length()I`` (1,059) -- leaf methods whose typical receiver has a trivially synchronous implementation. JavascriptReachability already computed the receiver-type information needed to fix this and kept it private. It now publishes its instantiated set and a Model that resolves a call site against its subtype cone, and three separate over-approximations go away: * Suspension propagates through per-implementation edges resolved from the receiver cone instead of through the bare signature. Emitter and analysis share one DispatchModel, because a ``yield*`` emitted into a plain ``function`` is a JS SyntaxError rather than a subtle mistake. * A signature declared on a JSO bridge class is suspending only when the call site's cone can actually reach such a class. ``getWidth()I`` is declared on three JSO bridge interfaces, which was making every ``Component.getWidth()`` in the program a suspension point. * collectBridgeReferencedCn1Tokens scraped every ``"cn1_..."`` literal out of the bridge JS and seeded every matching method suspending. That is right for a name the bridge REPLACES (bindNative, or ``classDef.methods[id] = fn``) and wrong for one it merely LOOKS UP through resolveVirtual, whose result is driven by cn1_ivAdapt / adaptVirtualResult -- both of which tolerate a plain function. A token is now dropped only when EVERY occurrence of it is a resolveVirtual argument, so a name reached through a variable stays protected. That drops 10 of 705 tokens, including toString, equals, hashCode and run. Object.toString and Object.hashCode bind as plain ``function`` and Object.equals is not bound at all, so all three were suspending for no reason. Also fixed while in here: the cone resolver treated an instantiated receiver whose body the superclass walk could not find as contributing nothing, which under-approximates -- resolveVirtual also walks interfaces and then falls back to the global native table, and either can land on a generator the walk never saw. It now fails the whole query instead. And both published statics are reset per translation; Surefire reuses one JVM across fixtures, so a stale instantiated set would have resolved one app's call sites against another app's type graph. Measured, hellocodenameone: yield* sites 54,549 -> 40,741 (-25.3%), generators 13,068 -> 11,044 (-15.5%), suspending virtual dispatch 28,569 -> 19,073 (-33.2%), synchronous methods 8,456 -> 9,875 (+16.8%), bridge-referenced seeds 896 -> 236. The bundle barely moves (-1.2% raw, -0.25% gzip): generator machinery is nearly free in bytes and expensive only in time. That last point is why this change also brings a benchmark. Nothing in the tree could price a JavaScript backend change -- the bundle does not move, the screenshot suite is pass/fail and the lifecycle harness reports milestones rather than time, so a real reduction in generator density and a no-op looked identical. scripts/run-javascript-throughput-benchmark.sh translates vm/benchmarks/javascript/JsThroughputBench.java and runs it under Node in about 55 seconds. Interleaved against master, best of three: hashCodeHeavy -57.6%, toStringHeavy -18.9%, equalsHeavy -7.8%, mapChurn -6.6%, against a master-vs-master noise floor of 0.3-4.9% and two controls that the dispatch work cannot touch (-1.8% and 0.0%). Four properties of that benchmark were each paid for with a wrong answer, and the comments say so: every workload reports a checksum and the comparison refuses a workload whose checksum moved; there are two controls; each workload runs in its own process, because sharing one made iteratorWalk measure 8.5ms run first and 15ms run eighth, so speeding up an earlier workload reported a regression in byte-identical code; and the harness counts the times cn1_ivsDrive silently steps a generator that reached the sync dispatcher, which would otherwise show up as a speedup rather than as the bug it is. That counter reads 0 on every arm measured here. One result is unexplained and is not claimed as noise: iteratorWalk regressed 13.7%, twelve times its own noise floor, reproducibly. Its emitted body is byte-identical between arms, all nineteen iterator functions are byte-identical, an exhaustive function-level diff finds 42 differing functions and none in its execution path, both bundles define the same function set, and V8 deopt counts match. It survives process isolation and hoisting the allocation out of the timed region. vm/tests: 305 tests, 0 failures, 1 pre-existing skip. SpotBugs clean. The 181-screenshot suite has NOT been run locally -- Cn1ssScreenshotServer binds a hardcoded port 8765 and a concurrent checkout holds it -- which is the main thing CI needs to answer here. Co-Authored-By: Claude Opus 5 (1M context) --- scripts/lib/js_throughput_report.py | 138 ++++++ .../run-javascript-throughput-benchmark.sh | 230 ++++++++++ .../translator/JavascriptBundleWriter.java | 43 +- .../translator/JavascriptMethodGenerator.java | 23 +- .../translator/JavascriptReachability.java | 217 ++++++++++ .../JavascriptSuspensionAnalysis.java | 407 ++++++++++++++++-- .../codename1/tools/translator/Parser.java | 2 +- .../javascript/JsThroughputBench.java | 348 +++++++++++++++ 8 files changed, 1376 insertions(+), 32 deletions(-) create mode 100755 scripts/lib/js_throughput_report.py create mode 100755 scripts/run-javascript-throughput-benchmark.sh create mode 100644 vm/benchmarks/javascript/JsThroughputBench.java diff --git a/scripts/lib/js_throughput_report.py b/scripts/lib/js_throughput_report.py new file mode 100755 index 00000000000..014f5933ec1 --- /dev/null +++ b/scripts/lib/js_throughput_report.py @@ -0,0 +1,138 @@ +#!/usr/bin/env python3 +"""Formats and compares JavaScript-target throughput benchmark results. + +Split out of run-javascript-throughput-benchmark.sh so the comparison rule +lives somewhere it can be read: a workload whose CHECKSUM moved is not +comparable on time, and saying so is the whole point. "Fewer generators" and +"fewer calls" are easy to confuse, so a run that got faster by doing less work +is the failure mode this benchmark is most exposed to. +""" +import json +import os +import re +import sys + +BENCH_RE = re.compile(r"^BENCH id=(\S+) ns=(\d+) checksum=(-?\d+)$") +SUITE_RE = re.compile(r"^BENCHSUITE checksum=(-?\d+)$") +PROBE_RE = re.compile(r"^BENCHPROBE syncDroveGenerator=(\d+)$") +# A workload the dispatch work cannot affect. If these move, the run measured +# the machine and every other delta in it is suspect. +CONTROLS = ("arithControl", "suspendControl") + + +def parse_raw(path): + workloads, suite, drove = {}, None, 0 + with open(path, encoding="utf-8", errors="replace") as handle: + for line in handle: + line = line.strip() + match = BENCH_RE.match(line) + if match: + workloads[match.group(1)] = { + "ns": int(match.group(2)), + "checksum": int(match.group(3)), + } + continue + match = SUITE_RE.match(line) + if match: + suite = int(match.group(1)) + continue + match = PROBE_RE.match(line) + if match: + drove = int(match.group(1)) + return workloads, suite, drove + + +def parse_suspension(path): + if not path or not os.path.isfile(path): + return {} + out = {} + with open(path, encoding="utf-8", errors="replace") as handle: + for line in handle: + parts = line.split() + if len(parts) == 2 and parts[0] in ("TOTAL", "SYNC", "SUSPENDING", "SUSPENDING_SIGS"): + out[parts[0].lower()] = int(parts[1]) + elif line.startswith("#") or line.startswith("M "): + if line.startswith("M "): + break + return out + + +def main(): + workloads, suite, drove = parse_raw(os.environ["RAW"]) + if not workloads or suite is None: + print("no benchmark results parsed", file=sys.stderr) + return 1 + + result = { + "workloads": workloads, + "suite_checksum": suite, + "bundle": { + "translated_bytes": int(os.environ.get("BUNDLE_BYTES", 0)), + "yield_sites": int(os.environ.get("YIELDS", 0)), + "generators": int(os.environ.get("GENERATORS", 0)), + }, + "suspension": parse_suspension(os.environ.get("REPORT")), + # Non-zero means the sync dispatcher met a generator: the + # classification was wrong and the runtime absorbed it. + "sync_drove_generator": drove, + } + + baseline_path = os.environ.get("BASELINE") or "" + baseline = None + if baseline_path: + with open(baseline_path, encoding="utf-8") as handle: + baseline = json.load(handle) + + width = max(len(name) for name in workloads) + if baseline is None: + print("%-*s %12s" % (width, "workload", "ms")) + for name in sorted(workloads): + print("%-*s %12.3f" % (width, name, workloads[name]["ns"] / 1e6)) + else: + print("%-*s %12s %12s %9s" % (width, "workload", "base ms", "new ms", "change")) + mismatched = [] + for name in sorted(workloads): + new = workloads[name] + old = baseline.get("workloads", {}).get(name) + if old is None: + print("%-*s %12s %12.3f %9s" % (width, name, "-", new["ns"] / 1e6, "new")) + continue + if old["checksum"] != new["checksum"]: + mismatched.append(name) + print("%-*s %12.3f %12.3f %9s" % ( + width, name, old["ns"] / 1e6, new["ns"] / 1e6, "CHECKSUM")) + continue + # Negative percent means faster. + change = (new["ns"] - old["ns"]) / float(old["ns"]) * 100.0 + flag = " <-control" if name in CONTROLS else "" + print("%-*s %12.3f %12.3f %8.1f%%%s" % ( + width, name, old["ns"] / 1e6, new["ns"] / 1e6, change, flag)) + for key, label in (("translated_bytes", "translated bytes"), + ("yield_sites", "yield* sites"), + ("generators", "generators")): + old = baseline.get("bundle", {}).get(key) + new = result["bundle"][key] + if old: + print("%-*s %12d %12d %8.1f%%" % ( + width, label, old, new, (new - old) / float(old) * 100.0)) + if mismatched: + print("\nREFUSING the comparison: checksum changed for %s." + "\nA workload that computes something different cannot be compared on time." + % ", ".join(mismatched), file=sys.stderr) + return 1 + + if drove: + print("\nWARNING: the sync virtual dispatcher met a generator %d time(s)." + "\nThe analysis classified a signature synchronous that is not; the runtime" + "\nabsorbed it by stepping the generator once. Fix the classification --" + "\ndo not read the timings as a clean result." % drove, file=sys.stderr) + + if os.environ.get("JSON_OUT"): + with open(os.environ["JSON_OUT"], "w", encoding="utf-8") as handle: + json.dump(result, handle, indent=2, sort_keys=True) + handle.write("\n") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/scripts/run-javascript-throughput-benchmark.sh b/scripts/run-javascript-throughput-benchmark.sh new file mode 100755 index 00000000000..37a60d002b7 --- /dev/null +++ b/scripts/run-javascript-throughput-benchmark.sh @@ -0,0 +1,230 @@ +#!/usr/bin/env bash +# +# Dispatch-shaped throughput benchmark for the JavaScript target. +# +# Translates vm/benchmarks/javascript/JsThroughputBench.java to JavaScript and +# runs it under Node (same V8 the browser uses), reporting per-workload times +# and a checksum per workload. +# +# It exists because nothing else in the tree can price a JS backend change. +# The bundle barely moves when generator density does (``yield* `` is seven +# characters), the screenshot suite is pass/fail, and the lifecycle harness +# reports milestones rather than time -- so a real improvement and a no-op +# were indistinguishable. +# +# Usage: +# scripts/run-javascript-throughput-benchmark.sh [--json OUT] [--baseline IN] +# +# --json OUT write results as JSON +# --baseline IN compare against an earlier --json file and print deltas. +# REFUSES the comparison if any checksum differs, because a +# workload that changed what it computes cannot be compared +# on time. +# +# Exit codes: 0 clean, 1 benchmark/comparison failure, 2 misconfiguration. +set -euo pipefail + +bench_log() { echo "[js-throughput] $1" >&2; } + +SCRIPT_DIR="$(CDPATH= cd -- "$(dirname -- "$0")" && pwd)" +REPO_ROOT="$(CDPATH= cd -- "$SCRIPT_DIR/.." && pwd)" + +JSON_OUT="" +BASELINE="" +while [ $# -gt 0 ]; do + case "$1" in + --json) JSON_OUT="$2"; shift 2 ;; + --baseline) BASELINE="$2"; shift 2 ;; + -h|--help) sed -n '2,24p' "$0" >&2; exit 0 ;; + *) bench_log "unknown argument: $1"; exit 2 ;; + esac +done + +# CN1_JS_BENCH_SRC lets a probe run through the identical pipeline, which is +# how you tell "the benchmark is wrong" from "the harness is wrong". +BENCH_SRC="${CN1_JS_BENCH_SRC:-$REPO_ROOT/vm/benchmarks/javascript/JsThroughputBench.java}" +APP_NAME="$(basename "$BENCH_SRC" .java)" +# Overridable so two translator builds can be A/B'd by INTERLEAVING runs +# against saved jars, rather than by rebuilding between arms and comparing +# numbers taken minutes apart on a host whose mood has changed. +COMPILER_JAR="${CN1_JS_BENCH_COMPILER_JAR:-$REPO_ROOT/maven/parparvm/target/bundle/parparvm-compiler.jar}" +JAVA_API_JAR="$REPO_ROOT/maven/parparvm/target/bundle/parparvm-java-api.jar" + +for required in "$BENCH_SRC" "$COMPILER_JAR" "$JAVA_API_JAR"; do + if [ ! -e "$required" ]; then + bench_log "missing $required" + bench_log "build it with: mvn -f maven/pom.xml -pl parparvm -am -DskipTests package" + exit 2 + fi +done + +JAVA_BIN="${JAVA_HOME:+$JAVA_HOME/bin/java}"; [ -x "${JAVA_BIN:-}" ] || JAVA_BIN="$(command -v java)" +JAVAC_BIN="${JAVA_HOME:+$JAVA_HOME/bin/javac}"; [ -x "${JAVAC_BIN:-}" ] || JAVAC_BIN="$(command -v javac)" +NODE_BIN="$(command -v node || true)" +if [ -z "$NODE_BIN" ]; then bench_log "node is required"; exit 2; fi + +WORK_DIR="$(mktemp -d "${TMPDIR:-/tmp}/cn1-js-throughput-XXXXXX")" +[ -n "${KEEP_BENCH_DIR:-}" ] && bench_log "keeping $WORK_DIR" || trap "rm -rf $WORK_DIR" EXIT + +STAGE="$WORK_DIR/stage" +OUT="$WORK_DIR/out" +mkdir -p "$STAGE" "$OUT" + +# The translator consumes ONE class tree holding both the JavaAPI and the +# application, exactly as the hellocodenameone build stages it. +( cd "$STAGE" && "${JAVA_HOME:-/usr}/bin/jar" xf "$JAVA_API_JAR" 2>/dev/null || unzip -qo "$JAVA_API_JAR" -d "$STAGE" ) +rm -rf "$STAGE/META-INF" + +bench_log "compiling $APP_NAME against the ParparVM JavaAPI" +"$JAVAC_BIN" -source 8 -target 8 -Xlint:-options -nowarn \ + -bootclasspath "$STAGE" -d "$STAGE" "$BENCH_SRC" >&2 + +bench_log "translating to JavaScript" +# Identifier minification off, as JavascriptTargetIntegrationTest does it. +# The whole-bundle renamer rewrites the ``cn1_*`` names that bindNative +# registers its natives under, and without port.js's fallback dance to repair +# the lookup, System.out.println resolves to nothing -- the benchmark then +# runs to completion and prints NOTHING, which reads as a harness bug rather +# than a broken lookup. It is also irrelevant to what is being measured here: +# this benchmark prices call dispatch, not identifier length. +"$JAVA_BIN" -cp "$COMPILER_JAR" \ + -Dparparvm.js.minify.idents.off=1 \ + -Dparparvm.js.alias.off=1 \ + -Dcodename1.javascriptport.webapp="$REPO_ROOT/Ports/JavaScriptPort/src/main/webapp" \ + com.codename1.tools.translator.ByteCodeTranslator \ + javascript "$STAGE" "$OUT" "$APP_NAME" "com.codenameone.bench" "$APP_NAME" "1.0" "ios" "none" >&2 + +DIST="$OUT/dist/$APP_NAME-js" +[ -d "$DIST" ] || DIST="$(dirname "$(find "$OUT/dist" -name worker.js -print -quit)")" +if [ ! -d "$DIST" ]; then bench_log "translated bundle not found under $OUT/dist"; exit 1; fi + +# Real timers and a real clock. The vm/tests harness deliberately stubs +# ``Date.now`` and drives a virtual clock so thread tests are deterministic -- +# which is exactly wrong here, where the measurement IS wall time. +cat > "$WORK_DIR/harness.js" <<'HARNESS' +const fs = require('fs'); +const path = require('path'); +const vm = require('vm'); +const distDir = process.argv[2]; +const timers = []; +let timerId = 1; +global.self = global; +global.window = global; +global.global = global; +// The VM reports a dead green thread by posting an error rather than by +// throwing, so a no-op postMessage turns a crashed benchmark into a silent +// empty result. Surface it and make the run fail. +let vmError = null; +global.postMessage = function(msg) { + if (msg && msg.type === 'error') { + vmError = msg; + process.stderr.write('VM ERROR: ' + JSON.stringify(msg) + '\n'); + } +}; +global.setTimeout = function(fn, millis) { + const t = { id: timerId++, due: Date.now() + Math.max(0, millis | 0), fn: fn, cleared: false }; + timers.push(t); + return t; +}; +global.clearTimeout = function(t) { if (t) { t.cleared = true; } }; +global.setInterval = function() { return { cleared: true }; }; +global.clearInterval = function() {}; +global.importScripts = function() { + for (const script of arguments) { + const p = path.join(distDir, String(script)); + vm.runInThisContext(fs.readFileSync(p, 'utf8'), { filename: p }); + } +}; +importScripts('parparvm_runtime.js'); +const chunks = fs.readdirSync(distDir) + .filter((f) => /^translated_app_\d+\.js$/.test(f)) + .sort((a, b) => parseInt(a.match(/\d+/)[0], 10) - parseInt(b.match(/\d+/)[0], 10)); +for (const c of chunks) { importScripts(c); } +importScripts('translated_app.js'); +// Soundness probe. ``cn1_ivs*`` is the SYNC virtual dispatcher: the analysis +// selected it because it proved no implementation of that signature suspends. +// If the function it resolves turns out to be a generator anyway, cn1_ivsDrive +// silently steps it once and carries on -- correct for a body that does not +// actually yield, and a defect the benchmark would otherwise report as a +// speedup. Count those, and print the count next to the timings so a +// classification that is wrong-but-survivable cannot masquerade as one that +// is right. +if (typeof global.cn1_ivsDrive === 'function') { + const inner = global.cn1_ivsDrive; + global.__cn1SyncDroveGenerator = 0; + global.cn1_ivsDrive = function(r, mid) { + if (r && typeof r.next === 'function') { global.__cn1SyncDroveGenerator++; } + return inner(r, mid); + }; +} +// worker.js does exactly this after its imports, and it is not optional: a +// bindNative that ran while jvm.classes was still empty could not compute the +// class-free dispatch id its callers use, so the native never reaches the +// method table. Skipping it leaves System.out.println resolving to the +// translated Java body, which prints nothing and reports no error -- the +// benchmark then runs to completion and produces no output at all. +if (typeof global.__parparInstallNativeBindings === 'function') { + global.__parparInstallNativeBindings(); +} +// Select a single workload, so each one is measured in a process that has +// run nothing else. See the comment on JsThroughputBench.only. +const only = process.argv[3] || ''; +if (only) { + const cls = jvm.classes[process.argv[4]]; + if (!cls || !cls.staticFields) { throw new Error('cannot reach bench class to set filter'); } + cls.staticFields['only'] = jvm.createStringLiteral(only); +} +// Go through the runtime's own entry point rather than hand-spawning main: +// start() also installs the native overrides and the System print streams, +// and hand-rolling that is how a harness ends up measuring a VM the browser +// never runs. +jvm.start(); +while (jvm.runnable.length || timers.length) { + if (jvm.runnable.length) { jvm.drain(); continue; } + timers.sort((a, b) => a.due - b.due || a.id - b.id); + const t = timers.shift(); + if (!t || t.cleared) { continue; } + t.fn(); +} +console.log('BENCHPROBE syncDroveGenerator=' + (global.__cn1SyncDroveGenerator | 0)); +if (vmError) { + process.exitCode = 1; +} +HARNESS + +bench_log "running under node $("$NODE_BIN" -v)" +RAW="$WORK_DIR/raw.txt" +: > "$RAW" + +# One process per workload. Sharing a process makes every measurement depend +# on what ran before it, which reports phantom regressions in unchanged code. +WORKLOADS="$(grep -oE 'run\("[A-Za-z]+"' "$BENCH_SRC" | sed 's/run("//;s/"//' | sort -u)" +if [ -z "$WORKLOADS" ]; then bench_log "could not read the workload list from $BENCH_SRC"; exit 2; fi + +for workload in $WORKLOADS; do + if ! "$NODE_BIN" "$WORK_DIR/harness.js" "$DIST" "$workload" "$APP_NAME" \ + >> "$RAW" 2>>"$WORK_DIR/stderr.txt"; then + bench_log "workload $workload failed" + sed -n '1,40p' "$WORK_DIR/stderr.txt" >&2 + exit 1 + fi +done + +if ! grep -q '^BENCHSUITE ' "$RAW"; then + bench_log "benchmark did not reach its final marker -- output follows" + sed -n '1,40p' "$RAW" >&2 + sed -n '1,20p' "$WORK_DIR/stderr.txt" >&2 + exit 1 +fi + +SUSPENSION_REPORT="$DIST/suspension-report.txt" +[ -f "$SUSPENSION_REPORT" ] || SUSPENSION_REPORT="$(find "$OUT" -name suspension-report.txt -print -quit || true)" + +BUNDLE_BYTES=$(cat "$DIST"/translated_app*.js | wc -c | tr -d ' ') +YIELDS=$(cat "$DIST"/translated_app*.js | grep -o 'yield\*' | wc -l | tr -d ' ') +GENERATORS=$(cat "$DIST"/translated_app*.js | grep -o 'function\*' | wc -l | tr -d ' ') + +exec 3>&1 +BASELINE="$BASELINE" JSON_OUT="$JSON_OUT" RAW="$RAW" REPORT="$SUSPENSION_REPORT" \ + BUNDLE_BYTES="$BUNDLE_BYTES" YIELDS="$YIELDS" GENERATORS="$GENERATORS" \ + python3 "$SCRIPT_DIR/lib/js_throughput_report.py" >&3 diff --git a/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptBundleWriter.java b/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptBundleWriter.java index 98d6b085961..9fe8822832b 100644 --- a/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptBundleWriter.java +++ b/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptBundleWriter.java @@ -1707,15 +1707,56 @@ static Set collectBridgeReferencedCn1Tokens() { // to the in-bundle string scan only) } java.util.regex.Pattern literal = java.util.regex.Pattern.compile("[\"'](cn1_[A-Za-z0-9_]+)[\"']"); + // A name the bridge only ever LOOKS UP is not a name the bridge + // replaces, and only replacement needs protection. + // + // ``resolveVirtual(x, "cn1_s_toString_R_java_lang_String")`` is a call; + // its result is driven through ``cn1_ivAdapt`` / ``adaptVirtualResult``, + // which tolerate a plain function. Replacement instead assigns to + // ``classDef.methods[...]``, and every such site in the bridge is + // scoped to one class. Counting a lookup as a replacement is not the + // "handful of generators" the original comment estimated: ``toString`` + // and ``equals`` are named exactly this way, and between them that + // seeded 781 methods suspending and made 2,874 dispatch sites + // ``yield*`` -- the two largest entries in the whole report. + // + // The test is deliberately all-or-nothing: a token is dropped only + // when EVERY occurrence of it is a resolveVirtual argument. One + // assignment, one bindNative array entry, one mention anywhere else, + // and it stays protected. That keeps names reached through a variable + // (``const id = "cn1_s_..."; cls.methods[id] = fn``) safe, because the + // literal that feeds the variable is not itself a lookup. + java.util.regex.Pattern lookup = java.util.regex.Pattern.compile( + "resolveVirtual\\s*\\([^,()]*,\\s*[\"'](cn1_[A-Za-z0-9_]+)[\"']"); + Map counts = new HashMap(); for (String src : sources) { java.util.regex.Matcher m = literal.matcher(src); while (m.find()) { - tokens.add(m.group(1)); + bump(counts, m.group(1), 0); + } + java.util.regex.Matcher l = lookup.matcher(src); + while (l.find()) { + bump(counts, l.group(1), 1); + } + } + for (Map.Entry entry : counts.entrySet()) { + int[] seen = entry.getValue(); + if (seen[0] > seen[1]) { + tokens.add(entry.getKey()); } } return tokens; } + private static void bump(Map counts, String token, int slot) { + int[] seen = counts.get(token); + if (seen == null) { + seen = new int[2]; + counts.put(token, seen); + } + seen[slot]++; + } + /** * Collects the {@code cn1_...} method tokens of native bridge bindings * whose wrapper is a plain {@code function} (NOT {@code function*}). diff --git a/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptMethodGenerator.java b/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptMethodGenerator.java index 3341dbe82b6..1ad0c83e153 100644 --- a/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptMethodGenerator.java +++ b/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptMethodGenerator.java @@ -614,11 +614,24 @@ private static boolean emitsMonitor(BytecodeMethod method) { private static boolean isInvokeSuspending(Invoke invoke) { int op = invoke.getOpcode(); if (op == Opcodes.INVOKEVIRTUAL || op == Opcodes.INVOKEINTERFACE) { - // CHA result: the signature is sync only if NO class's - // impl is suspending. Consult the set exported by the - // suspension analysis. Default (no set → all dispatches - // suspending) preserves the historical over-conservative - // behaviour when the analysis is disabled. + // Ask the analysis about THIS call site, receiver type included. + // Keying on the bare signature -- which is what this did, and what + // the whole JS backend still does for its runtime dispatch ids -- + // means one blocking ``run()V`` anywhere makes every ``run()V`` + // call site in the program a suspension point. + // + // The answer MUST match the one the analysis used when it decided + // whether the ENCLOSING method is a generator, so both sides go + // through the same DispatchModel rather than reimplementing the + // rule; a ``yield*`` emitted into a plain ``function`` is a JS + // SyntaxError, not a subtle mistake. + JavascriptSuspensionAnalysis.DispatchModel model = + JavascriptSuspensionAnalysis.exportedDispatchModel; + if (model != null) { + return model.isDispatchSuspending(invoke.getOwner(), invoke.getName(), invoke.getDesc()); + } + // Analysis disabled (-Dparparvm.js.suspension.off): unchanged + // historical behaviour. java.util.Set suspendingSigs = JavascriptSuspensionAnalysis.exportedSuspendingSigs; if (suspendingSigs == null) { return true; diff --git a/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptReachability.java b/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptReachability.java index c636072bd77..b03e0c0f9b3 100644 --- a/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptReachability.java +++ b/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptReachability.java @@ -128,6 +128,12 @@ private static final class VirtualCall { static int run(List classes, List classPool, String[] nativeSources) { + // Drop any previous translation's answer FIRST. Surefire reuses one + // JVM across fixtures and the build server translates repeatedly, so a + // stale instantiated set would let the suspension analysis resolve + // this app's call sites against another app's type graph -- which + // under-approximates, i.e. fails in the unsafe direction. + exportedInstantiated = Collections.emptySet(); JavascriptReachability rta = new JavascriptReachability(); // The conservative pass may have removed an entire class after // eliminating the only method that instantiated it. RTA can later @@ -140,9 +146,220 @@ static int run(List classes, List classPool, rta.propagate(); int eliminated = rta.eliminate(candidates); rta.mergeInstantiatedClasses(classes, candidates); + // Publish the instantiated set. The suspension analysis runs straight + // after us over the SURVIVING classes and needs exactly this fact to + // decide which overrides a given call site can really reach; without + // it, it has to assume every same-named method in the program. + exportedInstantiated = Collections.unmodifiableSet(new HashSet(rta.instantiated)); return eliminated; } + /** + * Classes RTA proved are actually instantiated, published by + * {@link #run}. Empty when RTA did not run (``-Dparparvm.js.rta.off``), + * which every consumer must read as "no information" rather than "nothing + * is instantiated" -- see {@link Model#resolveImpls}. + */ + static volatile Set exportedInstantiated = Collections.emptySet(); + + /** + * The subtype relation plus RTA's instantiated set, indexed over one class + * list, answering "which method bodies can this call site actually reach". + * + * This exists because keying a virtual call by ``name + descriptor`` alone + * -- which is what the JS backend did everywhere -- collapses every + * same-named method in the program into one bucket. One blocking + * ``run()V`` then makes every ``run()V`` call site in the program a + * suspension point. + */ + static final class Model { + private final Map byName; + private final Map> subclassesOf; + private final Set instantiated; + // resolveImpls is called once per virtual call site and the subtype + // walk is recursive, so memoise on ``owner#name+desc``. Class names + // are sanitized to identifier characters, so '#' cannot collide. + private final Map> memo = new HashMap>(); + private final Map> coneMemo = new HashMap>(); + + private Model(Map byName, Map> subclassesOf, + Set instantiated) { + this.byName = byName; + this.subclassesOf = subclassesOf; + this.instantiated = instantiated; + } + + /** + * The concrete bodies an {@code INVOKEVIRTUAL} / {@code + * INVOKEINTERFACE} on {@code owner} can dispatch to, given what RTA + * proved instantiated. + * + * Returns {@code null} for "no information", and the caller MUST fall + * back to the signature-wide answer when it does. That happens when + * the owner is not a class we indexed (an array type, a class the + * conservative pass removed), or when nothing in the owner's subtype + * cone is instantiated. The second case is the important one: an empty + * result would otherwise read as "reaches nothing, so it cannot + * suspend", which is exactly the wrong conclusion if the instantiated + * set is missing an edge. + */ + List resolveImpls(String owner, String name, String desc) { + if (owner == null || name == null || desc == null) { + return null; + } + String cls = JavascriptNameUtil.sanitizeClassName(owner); + String key = cls + "#" + name + desc; + if (memo.containsKey(key)) { + return memo.get(key); + } + List result = null; + if (byName.containsKey(cls)) { + Set found = Collections.newSetFromMap( + new IdentityHashMap()); + if (collectFrom(cls, name, desc, found, new HashSet()) && !found.isEmpty()) { + result = new ArrayList(found); + } + } + memo.put(key, result); + return result; + } + + /** + * Every type a dispatch on {@code owner} could have as its runtime + * receiver: {@code owner} plus its transitive subtypes, DECLARED + * rather than filtered by {@link #instantiated}. + * + * The instantiated filter is deliberately not applied here. This is + * used to ask whether a bridge type is reachable through a call site, + * and a JSO bridge type is never created by a Java {@code new} -- it + * arrives from the host -- so RTA has no reason to consider it + * instantiated and filtering would answer "no bridge type here" for + * every call site. + * + * Returns {@code null} when the owner is not an indexed class, which + * the caller must read as "no information". + */ + Set coneTypes(String owner) { + if (owner == null) { + return null; + } + String cls = JavascriptNameUtil.sanitizeClassName(owner); + if (coneMemo.containsKey(cls)) { + return coneMemo.get(cls); + } + Set result = null; + if (byName.containsKey(cls)) { + result = new HashSet(); + collectCone(cls, result); + } + coneMemo.put(cls, result); + return result; + } + + private void collectCone(String type, Set out) { + if (!out.add(type)) { + return; + } + Set subs = subclassesOf.get(type); + if (subs != null) { + for (String sub : subs) { + collectCone(sub, out); + } + } + } + + /** + * Mirrors {@link JavascriptReachability#dispatchVirtualFromInstantiated} + * plus {@link JavascriptReachability#enqueueResolved}: every + * instantiated type in the cone contributes the concrete body it + * inherits, found by walking its superclass chain. + */ + private boolean collectFrom(String type, String name, String desc, + Set out, Set seen) { + if (!seen.add(type)) { + return true; + } + if (instantiated.contains(type)) { + BytecodeMethod impl = walkUp(type, name, desc); + if (impl == null) { + // An instantiated receiver whose body we cannot name is + // NOT "contributes nothing" -- the runtime's resolveVirtual + // also walks interfaces and then falls back to the global + // native table, and both of those can land on a generator + // this walk never saw. Silently skipping the type would + // under-approximate, which is the one direction that + // breaks. Give up on the whole query instead. + return false; + } + out.add(impl); + } + Set subs = subclassesOf.get(type); + if (subs != null) { + for (String sub : subs) { + if (!collectFrom(sub, name, desc, out, seen)) { + return false; + } + } + } + return true; + } + + private BytecodeMethod walkUp(String startClass, String name, String desc) { + String normalized; + if ("".equals(name)) { + normalized = "__INIT__"; + } else if ("".equals(name)) { + normalized = "__CLINIT__"; + } else { + normalized = name; + } + String current = startClass; + Set visited = new HashSet(); + while (current != null && visited.add(current)) { + ByteCodeClass cls = byName.get(current); + if (cls == null) { + return null; + } + for (BytecodeMethod m : cls.getMethods()) { + if (!normalized.equals(m.getMethodName()) || !desc.equals(m.getSignature())) { + continue; + } + if (m.isAbstract()) { + break; + } + // An eliminated body is not reachable, so it contributes + // nothing to what this call site can suspend on. + if (m.isEliminated()) { + break; + } + return m; + } + String base = cls.getBaseClass(); + current = base == null ? null : JavascriptNameUtil.sanitizeClassName(base); + } + return null; + } + } + + /** + * Builds a {@link Model} over {@code classes} -- which must be the list + * the CALLER is going to classify, not the wider pool RTA indexed, or the + * model can hand back method objects that list does not contain. + * + * Returns {@code null} when RTA published no instantiated set, so callers + * keep their existing conservative behaviour under + * ``-Dparparvm.js.rta.off``. + */ + static Model modelFor(List classes) { + Set instantiated = exportedInstantiated; + if (instantiated.isEmpty()) { + return null; + } + JavascriptReachability indexer = new JavascriptReachability(); + indexer.index(classes); + return new Model(indexer.byName, indexer.subclassesOf, instantiated); + } + private void index(List classes) { for (ByteCodeClass cls : classes) { byName.put(cls.getClsName(), cls); diff --git a/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptSuspensionAnalysis.java b/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptSuspensionAnalysis.java index 2c32465daf5..c35e979df9a 100644 --- a/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptSuspensionAnalysis.java +++ b/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptSuspensionAnalysis.java @@ -6,6 +6,19 @@ * published by the Free Software Foundation. Codename One designates this * particular file as subject to the "Classpath" exception as provided * by Oracle in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Codename One through http://www.codenameone.com/ if you + * need additional information or have any questions. */ package com.codename1.tools.translator; @@ -13,6 +26,7 @@ import com.codename1.tools.translator.bytecodes.BasicInstruction; import com.codename1.tools.translator.bytecodes.Instruction; import com.codename1.tools.translator.bytecodes.Invoke; +import java.io.File; import java.util.ArrayDeque; import java.util.ArrayList; import java.util.Collections; @@ -64,6 +78,16 @@ final class JavascriptSuspensionAnalysis { // on JSO-bridge classes, or string-referenced by the bridge JS (see // seedBridgeReferenced). Unconditionally suspending. private final Set jsoDeclaredSigs = new java.util.HashSet(); + // Sigs protected because the BRIDGE JS names their class-free dispatch id + // as a string literal. Unlike the JSO set these stay signature-wide: the + // bridge also installs overrides through ``classDef.methods[id] = fn`` + // with a computed key, so there is no receiver type to reason about. + private final Set bridgeDispatchSigs = new java.util.HashSet(); + // Every class assignable to JSObject. A ``bindNative`` override can only + // land on one of these, so a call site whose receiver cone contains none + // of them cannot reach one -- which is what makes the JSO protection + // cone-aware rather than signature-wide. + private final Set jsoBridgeClasses = new java.util.HashSet(); // Native bridge bindings whose wrapper is a plain ``function`` (not // ``function*``): SYNCHRONOUS natives that never yield. They must NOT be @@ -80,11 +104,103 @@ final class JavascriptSuspensionAnalysis { // drop the ``yield*`` ceremony and use a sync dispatcher. static volatile java.util.Set exportedSuspendingSigs = java.util.Collections.emptySet(); - static int run(List classes) { + // Receiver-type information from the RTA pass that ran immediately before + // us. Null when RTA did not run, or when owner-aware classification is + // switched off -- both cases fall back to the historical signature-wide + // behaviour, which is strictly more conservative. + private JavascriptReachability.Model rta; + + /** + * The call-site decision, shared by this analysis and the emitter. + * + * They MUST agree: the analysis decides whether a method is emitted + * ``function*``, the emitter decides whether each call inside it is + * ``yield*``, and a ``yield*`` inside a plain ``function`` is a JS + * SyntaxError rather than a subtle bug. So there is exactly one + * implementation of the rule and both sides call it. + */ + static final class DispatchModel { + private final JavascriptReachability.Model rta; + // Declared on a JSO bridge class. Suspending only when the call + // site's receiver cone can actually reach one of those classes. + private final java.util.Set jsoSigs; + // Named by the bridge JS as a class-free dispatch id. Suspending + // whatever the receiver is -- see bridgeDispatchSigs. + private final java.util.Set bridgeSigs; + private final java.util.Set jsoClasses; + // Signature-wide fallback, i.e. the historical answer. + private final java.util.Set suspendingSigs; + + DispatchModel(JavascriptReachability.Model rta, java.util.Set jsoSigs, + java.util.Set bridgeSigs, java.util.Set jsoClasses, + java.util.Set suspendingSigs) { + this.rta = rta; + this.jsoSigs = jsoSigs; + this.bridgeSigs = bridgeSigs; + this.jsoClasses = jsoClasses; + this.suspendingSigs = suspendingSigs; + } + + boolean isDispatchSuspending(String owner, String name, String desc) { + String sig = name + desc; + if (rta == null || isUnconditionallySuspendingDispatch(rta, jsoSigs, bridgeSigs, + jsoClasses, owner, sig)) { + return rta == null ? suspendingSigs.contains(sig) : true; + } + List impls = rta.resolveImpls(owner, name, desc); + if (impls == null) { + return suspendingSigs.contains(sig); + } + for (int i = 0; i < impls.size(); i++) { + if (impls.get(i).isJavascriptSuspending()) { + return true; + } + } + return false; + } + } + + /** + * Published for {@link JavascriptMethodGenerator}. Null until this + * analysis has run, which the emitter reads as "assume suspending". + */ + static volatile DispatchModel exportedDispatchModel = null; + + // Where the suspension report is written -- always, beside the bundle, as + // ``suspension-report.txt``. It exists because the sync/suspending split + // is the number this pass exists to move, and the only thing emitted + // before was a total (Parser, behind -verbose) that could not say WHICH + // rule was responsible for the suspending half. Null only when no output + // directory was supplied, which is the in-memory unit-test path. + private String reportPath; + // Method -> the rule that FIRST classified it suspending. A method can + // have several independent causes; this records the one that won the race + // in the worklist, which is why the ranking below is documented as an + // upper bound on beneficiaries rather than a prediction. + private final Map suspendReason = new IdentityHashMap(); + // Signature -> number of INVOKEVIRTUAL / INVOKEINTERFACE call sites that + // dispatch on it. Captured in propagate(); for a suspending signature this + // is literally the number of ``yield*`` sites it is responsible for. + private Map dispatchSiteCount = java.util.Collections.emptyMap(); + + static int run(List classes, File outputDirectory) { + // Same reason as JavascriptReachability.run: never let a previous + // translation's model answer this one's questions. Cleared before the + // kill-switch return too, so the disabled path cannot inherit a model + // either. + exportedDispatchModel = null; if (System.getProperty("parparvm.js.suspension.off") != null) { return 0; } JavascriptSuspensionAnalysis a = new JavascriptSuspensionAnalysis(); + // Always written, always beside the bundle. A diagnostic behind a + // system property is a diagnostic nobody sets, and the sync/suspending + // split is the number this whole pass exists to move -- it belongs in + // the build output where CI and a bisect can both read it. + if (outputDirectory != null) { + a.reportPath = new File(outputDirectory, "suspension-report.txt").getAbsolutePath(); + } + a.rta = JavascriptReachability.modelFor(classes); a.index(classes); a.seedDirectlySuspending(classes); a.seedBridgeReferenced(classes); @@ -98,6 +214,22 @@ private void index(List classes) { } } + /** + * Adds {@code m} to the suspending set, recording WHY when the opt-in + * report is on. Returns true when this call is the one that added it, so + * it is a drop-in for {@code suspending.add(m)} at the propagation + * worklist sites that depend on that return value. + */ + private boolean markSuspending(BytecodeMethod m, String reason) { + if (!suspending.add(m)) { + return false; + } + if (reportPath != null) { + suspendReason.put(m, reason); + } + return true; + } + private void seedDirectlySuspending(List classes) { // Every method on a JSO-bridge class is conservatively // suspending. These classes (anything assignable to @@ -111,7 +243,6 @@ private void seedDirectlySuspending(List classes) { // already seen this manifest as ``Window.current()`` returning // a non-wrapped value in the init path). Mark them suspending // up front so the caller stays ``yield*``-wrapped regardless. - java.util.Set jsoBridgeClasses = new java.util.HashSet(); for (ByteCodeClass cls : classes) { if (isJsoBridgeClass(cls)) { jsoBridgeClasses.add(cls.getClsName()); @@ -162,11 +293,21 @@ private void seedDirectlySuspending(List classes) { // generator leak as a value; ``cn1_ivs*`` drives a // one-shot and throws a named error on a true gap // instead (see the runtime helper). - if ((m.isNative() && !isSyncNativeBinding(cls, m)) - || m.isSynchronizedMethod() - || hasMonitorOps(m) - || (clsIsJso && !isSyncNativeBinding(cls, m))) { - suspending.add(m); + // Split into a labelled chain rather than one boolean so the + // report can name the rule. The disjunction is unchanged -- + // order decides only which label wins, never the outcome. + String seed = null; + if (m.isNative() && !isSyncNativeBinding(cls, m)) { + seed = "native"; + } else if (m.isSynchronizedMethod()) { + seed = "synchronized"; + } else if (hasMonitorOps(m)) { + seed = "monitor-op"; + } else if (clsIsJso && !isSyncNativeBinding(cls, m)) { + seed = "jso-bridge-class"; + } + if (seed != null) { + markSuspending(m, seed); } } } @@ -207,12 +348,11 @@ private void seedBridgeReferenced(List classes) { referenced = true; } if (referenced && !isSyncNativeBinding(cls, m)) { - suspending.add(m); + markSuspending(m, "bridge-referenced"); if (dispatchable) { // Virtual dispatch can land on the runtime-installed - // override too -- protect the whole signature, same - // as the JSO-declared sigs. - jsoDeclaredSigs.add(m.getMethodName() + m.getSignature()); + // override too -- protect the whole signature. + bridgeDispatchSigs.add(m.getMethodName() + m.getSignature()); } } } @@ -326,6 +466,7 @@ private void propagate(List classes) { // Must be folded in BEFORE the caller scan below so dispatching // callers get escalated. suspendingSigs.addAll(jsoDeclaredSigs); + suspendingSigs.addAll(bridgeDispatchSigs); for (ByteCodeClass cls : classes) { for (BytecodeMethod caller : cls.getMethods()) { if (caller.isEliminated() || caller.isAbstract()) { @@ -346,27 +487,43 @@ private void propagate(List classes) { if (target == null) { continue; } - List callers = callersOf.get(target); - if (callers == null) { - callers = new ArrayList(); - callersOf.put(target, callers); - } - callers.add(caller); + addCaller(callersOf, target, caller); } else if (op == Opcodes.INVOKEVIRTUAL || op == Opcodes.INVOKEINTERFACE) { String sig = inv.getName() + inv.getDesc(); - List callers = sigCallersOf.get(sig); - if (callers == null) { - callers = new ArrayList(); - sigCallersOf.put(sig, callers); + // Resolve the call site against its RECEIVER TYPE + // rather than its bare signature. A cone that + // resolves gives us exact per-impl edges, so a + // blocking ``run()V`` somewhere else in the program + // no longer reaches this caller at all. + List impls = rta == null + || isUnconditionallySuspendingDispatch(rta, jsoDeclaredSigs, + bridgeDispatchSigs, jsoBridgeClasses, inv.getOwner(), sig) + ? null + : rta.resolveImpls(inv.getOwner(), inv.getName(), inv.getDesc()); + if (impls != null) { + for (int i = 0; i < impls.size(); i++) { + BytecodeMethod impl = impls.get(i); + addCaller(callersOf, impl, caller); + if (suspending.contains(impl)) { + markSuspending(caller, "dispatch:" + + JavascriptNameUtil.sanitizeClassName(inv.getOwner()) + + "." + sig); + } + } + continue; } - callers.add(caller); + // No receiver information (array owner, unindexed + // class, nothing in the cone instantiated, or a + // signature the bridge can override at runtime): + // keep the historical signature-wide edge. + addSigCaller(sigCallersOf, sig, caller); // Early escalation: if ANY impl of the sig is // already known suspending, this caller also // needs to be suspending. Add to the initial // worklist via the standard ``suspending.add`` // + propagate path below. if (suspendingSigs.contains(sig)) { - suspending.add(caller); + markSuspending(caller, "dispatch:" + sig); } } } @@ -380,7 +537,7 @@ private void propagate(List classes) { List directCallers = callersOf.get(suspended); if (directCallers != null) { for (BytecodeMethod caller : directCallers) { - if (suspending.add(caller)) { + if (markSuspending(caller, "calls:" + qualify(suspended))) { worklist.add(caller); } } @@ -395,7 +552,7 @@ private void propagate(List classes) { List sigCallers = sigCallersOf.get(sig); if (sigCallers != null) { for (BytecodeMethod caller : sigCallers) { - if (suspending.add(caller)) { + if (markSuspending(caller, "dispatch:" + sig)) { worklist.add(caller); } } @@ -403,10 +560,85 @@ private void propagate(List classes) { } } } + if (reportPath != null) { + // One entry per dispatch INSTRUCTION, so for a suspending sig this + // counts the ``yield*`` sites it costs. + Map counts = new HashMap(); + for (Map.Entry> e : sigCallersOf.entrySet()) { + counts.put(e.getKey(), Integer.valueOf(e.getValue().size())); + } + dispatchSiteCount = counts; + } // Publish the final suspending-sig set so the emitter can // consult it when deciding whether an INVOKEVIRTUAL / // INVOKEINTERFACE call site needs ``yield*`` wrapping. exportedSuspendingSigs = suspendingSigs; + exportedDispatchModel = new DispatchModel(rta, + new java.util.HashSet(jsoDeclaredSigs), + new java.util.HashSet(bridgeDispatchSigs), + new java.util.HashSet(jsoBridgeClasses), + suspendingSigs); + } + + /** + * True when this dispatch must be suspending regardless of which body it + * resolves to, so neither the analysis nor the emitter may consult the + * receiver cone. + * + * Two different protections live here and they are NOT the same rule: + * + * - {@code bridgeSigs} is signature-wide. The bridge JS installs an + * override through {@code classDef.methods[id] = fn} with a computed + * key, so there is no receiver type a static walk could check. + * - {@code jsoSigs} is cone-aware. Those overrides land on JSO bridge + * classes specifically, so a call whose receiver cone contains no such + * class cannot reach one. That distinction is the whole point: {@code + * getWidth()I} is declared on three JSO bridge interfaces, which used + * to make every {@code Component.getWidth()} in the program a + * suspension point. + * + * An unresolvable cone (array owner, class we did not index) answers true, + * because "we do not know" has to mean "assume the bridge can reach it". + */ + private static boolean isUnconditionallySuspendingDispatch(JavascriptReachability.Model rta, + Set jsoSigs, Set bridgeSigs, Set jsoClasses, + String owner, String sig) { + if (bridgeSigs.contains(sig)) { + return true; + } + if (!jsoSigs.contains(sig)) { + return false; + } + Set cone = rta.coneTypes(owner); + if (cone == null) { + return true; + } + for (String type : cone) { + if (jsoClasses.contains(type)) { + return true; + } + } + return false; + } + + private static void addCaller(Map> callersOf, + BytecodeMethod callee, BytecodeMethod caller) { + List callers = callersOf.get(callee); + if (callers == null) { + callers = new ArrayList(); + callersOf.put(callee, callers); + } + callers.add(caller); + } + + private static void addSigCaller(Map> sigCallersOf, + String sig, BytecodeMethod caller) { + List callers = sigCallersOf.get(sig); + if (callers == null) { + callers = new ArrayList(); + sigCallersOf.put(sig, callers); + } + callers.add(caller); } /** @@ -449,6 +681,8 @@ private BytecodeMethod resolveTarget(String owner, String name, String desc) { private int applyResults(List classes) { int sync = 0; int total = 0; + List methodLines = reportPath == null ? null : new ArrayList(); + Map causeCount = reportPath == null ? null : new HashMap(); for (ByteCodeClass cls : classes) { for (BytecodeMethod m : cls.getMethods()) { if (m.isEliminated()) { @@ -460,8 +694,131 @@ private int applyResults(List classes) { if (!isSuspending) { sync++; } + if (methodLines == null) { + continue; + } + String qualified = cls.getClsName() + "." + m.getMethodName() + m.getSignature(); + if (!isSuspending) { + methodLines.add("M SYNC " + qualified); + continue; + } + // An abstract method has no body to classify -- it is forced + // suspending so a caller that cannot see the override still + // emits ``yield*``. It has no seed rule, so name it as itself. + String cause = suspendReason.get(m); + if (cause == null) { + cause = m.isAbstract() ? "abstract" : "unattributed"; + } + methodLines.add("M SUSP " + qualified + " " + cause); + Integer prev = causeCount.get(cause); + causeCount.put(cause, Integer.valueOf(prev == null ? 1 : prev.intValue() + 1)); } } + if (methodLines != null) { + writeReport(total, sync, methodLines, causeCount); + } return sync; } + + /** + * Orders report keys by their count, largest first, then by name so the + * order is total and two runs of the report diff cleanly. A missing key + * counts as zero. Static and named rather than an anonymous inner class + * because SpotBugs runs as a zero-findings gate over this project. + */ + private static final class ByCountDescending + implements java.util.Comparator, java.io.Serializable { + private static final long serialVersionUID = 1L; + private final Map counts; + + ByCountDescending(Map counts) { + this.counts = counts; + } + + public int compare(String a, String b) { + Integer ca = counts.get(a); + Integer cb = counts.get(b); + int va = ca == null ? 0 : ca.intValue(); + int vb = cb == null ? 0 : cb.intValue(); + if (va != vb) { + return vb < va ? -1 : 1; + } + return a.compareTo(b); + } + } + + /** ``owner.name+descriptor``, the identity used throughout the report. */ + private static String qualify(BytecodeMethod m) { + return m.getClsName() + "." + m.getMethodName() + m.getSignature(); + } + + /** + * Writes the opt-in report. Deliberately plain text and sorted, so two + * runs diff cleanly and a shell can aggregate it without a parser. + * + * The ranking in section 2 is the point of the whole file: a suspending + * SIGNATURE costs one ``yield*`` per dispatch site AND forces every + * method containing one of those sites to be a generator, so the + * signatures at the top are where the bundle's generator population + * actually comes from. + * + * Read ``firstCause`` as an UPPER BOUND on beneficiaries, not a + * prediction: a method is recorded against whichever cause reached it + * first, and removing that cause can leave it suspending for another. + */ + private void writeReport(int total, int sync, List methodLines, + Map causeCount) { + java.util.Set suspendingSigs = exportedSuspendingSigs; + List sigs = new ArrayList(suspendingSigs); + Collections.sort(sigs); + // Rank suspending signatures by dispatch sites, then by name so the + // order is total and the file diffs cleanly. + Map sites = dispatchSiteCount; + Collections.sort(sigs, new ByCountDescending(sites)); + List causes = new ArrayList(causeCount.keySet()); + Collections.sort(causes, new ByCountDescending(causeCount)); + Collections.sort(methodLines); + java.io.PrintWriter out = null; + try { + out = new java.io.PrintWriter(new java.io.OutputStreamWriter( + new java.io.FileOutputStream(reportPath), "UTF-8")); + out.println("# ParparVM JavaScript suspension report"); + out.println("# A suspending method is emitted ``function*`` and every call to it is"); + out.println("# ``yield*``; a synchronous one is a plain function called directly."); + out.println("TOTAL " + total); + out.println("SYNC " + sync); + out.println("SUSPENDING " + (total - sync)); + out.println("SUSPENDING_SIGS " + suspendingSigs.size()); + out.println("#"); + out.println("# Section 1: first-recorded cause, most common first."); + out.println("# CAUSE "); + for (String cause : causes) { + out.println("CAUSE " + causeCount.get(cause) + " " + cause); + } + out.println("#"); + out.println("# Section 2: suspending signatures ranked by dispatch call sites."); + out.println("# SIG "); + for (String sig : sigs) { + Integer siteCount = sites.get(sig); + Integer firstCause = causeCount.get("dispatch:" + sig); + out.println("SIG " + (siteCount == null ? 0 : siteCount.intValue()) + + " " + (firstCause == null ? 0 : firstCause.intValue()) + + " " + sig); + } + out.println("#"); + out.println("# Section 3: every live method."); + out.println("# M SYNC | M SUSP "); + for (String line : methodLines) { + out.println(line); + } + } catch (java.io.IOException err) { + // A diagnostic must never be the thing that breaks the build. + System.out.println("JS suspension report could not be written to " + + reportPath + ": " + err.getMessage()); + } finally { + if (out != null) { + out.close(); + } + } + } } diff --git a/vm/ByteCodeTranslator/src/com/codename1/tools/translator/Parser.java b/vm/ByteCodeTranslator/src/com/codename1/tools/translator/Parser.java index d462fe03d28..bb5282ba34a 100644 --- a/vm/ByteCodeTranslator/src/com/codename1/tools/translator/Parser.java +++ b/vm/ByteCodeTranslator/src/com/codename1/tools/translator/Parser.java @@ -842,7 +842,7 @@ public static void writeOutput(File outputDirectory) throws Exception { // methods don't pollute the analysis. if (ByteCodeTranslator.output == ByteCodeTranslator.OutputType.OUTPUT_TYPE_JAVASCRIPT) { Date suspStart = new Date(); - int syncCount = JavascriptSuspensionAnalysis.run(classes); + int syncCount = JavascriptSuspensionAnalysis.run(classes, outputDirectory); Date suspEnd = new Date(); if (ByteCodeTranslator.verbose) { System.out.println("JS suspension analysis: " + syncCount diff --git a/vm/benchmarks/javascript/JsThroughputBench.java b/vm/benchmarks/javascript/JsThroughputBench.java new file mode 100644 index 00000000000..712c52a120d --- /dev/null +++ b/vm/benchmarks/javascript/JsThroughputBench.java @@ -0,0 +1,348 @@ +/* + * Copyright (c) 2026, Codename One and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Codename One designates this + * particular file as subject to the "Classpath" exception as provided + * by Oracle in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Codename One through http://www.codenameone.com/ if you + * need additional information or have any questions. + */ + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.Iterator; +import java.util.List; +import java.util.Map; + +/** + * Dispatch-shaped throughput benchmark for the JavaScript target. + * + * WHY THIS EXISTS. The JS backend compiles a method that can block into a JS + * generator and every call to it into ``yield*``. Reducing how many methods + * and call sites need that is the main lever the backend has, and until this + * file there was no way to say what reducing it was WORTH: the bundle barely + * changes (``yield* `` is seven characters), the screenshot suite is pass/fail, + * and the lifecycle harness only reports milestones. So a real reduction in + * generator density and a change that did nothing looked identical. + * + * Every workload here is therefore chosen to be dominated by CALL DISPATCH + * rather than by arithmetic, allocation or host round-trips, and the suite + * deliberately contains controls that the dispatch work CANNOT improve + * (``arithControl``, ``suspendControl``) so a global speedup -- a faster + * machine, a quieter host, a JIT that warmed differently -- is distinguishable + * from a real one. + * + * EVERY WORKLOAD REPORTS A CHECKSUM, and the runner refuses a result whose + * checksum moved. A benchmark that gets faster by doing less work is the + * failure mode this suite is most exposed to, because "fewer generators" and + * "fewer calls" are easy to confuse. + * + * Output is one ``BENCH`` line per workload plus a ``BENCHSUITE`` line; see + * scripts/run-javascript-throughput-benchmark.sh, which parses them. + */ +public class JsThroughputBench { + + /** Timed repetitions kept; the runner reports the fastest. */ + private static final int REPS = 7; + /** Untimed repetitions first, so V8 has tiered up before we measure. */ + private static final int WARMUPS = 3; + + private static long suiteChecksum; + + /** + * Set by the harness before {@code main} runs (a plain static write into + * the class's field table). Empty means "run everything". + * + * Per-workload isolation is not a convenience, it is a correctness + * requirement for this suite. Measured in one process, a workload inherits + * whatever heap and JIT state the workloads before it left behind: + * ``iteratorWalk`` takes 8.5ms run first and 15ms run eighth, so making an + * EARLIER workload faster shifts a LATER one and the suite reports a + * regression in code that is byte-for-byte identical. That happened, and + * it cost an hour to disprove. + */ + public static String only = ""; + + public static void main(String[] args) { + run("monoVirtual", 1); + run("polyVirtual", 2); + run("megaVirtual", 3); + run("ifaceDispatch", 4); + run("toStringHeavy", 5); + run("equalsHeavy", 6); + run("hashCodeHeavy", 7); + run("iteratorWalk", 8); + run("stringOps", 9); + run("mapChurn", 10); + run("arithControl", 11); + run("suspendControl", 12); + System.out.println("BENCHSUITE checksum=" + suiteChecksum); + } + + /** + * Runs one workload and prints its best time. The fastest of N is used + * rather than the mean because the slow tail here is the host (GC, another + * process, a timer) and not a property of the code under test; the mean + * would measure this machine's mood. + */ + private static void run(String id, int which) { + if (only != null && only.length() > 0 && !only.equals(id)) { + return; + } + long best = Long.MAX_VALUE; + long checksum = 0; + for (int i = 0; i < WARMUPS; i++) { + checksum = dispatch(which); + } + for (int i = 0; i < REPS; i++) { + long start = System.nanoTime(); + checksum = dispatch(which); + long elapsed = System.nanoTime() - start; + if (elapsed < best) { + best = elapsed; + } + } + suiteChecksum = suiteChecksum * 31 + checksum; + System.out.println("BENCH id=" + id + " ns=" + best + " checksum=" + checksum); + } + + private static long dispatch(int which) { + switch (which) { + case 1: return monoVirtual(); + case 2: return polyVirtual(); + case 3: return megaVirtual(); + case 4: return ifaceDispatch(); + case 5: return toStringHeavy(); + case 6: return equalsHeavy(); + case 7: return hashCodeHeavy(); + case 8: return iteratorWalk(); + case 9: return stringOps(); + case 10: return mapChurn(); + case 11: return arithControl(); + case 12: return suspendControl(); + default: return 0; + } + } + + // ---------------------------------------------------------------- shapes + + abstract static class Shape { + abstract int area(int n); + public String toString() { return "Shape"; } + } + static final class Sq extends Shape { + int area(int n) { return n * n; } + public String toString() { return "Sq"; } + } + static final class Tri extends Shape { + int area(int n) { return (n * n) / 2; } + public String toString() { return "Tri"; } + } + static final class Cir extends Shape { + int area(int n) { return 3 * n * n; } + public String toString() { return "Cir"; } + } + static final class Hex extends Shape { int area(int n) { return 6 * n; } } + static final class Oct extends Shape { int area(int n) { return 8 * n; } } + static final class Pen extends Shape { int area(int n) { return 5 * n; } } + static final class Rho extends Shape { int area(int n) { return 4 * n; } } + static final class Trp extends Shape { int area(int n) { return 7 * n; } } + + interface Sink { int accept(int v); } + static final class AddSink implements Sink { public int accept(int v) { return v + 1; } } + static final class MulSink implements Sink { public int accept(int v) { return v * 2; } } + static final class XorSink implements Sink { public int accept(int v) { return v ^ 7; } } + static final class SubSink implements Sink { public int accept(int v) { return v - 3; } } + + // ------------------------------------------------------------- workloads + + /** One impl behind a supertype reference: the devirtualizable shape. */ + private static long monoVirtual() { + Shape s = new Sq(); + long acc = 0; + for (int i = 0; i < 400000; i++) { + acc += s.area(i & 63); + } + return acc; + } + + /** Three impls in rotation: too many to devirtualize, few enough to be a + * polymorphic inline cache hit in V8 IF the call is a direct call. */ + private static long polyVirtual() { + Shape[] shapes = new Shape[]{ new Sq(), new Tri(), new Cir() }; + long acc = 0; + for (int i = 0; i < 400000; i++) { + acc += shapes[i % 3].area(i & 63); + } + return acc; + } + + /** Eight impls: megamorphic, so the dispatch mechanism itself dominates. */ + private static long megaVirtual() { + Shape[] shapes = new Shape[]{ new Sq(), new Tri(), new Cir(), new Hex(), + new Oct(), new Pen(), new Rho(), new Trp() }; + long acc = 0; + for (int i = 0; i < 400000; i++) { + acc += shapes[i & 7].area(i & 63); + } + return acc; + } + + private static long ifaceDispatch() { + Sink[] sinks = new Sink[]{ new AddSink(), new MulSink(), new XorSink(), new SubSink() }; + long acc = 0; + for (int i = 0; i < 400000; i++) { + acc += sinks[i & 3].accept(i & 1023); + } + return acc; + } + + /** ``toString()`` is one of the two signatures the bridge protects + * program-wide, so this is the direct read on that protection. */ + private static long toStringHeavy() { + Object[] objs = new Object[]{ new Sq(), new Tri(), new Cir(), "literal", + Integer.valueOf(7), new StringBuilder("sb") }; + long acc = 0; + for (int i = 0; i < 120000; i++) { + acc += objs[i % 6].toString().length(); + } + return acc; + } + + /** ``equals(Object)`` is the other bridge-protected signature. */ + private static long equalsHeavy() { + Object[] objs = new Object[]{ "alpha", "beta", Integer.valueOf(3), + Integer.valueOf(4), Character.valueOf('x'), Long.valueOf(9L) }; + long acc = 0; + for (int i = 0; i < 200000; i++) { + if (objs[i % 6].equals(objs[(i + 1) % 6])) { + acc++; + } + acc += 2; + } + return acc; + } + + private static long hashCodeHeavy() { + Object[] objs = new Object[]{ "alpha", "beta", Integer.valueOf(3), + Integer.valueOf(4), Character.valueOf('x'), Boolean.TRUE }; + long acc = 0; + for (int i = 0; i < 200000; i++) { + acc += objs[i % 6].hashCode() & 1023; + } + return acc; + } + + /** ``hasNext()`` / ``next()`` are interface dispatch in the hottest loop + * shape ordinary Codename One code writes. */ + private static List iteratorList; + + private static long iteratorWalk() { + // Built ONCE, outside the timed region. Allocating 400 boxed Integers + // and growing an ArrayList inside the measurement made this workload + // dominated by allocation and GC timing rather than by the iterator + // dispatch it is named for -- it measured 8.5ms in one harness mode + // and 14.9ms in another for byte-identical code, and reported an 11% + // regression against a bundle whose iteratorWalk and all nineteen of + // its iterator callees were unchanged. + if (iteratorList == null) { + List list = new ArrayList(); + for (int i = 0; i < 400; i++) { + list.add(Integer.valueOf(i)); + } + iteratorList = list; + } + List list = iteratorList; + long acc = 0; + for (int rep = 0; rep < 300; rep++) { + Iterator it = list.iterator(); + while (it.hasNext()) { + acc += ((Integer) it.next()).intValue() & 63; + } + } + return acc; + } + + /** ``length()`` / ``charAt()`` / ``substring()`` -- leaf String methods + * that signature-wide poisoning turned into suspension points. */ + private static long stringOps() { + String base = "the quick brown fox jumps over the lazy dog"; + long acc = 0; + for (int i = 0; i < 120000; i++) { + int len = base.length(); + acc += len; + acc += base.charAt(i % len); + acc += base.substring(i % 8, (i % 8) + 5).length(); + } + return acc; + } + + /** The published port-status suite reports the JS port ~190x slower than + * native here, far worse than any other workload, so keep a read on it. */ + private static long mapChurn() { + Map map = new HashMap(); + long acc = 0; + for (int rep = 0; rep < 40; rep++) { + for (int i = 0; i < 2000; i++) { + map.put(Integer.valueOf(i), Integer.valueOf(i * 3)); + } + for (int i = 0; i < 2000; i++) { + Object v = map.get(Integer.valueOf(i)); + if (v != null) { + acc += ((Integer) v).intValue() & 255; + } + // A miss as well as a hit: an open-addressed table's miss path + // is the one a hit-only benchmark cannot see. + if (map.get(Integer.valueOf(i + 100000)) != null) { + acc++; + } + } + map.clear(); + } + return acc; + } + + /** CONTROL. No calls in the loop, so nothing the dispatch work does can + * move this. If it moves, the measurement is measuring the host. */ + private static long arithControl() { + long acc = 0; + for (int i = 0; i < 3000000; i++) { + acc += (i * 31) ^ (i >> 3); + } + return acc; + } + + /** CONTROL. A ``synchronized`` block makes the callee genuinely + * suspending, so this workload must stay on the generator path however + * precise the analysis becomes. It bounds how much of any measured win + * could have come from somewhere other than dispatch. */ + private static long suspendControl() { + Counter c = new Counter(); + long acc = 0; + for (int i = 0; i < 200000; i++) { + acc += c.bump(i & 15); + } + return acc; + } + + static final class Counter { + private int total; + synchronized int bump(int by) { + total = (total + by) & 65535; + return total; + } + } +} From 67cef083326acb736b592caeda5ae9c86e2d5f7d Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Wed, 9 Sep 2026 20:22:31 +0300 Subject: [PATCH 02/11] Keep the bridge's name protections wide; narrow only the suspension seed The screenshot suite caught a real regression: nine theme tests (CheckBoxRadioTheme x4, ShowcaseTheme x4, DesktopMode) differed, and master is green on this PR's base commit, so they were mine. collectBridgeReferencedCn1Tokens has SIX consumers, not one. Besides the suspension seed it feeds minifyGeneratedIdentifiers, mangleDispatchIds, mangleInstanceFieldProps and the devirtualization exclusions in JavascriptBundleWriter and JavascriptMethodGenerator -- four of them on by default. Narrowing that shared set to "names the bridge can replace" therefore did not just relax suspension, it let the minifier RENAME names the bridge only looks up. Verified in the emitted bundle: cn1_com_codename1_ui_plaf_UIManager_getLookAndFeel_R_..., cn1_com_codename1_ui_plaf_LookAndFeel_getDefaultFormTintColor_R_int and cn1_com_codename1_ui_MenuBar_initMenuBar_com_codename1_ui_Form occur 0 times in the broken bundle and 1 time each after this commit. port.js resolves all three by literal, so the theme lookup failed and the fallback rendered differently. Those are two different questions and this commit stops conflating them. "The bridge NAMES this method" decides whether the identifier may be renamed, whether its m: entry may be pruned and whether the call may be devirtualized -- a lookup needs all of that. "The bridge REPLACES this method" decides whether callers must yield*, and only a replacement can turn out to be a generator. So collectBridgeReferencedCn1Tokens goes back to the full scrape and a new collectBridgeReplacedCn1Tokens carries the narrowing, used by the suspension analysis alone. The optimization is unaffected: yield* sites 40,741, generators 11,044 and suspending dispatch 19,073 are identical to the broken build; only 96 more dispatches take the sync helper because three names are no longer devirtualized. Two more review findings, both real: * js_throughput_report.py ASSIGNED the BENCHPROBE count instead of summing it. The runner uses one process per workload and concatenates their output, so only the last workload's count survived: a workload that reached cn1_ivsDrive with a generator would have been reported as zero and its warning suppressed, which defeats the one probe that can tell an unsound classification from a speedup. Proven non-vacuous against a synthetic log -- 4 and a warning with the fix, 0 and silence without it. * The RTA reset lived inside JavascriptReachability.run, which Parser skips entirely under -Dparparvm.js.rta.off. A reused JVM that translated one application with RTA on and a second with it off would have handed the second application's call sites the first application's type graph -- an under-approximation, i.e. the direction that picks the synchronous dispatcher for a suspending override, and a property this translator's own tests already flip mid-JVM for the minifier. The reset is now an explicit resetExportedFacts() that Parser calls before deciding whether to run RTA. vm/tests: 305 tests, 0 failures, 1 pre-existing skip. SpotBugs 0 findings. Co-Authored-By: Claude Opus 5 (1M context) --- scripts/lib/js_throughput_report.py | 7 +- .../translator/JavascriptBundleWriter.java | 79 +++++++++++++------ .../translator/JavascriptReachability.java | 24 ++++-- .../JavascriptSuspensionAnalysis.java | 5 +- .../codename1/tools/translator/Parser.java | 6 ++ 5 files changed, 91 insertions(+), 30 deletions(-) diff --git a/scripts/lib/js_throughput_report.py b/scripts/lib/js_throughput_report.py index 014f5933ec1..46ed0c10bca 100755 --- a/scripts/lib/js_throughput_report.py +++ b/scripts/lib/js_throughput_report.py @@ -38,7 +38,12 @@ def parse_raw(path): continue match = PROBE_RE.match(line) if match: - drove = int(match.group(1)) + # SUM, never assign. The runner uses one process per workload + # and concatenates their output, so an assignment keeps only + # the LAST workload's count -- an earlier workload reaching + # cn1_ivsDrive with a generator would then be reported as + # zero, which is the exact failure this probe exists to catch. + drove += int(match.group(1)) return workloads, suite, drove diff --git a/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptBundleWriter.java b/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptBundleWriter.java index 9fe8822832b..6ba3a26a9a5 100644 --- a/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptBundleWriter.java +++ b/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptBundleWriter.java @@ -1685,6 +1685,58 @@ private static String loadResource(String resourceName) throws IOException { * body would never run. */ static Set collectBridgeReferencedCn1Tokens() { + return collectBridgeCn1Tokens(false); + } + + /** + * The subset of {@link #collectBridgeReferencedCn1Tokens()} whose bodies + * the bridge can REPLACE at runtime, which is the only reason a name needs + * to be suspending. + * + * These two questions look like one and are not, and conflating them broke + * nine theme screenshots: + * + * - "the bridge NAMES this method" decides whether the emitted identifier + * may be renamed, whether its {@code m:} entry may be pruned, and + * whether the call may be devirtualized. A name the bridge merely LOOKS + * UP still needs every one of those protections, or + * {@code jvm.resolveVirtual(cls, "cn1_s_...")} stops finding it. + * - "the bridge REPLACES this method" decides whether callers must + * {@code yield*}. Only a replacement can turn out to be a generator. + * + * So the narrowing lives HERE and nowhere else. Do not push it back into + * the shared collector: five other call sites depend on the wide set, four + * of them on by default -- {@code minifyGeneratedIdentifiers}, + * {@code mangleDispatchIds}, {@code mangleInstanceFieldProps} and the + * devirtualization exclusions in this file and in + * {@link JavascriptMethodGenerator}. + */ + static Set collectBridgeReplacedCn1Tokens() { + return collectBridgeCn1Tokens(true); + } + + /** + * Scrapes {@code cn1_*} string literals out of the hand-written bridge JS + * (parparvm_runtime.js, browser_bridge.js, port.js). + * + * With {@code replacedOnly}, a token is dropped when EVERY occurrence of + * it is a {@code resolveVirtual} argument. Such a name is looked up and + * called, and the result is driven through {@code cn1_ivAdapt} / + * {@code adaptVirtualResult}, both of which tolerate a plain function; a + * REPLACEMENT instead assigns to {@code classDef.methods[...]}, and every + * such site in the bridge is scoped to one class. + * + * The test is deliberately all-or-nothing. One assignment, one + * {@code bindNative} array entry, one mention anywhere else, and the token + * stays. That keeps a name reached through a variable + * ({@code const id = "cn1_s_..."; cls.methods[id] = fn}) protected, + * because the literal feeding the variable is not itself a lookup. + * + * It is worth 10 tokens out of 705, but they are the expensive ones: + * {@code toString}, {@code equals}, {@code hashCode} and {@code run} + * between them seeded most of the bridge-referenced suspending set. + */ + private static Set collectBridgeCn1Tokens(boolean replacedOnly) { Set tokens = new HashSet(); List sources = new ArrayList(); for (String res : new String[]{ "parparvm_runtime.js", "browser_bridge.js" }) { @@ -1707,25 +1759,6 @@ static Set collectBridgeReferencedCn1Tokens() { // to the in-bundle string scan only) } java.util.regex.Pattern literal = java.util.regex.Pattern.compile("[\"'](cn1_[A-Za-z0-9_]+)[\"']"); - // A name the bridge only ever LOOKS UP is not a name the bridge - // replaces, and only replacement needs protection. - // - // ``resolveVirtual(x, "cn1_s_toString_R_java_lang_String")`` is a call; - // its result is driven through ``cn1_ivAdapt`` / ``adaptVirtualResult``, - // which tolerate a plain function. Replacement instead assigns to - // ``classDef.methods[...]``, and every such site in the bridge is - // scoped to one class. Counting a lookup as a replacement is not the - // "handful of generators" the original comment estimated: ``toString`` - // and ``equals`` are named exactly this way, and between them that - // seeded 781 methods suspending and made 2,874 dispatch sites - // ``yield*`` -- the two largest entries in the whole report. - // - // The test is deliberately all-or-nothing: a token is dropped only - // when EVERY occurrence of it is a resolveVirtual argument. One - // assignment, one bindNative array entry, one mention anywhere else, - // and it stays protected. That keeps names reached through a variable - // (``const id = "cn1_s_..."; cls.methods[id] = fn``) safe, because the - // literal that feeds the variable is not itself a lookup. java.util.regex.Pattern lookup = java.util.regex.Pattern.compile( "resolveVirtual\\s*\\([^,()]*,\\s*[\"'](cn1_[A-Za-z0-9_]+)[\"']"); Map counts = new HashMap(); @@ -1734,9 +1767,11 @@ static Set collectBridgeReferencedCn1Tokens() { while (m.find()) { bump(counts, m.group(1), 0); } - java.util.regex.Matcher l = lookup.matcher(src); - while (l.find()) { - bump(counts, l.group(1), 1); + if (replacedOnly) { + java.util.regex.Matcher l = lookup.matcher(src); + while (l.find()) { + bump(counts, l.group(1), 1); + } } } for (Map.Entry entry : counts.entrySet()) { diff --git a/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptReachability.java b/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptReachability.java index b03e0c0f9b3..1ba57a70230 100644 --- a/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptReachability.java +++ b/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptReachability.java @@ -128,12 +128,6 @@ private static final class VirtualCall { static int run(List classes, List classPool, String[] nativeSources) { - // Drop any previous translation's answer FIRST. Surefire reuses one - // JVM across fixtures and the build server translates repeatedly, so a - // stale instantiated set would let the suspension analysis resolve - // this app's call sites against another app's type graph -- which - // under-approximates, i.e. fails in the unsafe direction. - exportedInstantiated = Collections.emptySet(); JavascriptReachability rta = new JavascriptReachability(); // The conservative pass may have removed an entire class after // eliminating the only method that instantiated it. RTA can later @@ -162,6 +156,24 @@ static int run(List classes, List classPool, */ static volatile Set exportedInstantiated = Collections.emptySet(); + /** + * Forgets the previous translation's instantiated set. + * + * This CANNOT live inside {@link #run}, which is the obvious place and the + * wrong one: {@code Parser} skips {@code run} entirely under + * ``-Dparparvm.js.rta.off``, so a JVM that translated one application with + * RTA on and a second with it off would hand the second application's call + * sites the FIRST application's type graph -- and that under-approximates, + * which is the direction that picks the synchronous dispatcher for a + * suspending override. A reused JVM flipping the property mid-run is + * exactly what the translator's own tests do with the minifier properties. + * So the caller clears this unconditionally, before deciding whether to + * run RTA at all. + */ + static void resetExportedFacts() { + exportedInstantiated = Collections.emptySet(); + } + /** * The subtype relation plus RTA's instantiated set, indexed over one class * list, answering "which method bodies can this call site actually reach". diff --git a/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptSuspensionAnalysis.java b/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptSuspensionAnalysis.java index c35e979df9a..71c6c615690 100644 --- a/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptSuspensionAnalysis.java +++ b/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptSuspensionAnalysis.java @@ -331,7 +331,10 @@ private void seedDirectlySuspending(List classes) { * string-referenced name. */ private void seedBridgeReferenced(List classes) { - Set tokens = JavascriptBundleWriter.collectBridgeReferencedCn1Tokens(); + // REPLACED, not merely referenced -- see collectBridgeReplacedCn1Tokens. + // Narrowing the shared referenced-set instead renamed the names the + // bridge looks up and broke nine theme screenshots. + Set tokens = JavascriptBundleWriter.collectBridgeReplacedCn1Tokens(); if (tokens.isEmpty()) { return; } diff --git a/vm/ByteCodeTranslator/src/com/codename1/tools/translator/Parser.java b/vm/ByteCodeTranslator/src/com/codename1/tools/translator/Parser.java index bb5282ba34a..c4498711646 100644 --- a/vm/ByteCodeTranslator/src/com/codename1/tools/translator/Parser.java +++ b/vm/ByteCodeTranslator/src/com/codename1/tools/translator/Parser.java @@ -820,6 +820,12 @@ public static void writeOutput(File outputDirectory) throws Exception { // on OUTPUT_TYPE_JAVASCRIPT because the iOS runtime relies on // different dispatch mechanics and may break under stricter // reachability. + // Unconditionally, and BEFORE the RTA decision: the suspension + // analysis below reads what RTA published, and skipping RTA must + // mean "no information", not "the previous application's answer". + if (ByteCodeTranslator.output == ByteCodeTranslator.OutputType.OUTPUT_TYPE_JAVASCRIPT) { + JavascriptReachability.resetExportedFacts(); + } if (BytecodeMethod.optimizerOn && ByteCodeTranslator.output == ByteCodeTranslator.OutputType.OUTPUT_TYPE_JAVASCRIPT && System.getProperty("parparvm.js.rta.off") == null) { From fb87c38f6acb53b513513823103d6e15e788e532 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Wed, 9 Sep 2026 21:42:10 +0300 Subject: [PATCH 03/11] Two unsoundness bugs the screenshot suite found, both mine The suite went from nine theme mismatches to zero, and then failed on two test FAILURES instead. Both are the named errors this work added on purpose, firing exactly as intended rather than corrupting silently. CryptoApiTest: "cn1_ivs: sync virtual dispatch reached a yielding method (CHA unsound): cn1_s_aesEncrypt_...". collectSyncNativeTokens decides whether a bindNative wrapper is a plain function (synchronous native) or a function* (suspending) by searching FORWARD from the name array for the next occurrence of "function". When the wrapper is built by a factory -- bindNative([...aesEncrypt...], cn1CryptoAesBinding("aesEncrypt")); -- there is no function keyword in the call at all, so the search ran past the end of it and matched the "function cn1CryptoRsaBinding(op)" DECLARATION several lines below. aesEncrypt was classified a synchronous native; it is a generator. The wrapper is now required to be the literal argument that follows the array, and anything else -- a factory call, an identifier, an arrow -- is left out of the sync set, i.e. stays suspending, which is the safe direction. 315 genuine sync natives still classify as such. This bug predates the branch. It was unreachable because the signature-wide bridge seed made every one of those dispatches suspending regardless; resolving call sites against their receiver removed that cover. BytecodeTranslatorRegressionTest: "yield* (intermediate value)(intermediate value) is not iterable" -- the mirror image. A devirtualized call site names its single implementation directly through _dv*/_dw*, but took its suspending flag from isInvokeSuspending, which answers for the SIGNATURE. Those used to be the same thing. Once a call site resolves against its receiver they are not: a signature that suspends somewhere else in the program can devirtualize here to a plain function, and _dv* then put yield* in front of it. computeMonomorphicDispatch now records whether each devirtualized target is itself suspending and both emission sites follow the target. Verified in the emitted bundle: aesEncrypt was "function ...aesEncrypt..." with a _wN call site and is now "function* ...aesEncrypt..." with a _vN one. The optimization is intact -- yield* 40,753, generators 11,054, suspending dispatch 19,075, against master's 54,549 / 13,068 / 28,569. vm/tests: 305 tests, 0 failures, 1 pre-existing skip. SpotBugs 0 findings. The throughput benchmark's sync-dispatcher probe reads 0. Co-Authored-By: Claude Opus 5 (1M context) --- .../translator/JavascriptBundleWriter.java | 26 ++++++++++- .../translator/JavascriptMethodGenerator.java | 43 ++++++++++++++++++- 2 files changed, 65 insertions(+), 4 deletions(-) diff --git a/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptBundleWriter.java b/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptBundleWriter.java index 6ba3a26a9a5..726b82a74e2 100644 --- a/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptBundleWriter.java +++ b/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptBundleWriter.java @@ -1838,8 +1838,30 @@ static Set collectSyncNativeTokens() { if (close < 0) { continue; } - int fn = src.indexOf("function", close); // the wrapper keyword - if (fn < 0) { + // The wrapper must be the LITERAL argument that follows the + // name array. Searching forward for the next ``function`` + // walks straight past the end of the bindNative call when the + // wrapper is built by a factory -- + // ``bindNative([...], cn1CryptoAesBinding("aesEncrypt"))`` + // matched the ``function cn1CryptoRsaBinding(op)`` DECLARATION + // several lines below and classified aesEncrypt as a + // synchronous native. It is a generator, so callers took the + // sync dispatcher and the runtime raised + // ``cn1_ivs: ... (CHA unsound)``. Anything that is not a + // literal function expression here is left OUT of the sync + // set, i.e. stays suspending, which is the safe direction. + int fn = close + 1; + while (fn < src.length() && Character.isWhitespace(src.charAt(fn))) { + fn++; + } + if (fn >= src.length() || src.charAt(fn) != ',') { + continue; + } + fn++; + while (fn < src.length() && Character.isWhitespace(src.charAt(fn))) { + fn++; + } + if (!src.startsWith("function", fn)) { continue; } int k = fn + "function".length(); diff --git a/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptMethodGenerator.java b/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptMethodGenerator.java index 1ad0c83e153..f8f484a2c22 100644 --- a/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptMethodGenerator.java +++ b/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptMethodGenerator.java @@ -126,6 +126,17 @@ final class JavascriptMethodGenerator { // win. JSO-bridge types are excluded (they dispatch host-side via // the m: map). Kill switch: ``-Dparparvm.js.devirt.off``. private static volatile java.util.Map monomorphicDispatch = null; + // Dispatch id -> whether the SINGLE implementation ``monomorphicDispatch`` + // resolves it to is suspending. + // + // A devirtualized call site names its target directly, so its ``yield*`` + // must follow that target and NOT the signature-wide answer. Those two + // used to be the same thing; once a call site is resolved against its + // receiver they are not, and a signature that is suspending SOMEWHERE can + // devirtualize here to a plain function -- which made ``_dv*`` do + // ``yield*`` on a non-generator and throw "is not iterable" out of + // BytecodeTranslatorRegressionTest. + private static volatile java.util.Map monomorphicSuspending = null; // The class whose method is currently being emitted. Used by // ``appendInterpreterEnsureClassInitialized`` to elide // ``_I("X")`` when ``X`` is the containing class or one of @@ -187,6 +198,7 @@ static void setClassIndex(List allClasses) { // Monomorphic-devirtualization map must be computed BEFORE the // dispatch-reference scan so that scan can skip ids that will be // devirtualized (and thus need no m: entry / dispatch-id string). + monomorphicSuspending = new java.util.HashMap(); java.util.Map monoDispatch = computeMonomorphicDispatch(allClasses, index); monomorphicDispatch = monoDispatch; @@ -291,6 +303,7 @@ static void setClassIndex(List allClasses) { */ private static java.util.Map computeMonomorphicDispatch( List allClasses, Map index) { + java.util.Map suspendingByDispatchId = monomorphicSuspending; java.util.Map result = new java.util.HashMap(); if (System.getProperty("parparvm.js.devirt.off") != null) { return result; @@ -341,6 +354,7 @@ private static java.util.Map computeMonomorphicDispatch( Integer total = declCount.get(did); if (total != null && total == 1) { result.put(did, jsMethodIdentifier(c, m)); + suspendingByDispatchId.put(did, Boolean.valueOf(m.isJavascriptSuspending())); } } } @@ -611,6 +625,23 @@ private static boolean emitsMonitor(BytecodeMethod method) { * {@link BytecodeMethod#isJavascriptSuspending} flag itself * defaults to {@code true} for the same reason. */ + /** + * Whether a call site that has been DEVIRTUALIZED to {@code dispatchId} + * must be emitted suspending. + * + * Follows the single implementation rather than the signature: ``_dv*`` + * calls that implementation directly, so a signature-wide "suspending" + * would put ``yield*`` in front of a plain function. + */ + private static boolean isDevirtualizedInvokeSuspending(String dispatchId, boolean signatureAnswer) { + java.util.Map known = monomorphicSuspending; + if (known == null) { + return signatureAnswer; + } + Boolean target = known.get(dispatchId); + return target == null ? signatureAnswer : target.booleanValue(); + } + private static boolean isInvokeSuspending(Invoke invoke) { int op = invoke.getOpcode(); if (op == Opcodes.INVOKEVIRTUAL || op == Opcodes.INVOKEINTERFACE) { @@ -5130,10 +5161,13 @@ private static boolean appendStraightLineInvokeInstruction(StringBuilder out, In // Monomorphic devirtualization (see appendCompactVirtualDispatch): // direct ``_dv*`` / ``_dw*`` call to the single impl when known. String monoImpl = monomorphicDispatch == null ? null : monomorphicDispatch.get(dispatchId); - String devBase = monoImpl != null ? (suspending ? "_dv" : "_dw") : (suspending ? "_v" : "_w"); + // A devirtualized site follows its TARGET, not the signature. + boolean devSuspending = monoImpl != null + ? isDevirtualizedInvokeSuspending(dispatchId, suspending) : suspending; + String devBase = monoImpl != null ? (devSuspending ? "_dv" : "_dw") : (suspending ? "_v" : "_w"); String devSecond = monoImpl != null ? monoImpl : ("\"" + dispatchId + "\""); StringBuilder callExpr = new StringBuilder(); - callExpr.append(suspending ? "(yield* " : "(").append(devBase) + callExpr.append(devSuspending ? "(yield* " : "(").append(devBase) .append(argValues.length <= 4 ? String.valueOf(argValues.length) : "N") .append("(").append(target).append(", ").append(devSecond); if (argValues.length > 4) { @@ -7283,6 +7317,11 @@ private static void appendCompactVirtualDispatch(StringBuilder out, String inden // (bareword) impl function id, not a quoted dispatch id. java.util.Map mono = monomorphicDispatch; String monoImpl = mono == null ? null : mono.get(methodId); + // Devirtualized: follow the TARGET's suspending-ness, not the + // signature's -- see isDevirtualizedInvokeSuspending. + if (monoImpl != null) { + suspending = isDevirtualizedInvokeSuspending(methodId, suspending); + } String base = monoImpl != null ? (suspending ? "_dv" : "_dw") : (suspending ? "_v" : "_w"); // The second helper argument: bareword impl fn for devirt, else // the quoted dispatch-id string. From 873a2d13b377b6892885040164ec37be8dbce2de Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Wed, 9 Sep 2026 22:17:01 +0300 Subject: [PATCH 04/11] Classify a bindNative wrapper accurately, in both directions The previous commit made an unparseable wrapper "suspending", reasoning that was the safe direction. It is not: getting this wrong is unsound BOTH ways. Call a generator synchronously and the runtime raises "cn1_ivs ... (CHA unsound)"; put yield* in front of a plain function and it raises "is not iterable". Being conservative in one direction just moves the failure, and it did -- CryptoApiTest went green and DatabaseEncryptionTest went red, because SQLiteNative.isCipherAvailable documents itself in a comment between its ``],`` and its plain ``function``, so "must literally start with function" read it as unknown and made it suspending. So the wrapper is now classified properly: skip whitespace AND comments after the name array; a literal function/function* decides directly; a factory call (``cn1CryptoAesBinding("aesEncrypt")``) is resolved one level by finding that function's declaration and its first ``return function``. Deeper indirection is still unknown, and unknown still means suspending -- but nothing in the bridge is deeper than one level. All six non-literal wrappers in the bridge JS now classify correctly; 317 sync natives, with isCipherAvailable in and the four crypto bindings out. Verified directly against the collector: isCipherAvailable sync=true, aesEncrypt and rsaEncrypt sync=false. vm/tests: 305 tests, 0 failures. SpotBugs 0 findings. Co-Authored-By: Claude Opus 5 (1M context) --- .../translator/JavascriptBundleWriter.java | 121 +++++++++++++----- 1 file changed, 88 insertions(+), 33 deletions(-) diff --git a/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptBundleWriter.java b/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptBundleWriter.java index 726b82a74e2..eb7666a1918 100644 --- a/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptBundleWriter.java +++ b/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptBundleWriter.java @@ -1783,6 +1783,76 @@ private static Set collectBridgeCn1Tokens(boolean replacedOnly) { return tokens; } + /** + * Classifies the wrapper argument of a {@code bindNative([...], WRAPPER)} + * call whose name array ends at {@code close}. + * + * Returns TRUE for a {@code function*}, FALSE for a plain {@code + * function}, and null when it cannot tell -- which the caller must treat + * as "leave suspending", the direction virtual dispatch tolerates. + * + * One level of indirection is resolved, because the crypto bindings pass a + * factory result ({@code cn1CryptoAesBinding("aesEncrypt")}) rather than a + * literal: the named function is located and its first {@code return + * function} decides. Deeper indirection is deliberately not chased. + */ + private static Boolean classifyBindNativeWrapper(String src, int close) { + int i = skipSpaceAndComments(src, close + 1); + if (i >= src.length() || src.charAt(i) != ',') { + return null; + } + i = skipSpaceAndComments(src, i + 1); + if (src.startsWith("function", i)) { + return Boolean.valueOf(isGeneratorAt(src, i)); + } + // ``ident(`` -- a factory. Resolve its declaration once. + int j = i; + while (j < src.length() && (Character.isLetterOrDigit(src.charAt(j)) || src.charAt(j) == '_' + || src.charAt(j) == '$')) { + j++; + } + if (j == i || j >= src.length() || src.charAt(j) != '(') { + return null; + } + int decl = src.indexOf("function " + src.substring(i, j) + "("); + if (decl < 0) { + return null; + } + int ret = src.indexOf("return function", decl); + if (ret < 0) { + return null; + } + return Boolean.valueOf(isGeneratorAt(src, ret + "return ".length())); + } + + /** True when the {@code function} keyword at {@code i} is a generator. */ + private static boolean isGeneratorAt(String src, int i) { + int k = i + "function".length(); + while (k < src.length() && Character.isWhitespace(src.charAt(k))) { + k++; + } + return k < src.length() && src.charAt(k) == '*'; + } + + /** Advances past JS whitespace, {@code //} and block comments. */ + private static int skipSpaceAndComments(String src, int i) { + while (i < src.length()) { + char c = src.charAt(i); + if (Character.isWhitespace(c)) { + i++; + } else if (src.startsWith("//", i)) { + int nl = src.indexOf('\n', i); + i = nl < 0 ? src.length() : nl + 1; + } else if (src.startsWith("/*", i)) { + int endC = src.indexOf("*/", i); + i = endC < 0 ? src.length() : endC + 2; + } else { + return i; + } + } + return i; + } + private static void bump(Map counts, String token, int slot) { int[] seen = counts.get(token); if (seen == null) { @@ -1838,39 +1908,24 @@ static Set collectSyncNativeTokens() { if (close < 0) { continue; } - // The wrapper must be the LITERAL argument that follows the - // name array. Searching forward for the next ``function`` - // walks straight past the end of the bindNative call when the - // wrapper is built by a factory -- - // ``bindNative([...], cn1CryptoAesBinding("aesEncrypt"))`` - // matched the ``function cn1CryptoRsaBinding(op)`` DECLARATION - // several lines below and classified aesEncrypt as a - // synchronous native. It is a generator, so callers took the - // sync dispatcher and the runtime raised - // ``cn1_ivs: ... (CHA unsound)``. Anything that is not a - // literal function expression here is left OUT of the sync - // set, i.e. stays suspending, which is the safe direction. - int fn = close + 1; - while (fn < src.length() && Character.isWhitespace(src.charAt(fn))) { - fn++; - } - if (fn >= src.length() || src.charAt(fn) != ',') { - continue; - } - fn++; - while (fn < src.length() && Character.isWhitespace(src.charAt(fn))) { - fn++; - } - if (!src.startsWith("function", fn)) { - continue; - } - int k = fn + "function".length(); - while (k < src.length() && Character.isWhitespace(src.charAt(k))) { - k++; - } - boolean generator = k < src.length() && src.charAt(k) == '*'; - if (generator) { - continue; // function* -> suspending, leave seeded + // Classify the wrapper that is the ARGUMENT of this call. The + // original scan took the next ``function`` anywhere after the + // ``]``, which walks past the end of the bindNative call and + // reads an unrelated declaration -- it classified aesEncrypt + // (wrapper ``cn1CryptoAesBinding("aesEncrypt")``, a generator) + // as a synchronous native by matching ``function + // cn1CryptoRsaBinding(op)`` several lines below. + // + // Getting this wrong is unsound in BOTH directions, so it has + // to be accurate rather than conservative: call a generator + // synchronously and the runtime raises ``cn1_ivs ... (CHA + // unsound)``; ``yield*`` a plain function and it raises "is + // not iterable". Skipping comments matters for the same + // reason -- SQLiteNative.isCipherAvailable documents itself + // between the ``],`` and its plain ``function``. + Boolean generatorWrapper = classifyBindNativeWrapper(src, close); + if (generatorWrapper == null || generatorWrapper.booleanValue()) { + continue; // unknown, or a generator -> leave it suspending } java.util.regex.Matcher lit = literal.matcher(src.substring(bracket + 1, close)); while (lit.find()) { From 464aac5ad2b5e4dc65953f2966152de683ab2009 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Wed, 9 Sep 2026 22:18:01 +0300 Subject: [PATCH 05/11] Three-state wrapper kind instead of a nullable Boolean SpotBugs NP_BOOLEAN_RETURN_NULL, and it is right beyond the auto-unboxing risk: "cannot tell" is a real answer here and must not be expressible as something a caller can confuse with true or false. SpotBugs 0 findings; the classifier still answers isCipherAvailable sync and the four crypto bindings suspending. Co-Authored-By: Claude Opus 5 (1M context) --- .../translator/JavascriptBundleWriter.java | 33 ++++++++++++------- 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptBundleWriter.java b/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptBundleWriter.java index eb7666a1918..40bf4bf5d86 100644 --- a/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptBundleWriter.java +++ b/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptBundleWriter.java @@ -1787,23 +1787,22 @@ private static Set collectBridgeCn1Tokens(boolean replacedOnly) { * Classifies the wrapper argument of a {@code bindNative([...], WRAPPER)} * call whose name array ends at {@code close}. * - * Returns TRUE for a {@code function*}, FALSE for a plain {@code - * function}, and null when it cannot tell -- which the caller must treat - * as "leave suspending", the direction virtual dispatch tolerates. + * {@link WrapperKind#UNKNOWN} means it cannot tell, which the caller must + * treat as "leave suspending". * * One level of indirection is resolved, because the crypto bindings pass a * factory result ({@code cn1CryptoAesBinding("aesEncrypt")}) rather than a * literal: the named function is located and its first {@code return * function} decides. Deeper indirection is deliberately not chased. */ - private static Boolean classifyBindNativeWrapper(String src, int close) { + private static WrapperKind classifyBindNativeWrapper(String src, int close) { int i = skipSpaceAndComments(src, close + 1); if (i >= src.length() || src.charAt(i) != ',') { - return null; + return WrapperKind.UNKNOWN; } i = skipSpaceAndComments(src, i + 1); if (src.startsWith("function", i)) { - return Boolean.valueOf(isGeneratorAt(src, i)); + return kindAt(src, i); } // ``ident(`` -- a factory. Resolve its declaration once. int j = i; @@ -1812,19 +1811,30 @@ private static Boolean classifyBindNativeWrapper(String src, int close) { j++; } if (j == i || j >= src.length() || src.charAt(j) != '(') { - return null; + return WrapperKind.UNKNOWN; } int decl = src.indexOf("function " + src.substring(i, j) + "("); if (decl < 0) { - return null; + return WrapperKind.UNKNOWN; } int ret = src.indexOf("return function", decl); if (ret < 0) { - return null; + return WrapperKind.UNKNOWN; } - return Boolean.valueOf(isGeneratorAt(src, ret + "return ".length())); + return kindAt(src, ret + "return ".length()); } + /** What kind of wrapper the {@code function} keyword at {@code i} opens. */ + private static WrapperKind kindAt(String src, int i) { + return isGeneratorAt(src, i) ? WrapperKind.GENERATOR : WrapperKind.PLAIN; + } + + /** + * Three states, not a nullable Boolean: "cannot tell" is a real answer + * here and must not be confused with either of the other two. + */ + private enum WrapperKind { GENERATOR, PLAIN, UNKNOWN } + /** True when the {@code function} keyword at {@code i} is a generator. */ private static boolean isGeneratorAt(String src, int i) { int k = i + "function".length(); @@ -1923,8 +1933,7 @@ static Set collectSyncNativeTokens() { // not iterable". Skipping comments matters for the same // reason -- SQLiteNative.isCipherAvailable documents itself // between the ``],`` and its plain ``function``. - Boolean generatorWrapper = classifyBindNativeWrapper(src, close); - if (generatorWrapper == null || generatorWrapper.booleanValue()) { + if (classifyBindNativeWrapper(src, close) != WrapperKind.PLAIN) { continue; // unknown, or a generator -> leave it suspending } java.util.regex.Matcher lit = literal.matcher(src.substring(bracket + 1, close)); From 0eb8b82c70e6ed400a6223368d1c5cc1412c5acf Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Wed, 9 Sep 2026 23:15:25 +0300 Subject: [PATCH 06/11] Resolve a super call that lands on an interface default method Last failure in the JS suite: BytecodeTranslatorRegressionTest, "yield* (intermediate value)(intermediate value) is not iterable". GameViewer.drawToken calls super.drawToken(...). Its superclass chain -- PanelCanvas, CommonCanvas, ProxyCanvas, ShellComponent -- declares no drawToken at all; the body is the TokenMenuHost interface DEFAULT method, and the JVM (like the runtime's resolveVirtual) finds it by searching interfaces after the extends chain. Both of this backend's direct-invoke resolvers walked only getBaseClass(), so INVOKESPECIAL resolved to null -- and a null direct target is treated as suspending. The emitter therefore wrote yield* in front of a default method that is emitted as a plain function. That was harmless for as long as every default method was suspending anyway, which is what the signature-wide seed guaranteed. Resolving call sites against their receiver removed it, so the mismatch became live. Both resolvers now search the interface hierarchy breadth-first after the superclass walk, in the same order the runtime does, so the emitter and the analysis agree on what the call actually reaches. Also: the benchmark runner refused to run a source with no run("...") workloads, which is exactly the shape of a one-off probe. It now runs such a source once, unfiltered -- reproducing a translator bug in six seconds rather than a forty-seven-minute CI cycle is most of what that harness is for. vm/tests: 305 tests, 0 failures. SpotBugs 0 findings. Co-Authored-By: Claude Opus 5 (1M context) --- .../run-javascript-throughput-benchmark.sh | 13 +++- .../translator/JavascriptMethodGenerator.java | 64 +++++++++++++++++++ .../JavascriptSuspensionAnalysis.java | 52 +++++++++++++++ 3 files changed, 127 insertions(+), 2 deletions(-) diff --git a/scripts/run-javascript-throughput-benchmark.sh b/scripts/run-javascript-throughput-benchmark.sh index 37a60d002b7..d6e378aaf43 100755 --- a/scripts/run-javascript-throughput-benchmark.sh +++ b/scripts/run-javascript-throughput-benchmark.sh @@ -199,16 +199,25 @@ RAW="$WORK_DIR/raw.txt" # One process per workload. Sharing a process makes every measurement depend # on what ran before it, which reports phantom regressions in unchanged code. WORKLOADS="$(grep -oE 'run\("[A-Za-z]+"' "$BENCH_SRC" | sed 's/run("//;s/"//' | sort -u)" -if [ -z "$WORKLOADS" ]; then bench_log "could not read the workload list from $BENCH_SRC"; exit 2; fi +# A source with no run("...") calls is a PROBE, not the benchmark -- run it once +# unfiltered rather than refusing. Reproducing a translator bug in six seconds +# instead of a forty-seven-minute CI cycle is most of what this harness is for. +if [ -z "$WORKLOADS" ]; then + bench_log "no run(\"...\") workloads found; running $APP_NAME once, unfiltered" + WORKLOADS="__all__" +fi for workload in $WORKLOADS; do - if ! "$NODE_BIN" "$WORK_DIR/harness.js" "$DIST" "$workload" "$APP_NAME" \ + filter="$workload" + [ "$filter" = "__all__" ] && filter="" + if ! "$NODE_BIN" "$WORK_DIR/harness.js" "$DIST" "$filter" "$APP_NAME" \ >> "$RAW" 2>>"$WORK_DIR/stderr.txt"; then bench_log "workload $workload failed" sed -n '1,40p' "$WORK_DIR/stderr.txt" >&2 exit 1 fi done +cat "$RAW" >&2 if ! grep -q '^BENCHSUITE ' "$RAW"; then bench_log "benchmark did not reach its final marker -- output follows" diff --git a/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptMethodGenerator.java b/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptMethodGenerator.java index f8f484a2c22..b2c8056ff12 100644 --- a/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptMethodGenerator.java +++ b/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptMethodGenerator.java @@ -596,6 +596,70 @@ private static BytecodeMethod resolveDirectInvokeTarget(Invoke invoke) { String base = cls.getBaseClass(); current = base == null ? null : JavascriptNameUtil.sanitizeClassName(base); } + // A ``super`` call can land on an INTERFACE DEFAULT METHOD: the + // superclass chain declares nothing, and the JVM (like the runtime's + // resolveVirtual) then searches the interfaces. Walking only the + // extends chain returned null here, and a null direct target is + // treated as suspending -- so the emitter wrote ``yield*`` in front of + // a default method that is emitted as a plain function, which is + // exactly "yield* ... is not iterable". Harmless while every default + // was suspending anyway; live the moment call sites resolve against + // their receiver. + BytecodeMethod viaInterface = resolveThroughInterfaces(idx, + JavascriptNameUtil.sanitizeClassName(invoke.getOwner()), normalizedName, desc); + if (viaInterface != null) { + return viaInterface; + } + return null; + } + + /** + * Breadth-first search of the interface hierarchy above {@code owner} for + * a concrete (default) {@code name + desc}, mirroring the order + * {@code jvm.resolveVirtual} uses: superclasses first, then interfaces. + */ + private static BytecodeMethod resolveThroughInterfaces(Map idx, + String owner, String name, String desc) { + java.util.ArrayDeque pending = new java.util.ArrayDeque(); + java.util.HashSet seen = new java.util.HashSet(); + String current = owner; + while (current != null && seen.add(current)) { + ByteCodeClass cls = idx.get(current); + if (cls == null) { + break; + } + if (cls.getBaseInterfaces() != null) { + for (String iface : cls.getBaseInterfaces()) { + pending.add(JavascriptNameUtil.sanitizeClassName(iface)); + } + } + String base = cls.getBaseClass(); + current = base == null ? null : JavascriptNameUtil.sanitizeClassName(base); + } + java.util.HashSet visitedIfaces = new java.util.HashSet(); + while (!pending.isEmpty()) { + String ifaceName = pending.poll(); + if (ifaceName == null || !visitedIfaces.add(ifaceName)) { + continue; + } + ByteCodeClass iface = idx.get(ifaceName); + if (iface == null) { + continue; + } + for (BytecodeMethod m : iface.getMethods()) { + if (m.isEliminated() || m.isAbstract()) { + continue; + } + if (name.equals(m.getMethodName()) && desc.equals(m.getSignature())) { + return m; + } + } + if (iface.getBaseInterfaces() != null) { + for (String up : iface.getBaseInterfaces()) { + pending.add(JavascriptNameUtil.sanitizeClassName(up)); + } + } + } return null; } diff --git a/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptSuspensionAnalysis.java b/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptSuspensionAnalysis.java index 71c6c615690..800f54c8061 100644 --- a/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptSuspensionAnalysis.java +++ b/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptSuspensionAnalysis.java @@ -678,6 +678,58 @@ private BytecodeMethod resolveTarget(String owner, String name, String desc) { String base = cls.getBaseClass(); clsName = base == null ? null : JavascriptNameUtil.sanitizeClassName(base); } + // Same interface-default case the emitter handles: a ``super`` call + // whose superclass chain declares nothing resolves to an interface + // DEFAULT method. Returning null here would make the caller treat the + // site as suspending while the default itself is classified sync -- + // the two sides must agree, so search the interfaces exactly as the + // emitter and the runtime's resolveVirtual do. + return resolveThroughInterfaces(JavascriptNameUtil.sanitizeClassName(owner), + normalizedName, desc); + } + + /** Breadth-first interface search; superclasses have already been tried. */ + private BytecodeMethod resolveThroughInterfaces(String owner, String name, String desc) { + java.util.ArrayDeque pending = new java.util.ArrayDeque(); + java.util.HashSet seen = new java.util.HashSet(); + String current = owner; + while (current != null && seen.add(current)) { + ByteCodeClass cls = byName.get(current); + if (cls == null) { + break; + } + if (cls.getBaseInterfaces() != null) { + for (String iface : cls.getBaseInterfaces()) { + pending.add(JavascriptNameUtil.sanitizeClassName(iface)); + } + } + String base = cls.getBaseClass(); + current = base == null ? null : JavascriptNameUtil.sanitizeClassName(base); + } + java.util.HashSet visited = new java.util.HashSet(); + while (!pending.isEmpty()) { + String ifaceName = pending.poll(); + if (ifaceName == null || !visited.add(ifaceName)) { + continue; + } + ByteCodeClass iface = byName.get(ifaceName); + if (iface == null) { + continue; + } + for (BytecodeMethod m : iface.getMethods()) { + if (m.isEliminated() || m.isAbstract()) { + continue; + } + if (name.equals(m.getMethodName()) && desc.equals(m.getSignature())) { + return m; + } + } + if (iface.getBaseInterfaces() != null) { + for (String up : iface.getBaseInterfaces()) { + pending.add(JavascriptNameUtil.sanitizeClassName(up)); + } + } + } return null; } From 0f84c06041927b563fff80bf53aed389a9f4f9bf Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Thu, 10 Sep 2026 10:13:51 +0300 Subject: [PATCH 07/11] Six review findings, and the resolver disagreement behind the last CI failure All six are real; two made claims about RTA internals that I checked in the code before accepting. * The suspension report counted dispatch sites only from ``sigCallersOf``, so a receiver-resolved site -- the default path -- was never counted. That is what made the reported total collapse from 37,395 to 14,192 between two runs: an artefact of where the edge is recorded, not a reduction in emitted call sites. Counted at the instruction now, before the branch, and the section header says what the number does and does not mean. * js_throughput_report.py printed a warning and exited 0 when the sync dispatcher met a generator. A gate that cannot fail is not a gate, and the runner documents exit 1 as "benchmark failure" -- which is exactly what an unsound classification absorbed by the runtime is. It writes the JSON first so the evidence survives, then fails. * The comparison iterated only the workloads present in the NEW run, so a workload the baseline had and this run lost was silently skipped and the script exited 0. An incomplete suite could be read as a speedup. * markClassInstantiated() adds a name before it looks at what kind of type it is and then walks the supertype chain, so the exported set routinely contains abstract bases and interfaces. Treating those as receivers made walkUp() return null for a type that can never BE a receiver, which abandoned the whole query and dropped the call site back to the signature-wide answer. Concrete receivers only. * A concrete receiver can inherit a Java 8 default method without overriding it; walkUp() searched only the extends chain and returned null, so the receiver-specific answer was discarded. It now searches interfaces the way JavascriptReachability.enqueueInterfaceDefault and the runtime's resolveVirtual both do. * ``grep`` returns 1 on no match, and with set -e and pipefail that killed the benchmark runner at the assignment -- so the probe fallback below it was unreachable and a probe source produced no output at all. That is why the local reproduction of the CI failure appeared to do nothing. The last one unblocked the real bug. With a working six-second reproduction: the emitter and the analysis each resolved direct invokes with their OWN walker, and for a ``super`` call landing on an interface default they disagreed -- the call site said suspending, the method containing it did not, so a ``yield*`` was emitted inside a plain ``function``. That is ``ReferenceError: yield is not defined``, which is what BytecodeTranslatorRegressionTest was reporting. There is now one resolver: DispatchModel.isDirectSuspending answers direct call sites using the same resolveTarget that builds the propagation edges, so the two cannot disagree. Note master reproduces this too on the same fixture; the signature-wide seed was hiding it in the real app. resolveTarget is memoised. It is called once per direct invoke by the analysis and again per direct invoke by the emitter, and once it gained an interface walk on the miss path JavascriptTargetIntegrationTest went from 48s to 378s. With the cache it is 50s. Verified on the hellocodenameone bundle: zero plain functions containing a yield, across 12,207 emitted functions (checking this needs string literals stripped first -- a naive brace match walks straight past ``_L("AdError{code=")`` and reports a false positive). The bundle boots and passes every lifecycle milestone. yield* 40,656, generators 11,007, suspending dispatch 19,038, synchronous methods 9,901, against master's 54,549 / 13,068 / 28,569 / 8,456. vm/tests: 305 tests, 0 failures, 1 pre-existing skip. SpotBugs 0 findings. Co-Authored-By: Claude Opus 5 (1M context) --- scripts/lib/js_throughput_report.py | 28 ++++-- .../run-javascript-throughput-benchmark.sh | 6 +- .../translator/JavascriptMethodGenerator.java | 10 +++ .../translator/JavascriptReachability.java | 76 ++++++++++++++++- .../JavascriptSuspensionAnalysis.java | 85 ++++++++++++++++--- 5 files changed, 184 insertions(+), 21 deletions(-) diff --git a/scripts/lib/js_throughput_report.py b/scripts/lib/js_throughput_report.py index 46ed0c10bca..e82ca23fdc4 100755 --- a/scripts/lib/js_throughput_report.py +++ b/scripts/lib/js_throughput_report.py @@ -96,6 +96,11 @@ def main(): else: print("%-*s %12s %12s %9s" % (width, "workload", "base ms", "new ms", "change")) mismatched = [] + # A workload the baseline had and this run does not is not a workload + # to skip: either a selected process died before emitting its BENCH + # line or the suite shrank, and both mean this run did less work than + # the thing it is being compared against. + vanished = sorted(set(baseline.get("workloads", {})) - set(workloads)) for name in sorted(workloads): new = workloads[name] old = baseline.get("workloads", {}).get(name) @@ -120,22 +125,33 @@ def main(): if old: print("%-*s %12d %12d %8.1f%%" % ( width, label, old, new, (new - old) / float(old) * 100.0)) + if vanished: + print("\nREFUSING the comparison: %s present in the baseline and absent here." + "\nAn incomplete suite cannot be read as a speedup." + % ", ".join(vanished), file=sys.stderr) + return 1 if mismatched: print("\nREFUSING the comparison: checksum changed for %s." "\nA workload that computes something different cannot be compared on time." % ", ".join(mismatched), file=sys.stderr) return 1 - if drove: - print("\nWARNING: the sync virtual dispatcher met a generator %d time(s)." - "\nThe analysis classified a signature synchronous that is not; the runtime" - "\nabsorbed it by stepping the generator once. Fix the classification --" - "\ndo not read the timings as a clean result." % drove, file=sys.stderr) - if os.environ.get("JSON_OUT"): with open(os.environ["JSON_OUT"], "w", encoding="utf-8") as handle: json.dump(result, handle, indent=2, sort_keys=True) handle.write("\n") + + if drove: + # A diagnostic that only warns is not a gate. The runner documents + # exit 1 as "benchmark failure", and a run in which the sync + # dispatcher met a generator IS one: the classification is wrong and + # the runtime absorbed it, which shows up as a speedup rather than as + # the bug it is. Written to JSON first, so the evidence survives. + print("\nFAILING: the sync virtual dispatcher met a generator %d time(s)." + "\nThe analysis classified a signature synchronous that is not; the runtime" + "\nabsorbed it by stepping the generator once. Fix the classification --" + "\nthese timings are not a clean result." % drove, file=sys.stderr) + return 1 return 0 diff --git a/scripts/run-javascript-throughput-benchmark.sh b/scripts/run-javascript-throughput-benchmark.sh index d6e378aaf43..26c1e4b688e 100755 --- a/scripts/run-javascript-throughput-benchmark.sh +++ b/scripts/run-javascript-throughput-benchmark.sh @@ -88,6 +88,7 @@ bench_log "translating to JavaScript" # than a broken lookup. It is also irrelevant to what is being measured here: # this benchmark prices call dispatch, not identifier length. "$JAVA_BIN" -cp "$COMPILER_JAR" \ + ${CN1_TRANSLATOR_OPTS:-} \ -Dparparvm.js.minify.idents.off=1 \ -Dparparvm.js.alias.off=1 \ -Dcodename1.javascriptport.webapp="$REPO_ROOT/Ports/JavaScriptPort/src/main/webapp" \ @@ -198,7 +199,10 @@ RAW="$WORK_DIR/raw.txt" # One process per workload. Sharing a process makes every measurement depend # on what ran before it, which reports phantom regressions in unchanged code. -WORKLOADS="$(grep -oE 'run\("[A-Za-z]+"' "$BENCH_SRC" | sed 's/run("//;s/"//' | sort -u)" +# ``|| true`` is load bearing: with set -e and pipefail, grep's exit 1 on no +# match kills the script right here, so the probe fallback below was +# unreachable and a probe source produced no output at all. +WORKLOADS="$(grep -oE 'run\("[A-Za-z]+"' "$BENCH_SRC" | sed 's/run("//;s/"//' | sort -u || true)" # A source with no run("...") calls is a PROBE, not the benchmark -- run it once # unfiltered rather than refusing. Reproducing a translator bug in six seconds # instead of a forty-seven-minute CI cycle is most of what this harness is for. diff --git a/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptMethodGenerator.java b/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptMethodGenerator.java index b2c8056ff12..7458c1aa6b1 100644 --- a/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptMethodGenerator.java +++ b/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptMethodGenerator.java @@ -734,6 +734,16 @@ private static boolean isInvokeSuspending(Invoke invoke) { String sig = invoke.getName() + invoke.getDesc(); return suspendingSigs.contains(sig); } + // Direct calls go through the analysis's resolver too. Resolving them + // here independently is what let a ``super`` call to an interface + // default be "suspending" at the call site and synchronous in the + // method containing it -- a ``yield*`` inside a plain ``function``, + // i.e. ``ReferenceError: yield is not defined``. + JavascriptSuspensionAnalysis.DispatchModel direct = + JavascriptSuspensionAnalysis.exportedDispatchModel; + if (direct != null) { + return direct.isDirectSuspending(invoke.getOwner(), invoke.getName(), invoke.getDesc()); + } BytecodeMethod target = resolveDirectInvokeTarget(invoke); return target == null || target.isJavascriptSuspending(); } diff --git a/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptReachability.java b/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptReachability.java index 1ba57a70230..f9a40bf5fd0 100644 --- a/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptReachability.java +++ b/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptReachability.java @@ -291,7 +291,7 @@ private boolean collectFrom(String type, String name, String desc, if (!seen.add(type)) { return true; } - if (instantiated.contains(type)) { + if (instantiated.contains(type) && isConcreteReceiver(type)) { BytecodeMethod impl = walkUp(type, name, desc); if (impl == null) { // An instantiated receiver whose body we cannot name is @@ -316,6 +316,25 @@ private boolean collectFrom(String type, String name, String desc, return true; } + /** + * Whether {@code type} can actually BE a receiver at runtime. + * + * The exported set is not a set of concrete instantiations: + * {@link JavascriptReachability#markClassInstantiated} adds a name + * before it looks at what kind of type it is, and then walks the + * supertype chain, so constructing one concrete class routinely puts + * its abstract bases and its interfaces in there too. Treating those + * as receivers made {@link #walkUp} return null for a type that can + * never be a receiver, which abandoned the whole query and dropped the + * call site back to the signature-wide answer -- defeating the + * optimization for the ordinary "abstract base declares it abstractly" + * shape. + */ + private boolean isConcreteReceiver(String type) { + ByteCodeClass cls = byName.get(type); + return cls != null && !cls.isIsInterface() && !cls.isIsAbstract(); + } + private BytecodeMethod walkUp(String startClass, String name, String desc) { String normalized; if ("".equals(name)) { @@ -349,6 +368,61 @@ private BytecodeMethod walkUp(String startClass, String name, String desc) { String base = cls.getBaseClass(); current = base == null ? null : JavascriptNameUtil.sanitizeClassName(base); } + // Nothing on the extends chain: a Java 8 interface DEFAULT method + // may still supply the body, exactly as + // JavascriptReachability.enqueueInterfaceDefault resolves it for + // liveness and as the runtime's resolveVirtual resolves it for + // dispatch. Without this a concrete receiver that inherits a + // default without overriding it resolved to nothing, and the call + // site fell back to the signature-wide answer. + return walkInterfaces(startClass, normalized, desc, new HashSet()); + } + + private BytecodeMethod walkInterfaces(String clsName, String name, String desc, Set visited) { + if (clsName == null || !visited.add(clsName)) { + return null; + } + ByteCodeClass cls = byName.get(clsName); + if (cls == null) { + return null; + } + if (cls.getBaseInterfaces() != null) { + for (String iface : cls.getBaseInterfaces()) { + BytecodeMethod found = interfaceMethod( + JavascriptNameUtil.sanitizeClassName(iface), name, desc, visited); + if (found != null) { + return found; + } + } + } + String base = cls.getBaseClass(); + return base == null ? null + : walkInterfaces(JavascriptNameUtil.sanitizeClassName(base), name, desc, visited); + } + + private BytecodeMethod interfaceMethod(String ifaceName, String name, String desc, Set visited) { + if (ifaceName == null || !visited.add(ifaceName)) { + return null; + } + ByteCodeClass iface = byName.get(ifaceName); + if (iface == null) { + return null; + } + for (BytecodeMethod m : iface.getMethods()) { + if (!m.isEliminated() && !m.isAbstract() + && name.equals(m.getMethodName()) && desc.equals(m.getSignature())) { + return m; + } + } + if (iface.getBaseInterfaces() != null) { + for (String up : iface.getBaseInterfaces()) { + BytecodeMethod found = interfaceMethod( + JavascriptNameUtil.sanitizeClassName(up), name, desc, visited); + if (found != null) { + return found; + } + } + } return null; } } diff --git a/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptSuspensionAnalysis.java b/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptSuspensionAnalysis.java index 800f54c8061..ecf53aedf87 100644 --- a/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptSuspensionAnalysis.java +++ b/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptSuspensionAnalysis.java @@ -120,6 +120,16 @@ final class JavascriptSuspensionAnalysis { * implementation of the rule and both sides call it. */ static final class DispatchModel { + // The analysis instance, so a direct (static / special) call site is + // answered by the SAME resolver that built the propagation edges. + // + // The emitter used to resolve those itself. Two resolvers meant two + // answers, and the one that mattered was a ``super`` call landing on + // an interface default: the emitter resolved it and called it + // suspending, the analysis did not and left the caller synchronous, so + // a ``yield*`` was emitted inside a plain ``function`` -- which is not + // a subtle bug but ``ReferenceError: yield is not defined``. + private final JavascriptSuspensionAnalysis analysis; private final JavascriptReachability.Model rta; // Declared on a JSO bridge class. Suspending only when the call // site's receiver cone can actually reach one of those classes. @@ -131,9 +141,10 @@ static final class DispatchModel { // Signature-wide fallback, i.e. the historical answer. private final java.util.Set suspendingSigs; - DispatchModel(JavascriptReachability.Model rta, java.util.Set jsoSigs, - java.util.Set bridgeSigs, java.util.Set jsoClasses, - java.util.Set suspendingSigs) { + DispatchModel(JavascriptSuspensionAnalysis analysis, JavascriptReachability.Model rta, + java.util.Set jsoSigs, java.util.Set bridgeSigs, + java.util.Set jsoClasses, java.util.Set suspendingSigs) { + this.analysis = analysis; this.rta = rta; this.jsoSigs = jsoSigs; this.bridgeSigs = bridgeSigs; @@ -141,6 +152,15 @@ static final class DispatchModel { this.suspendingSigs = suspendingSigs; } + /** + * The answer for an {@code INVOKESTATIC} / {@code INVOKESPECIAL}. + * An unresolvable target stays suspending, as it always did. + */ + boolean isDirectSuspending(String owner, String name, String desc) { + BytecodeMethod target = analysis.resolveTarget(owner, name, desc); + return target == null || target.isJavascriptSuspending(); + } + boolean isDispatchSuspending(String owner, String name, String desc) { String sig = name + desc; if (rta == null || isUnconditionallySuspendingDispatch(rta, jsoSigs, bridgeSigs, @@ -182,6 +202,10 @@ boolean isDispatchSuspending(String owner, String name, String desc) { // dispatch on it. Captured in propagate(); for a suspending signature this // is literally the number of ``yield*`` sites it is responsible for. private Map dispatchSiteCount = java.util.Collections.emptyMap(); + // Every INVOKEVIRTUAL / INVOKEINTERFACE instruction, by signature, + // regardless of whether the site resolved against its receiver or fell + // back to the signature-wide answer. + private final Map dispatchSites = new HashMap(); static int run(List classes, File outputDirectory) { // Same reason as JavascriptReachability.run: never let a previous @@ -488,11 +512,33 @@ private void propagate(List classes) { if (op == Opcodes.INVOKESTATIC || op == Opcodes.INVOKESPECIAL) { BytecodeMethod target = resolveTarget(inv.getOwner(), inv.getName(), inv.getDesc()); if (target == null) { + // Unresolvable, and the EMITTER treats that as + // suspending (isDirectSuspending returns true for a + // null target). Skipping the caller here made the + // two sides mean opposite things by the same + // "unknown": a ``yield*`` at the call site inside a + // method emitted as a plain ``function``, which is + // ``ReferenceError: yield is not defined`` rather + // than anything subtle. If the site is suspending, + // so is the method containing it. + markSuspending(caller, "unresolved-direct:" + + JavascriptNameUtil.sanitizeClassName(inv.getOwner()) + + "." + inv.getName() + inv.getDesc()); continue; } addCaller(callersOf, target, caller); } else if (op == Opcodes.INVOKEVIRTUAL || op == Opcodes.INVOKEINTERFACE) { String sig = inv.getName() + inv.getDesc(); + // Count the site BEFORE the receiver-resolved branch + // returns. Counting from sigCallersOf alone measured + // only the fallback path, so under the default RTA + // path the report showed dispatch sites collapsing to + // near zero -- an artefact of where the edge was + // recorded, not a reduction in emitted call sites. + if (reportPath != null) { + Integer prev = dispatchSites.get(sig); + dispatchSites.put(sig, Integer.valueOf(prev == null ? 1 : prev.intValue() + 1)); + } // Resolve the call site against its RECEIVER TYPE // rather than its bare signature. A cone that // resolves gives us exact per-impl edges, so a @@ -563,20 +609,12 @@ private void propagate(List classes) { } } } - if (reportPath != null) { - // One entry per dispatch INSTRUCTION, so for a suspending sig this - // counts the ``yield*`` sites it costs. - Map counts = new HashMap(); - for (Map.Entry> e : sigCallersOf.entrySet()) { - counts.put(e.getKey(), Integer.valueOf(e.getValue().size())); - } - dispatchSiteCount = counts; - } + dispatchSiteCount = dispatchSites; // Publish the final suspending-sig set so the emitter can // consult it when deciding whether an INVOKEVIRTUAL / // INVOKEINTERFACE call site needs ``yield*`` wrapping. exportedSuspendingSigs = suspendingSigs; - exportedDispatchModel = new DispatchModel(rta, + exportedDispatchModel = new DispatchModel(this, rta, new java.util.HashSet(jsoDeclaredSigs), new java.util.HashSet(bridgeDispatchSigs), new java.util.HashSet(jsoBridgeClasses), @@ -652,7 +690,25 @@ private static void addSigCaller(Map> sigCallersOf, * to the translator's canonical ``__INIT__`` / ``__CLINIT__`` * form before comparison. */ + // owner#name+desc -> resolved direct-invoke target, null included. + // + // This is called once per direct invoke while building the propagation + // edges and again per direct invoke while emitting, and since it gained an + // interface walk on the miss path an unmemoised version made + // JavascriptTargetIntegrationTest go from 48s to 378s. + private final Map resolvedTargets = new HashMap(); + private BytecodeMethod resolveTarget(String owner, String name, String desc) { + String key = owner + "#" + name + desc; + if (resolvedTargets.containsKey(key)) { + return resolvedTargets.get(key); + } + BytecodeMethod resolved = resolveTargetUncached(owner, name, desc); + resolvedTargets.put(key, resolved); + return resolved; + } + + private BytecodeMethod resolveTargetUncached(String owner, String name, String desc) { String clsName = JavascriptNameUtil.sanitizeClassName(owner); String normalizedName; if ("".equals(name)) { @@ -852,6 +908,9 @@ private void writeReport(int total, int sync, List methodLines, } out.println("#"); out.println("# Section 2: suspending signatures ranked by dispatch call sites."); + out.println("# dispatchSites counts every INVOKEVIRTUAL / INVOKEINTERFACE on the"); + out.println("# signature, receiver-resolved and fallback alike -- NOT the number"); + out.println("# that end up emitting yield*, which depends on each site's receiver."); out.println("# SIG "); for (String sig : sigs) { Integer siteCount = sites.get(sig); From 50bdc20909795647dc8c206b24c1197579c4de37 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Thu, 10 Sep 2026 11:24:29 +0300 Subject: [PATCH 08/11] Count both dispatch-cause spellings in the report's ranking Section 2 looked its firstCauseMethods column up as ``dispatch:``, but a receiver-resolved edge records ``dispatch:.``. So the column counted the FALLBACK path alone -- which, once call sites resolve against their receiver, is the minority -- and understated exactly the signatures the owner-aware path is there to handle. The ranking that is supposed to point at the next thing worth fixing pointed away from it. Both spellings are folded onto the signature now. Splitting on the first '.' is exact rather than approximate: class names are sanitized to identifier characters and a JVM descriptor uses '/', so the only dot in the key is the owner separator. vm/tests: 305 tests, 0 failures. SpotBugs 0 findings. Co-Authored-By: Claude Opus 5 (1M context) --- .../JavascriptSuspensionAnalysis.java | 38 ++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptSuspensionAnalysis.java b/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptSuspensionAnalysis.java index ecf53aedf87..dfd43e1eb54 100644 --- a/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptSuspensionAnalysis.java +++ b/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptSuspensionAnalysis.java @@ -858,6 +858,37 @@ public int compare(String a, String b) { } } + /** + * Folds every {@code dispatch:} cause onto the bare signature it concerns. + * + * A receiver-resolved edge records {@code dispatch:.} and + * a fallback edge records {@code dispatch:}. Section 2 ranks by + * signature, so it has to count both; reading only the bare key counted + * the fallback path alone, which is the minority once call sites resolve + * against their receiver -- the ranking then understated exactly the + * signatures the owner-aware path handles. + * + * Splitting on the first {@code '.'} is exact: class names are sanitized to + * identifier characters, and a JVM descriptor uses {@code '/'} rather than + * {@code '.'}, so the only dot present is the owner separator. + */ + private static Map aggregateDispatchCauses(Map causeCount) { + Map bySig = new HashMap(); + for (Map.Entry entry : causeCount.entrySet()) { + String cause = entry.getKey(); + if (!cause.startsWith("dispatch:")) { + continue; + } + String rest = cause.substring("dispatch:".length()); + int dot = rest.indexOf('.'); + String sig = dot < 0 ? rest : rest.substring(dot + 1); + Integer prev = bySig.get(sig); + bySig.put(sig, Integer.valueOf( + (prev == null ? 0 : prev.intValue()) + entry.getValue().intValue())); + } + return bySig; + } + /** ``owner.name+descriptor``, the identity used throughout the report. */ private static String qualify(BytecodeMethod m) { return m.getClsName() + "." + m.getMethodName() + m.getSignature(); @@ -911,10 +942,15 @@ private void writeReport(int total, int sync, List methodLines, out.println("# dispatchSites counts every INVOKEVIRTUAL / INVOKEINTERFACE on the"); out.println("# signature, receiver-resolved and fallback alike -- NOT the number"); out.println("# that end up emitting yield*, which depends on each site's receiver."); + out.println("# firstCauseMethods aggregates BOTH cause spellings for the"); + out.println("# signature: the receiver-resolved ``dispatch:.`` and the"); + out.println("# fallback ``dispatch:``. Reading only the bare key counted the"); + out.println("# fallback path alone, which is the minority under RTA."); out.println("# SIG "); + Map dispatchCauseBySig = aggregateDispatchCauses(causeCount); for (String sig : sigs) { Integer siteCount = sites.get(sig); - Integer firstCause = causeCount.get("dispatch:" + sig); + Integer firstCause = dispatchCauseBySig.get(sig); out.println("SIG " + (siteCount == null ? 0 : siteCount.intValue()) + " " + (firstCause == null ? 0 : firstCause.intValue()) + " " + sig); From a408a61bb3976820423d3ca19afcf50a1e982287 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Thu, 10 Sep 2026 12:28:23 +0300 Subject: [PATCH 09/11] Walk interfaces the way the runtime does: breadth first The receiver model's interface search recursed depth first. jvm.resolveVirtual does not: it collects every interface of the class chain, walks that queue FIFO, and pushes each interface's own super-interfaces on the TAIL. The two disagree on a shape Java allows. For ``C implements Left, Right`` where Left only inherits ``Root.f()`` and Right overrides it, the depth-first walk descends Left -> Root and answers ``Root.f()`` before it has looked at Right; the runtime answers ``Right.f()``. If the root default is synchronous and the override suspends, the analysis picks the synchronous dispatcher for a call the runtime resolves to a generator, which is the ``cn1_ivs ... (CHA unsound)`` failure this branch has already paid for once. The other two interface walks added on this branch -- the emitter's and the analysis's direct-invoke resolvers -- were already queue-based; this one was the odd one out. vm/tests: 305 tests, 0 failures. SpotBugs 0 findings. Co-Authored-By: Claude Opus 5 (1M context) --- .../translator/JavascriptReachability.java | 81 ++++++++++--------- 1 file changed, 44 insertions(+), 37 deletions(-) diff --git a/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptReachability.java b/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptReachability.java index f9a40bf5fd0..123925f196b 100644 --- a/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptReachability.java +++ b/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptReachability.java @@ -378,48 +378,55 @@ private BytecodeMethod walkUp(String startClass, String name, String desc) { return walkInterfaces(startClass, normalized, desc, new HashSet()); } + /** + * BREADTH-first, because that is what {@code jvm.resolveVirtual} does: + * it collects every interface of the class chain, then walks that queue + * FIFO and pushes each interface's own super-interfaces on the TAIL. + * + * A depth-first walk picks a different method, and picks it wrongly. + * For {@code C implements Left, Right} where {@code Left} only inherits + * {@code Root.f()} and {@code Right} overrides it, depth-first descends + * Left -> Root and answers {@code Root.f()} before it has looked at + * Right; the runtime answers {@code Right.f()}. If the root default is + * synchronous and the override suspends, the analysis picks the sync + * dispatcher for a call the runtime resolves to a generator -- the + * ``cn1_ivs ... (CHA unsound)`` failure. + */ private BytecodeMethod walkInterfaces(String clsName, String name, String desc, Set visited) { - if (clsName == null || !visited.add(clsName)) { - return null; - } - ByteCodeClass cls = byName.get(clsName); - if (cls == null) { - return null; - } - if (cls.getBaseInterfaces() != null) { - for (String iface : cls.getBaseInterfaces()) { - BytecodeMethod found = interfaceMethod( - JavascriptNameUtil.sanitizeClassName(iface), name, desc, visited); - if (found != null) { - return found; + java.util.ArrayDeque pending = new java.util.ArrayDeque(); + String current = clsName; + Set chain = new HashSet(); + while (current != null && chain.add(current)) { + ByteCodeClass cls = byName.get(current); + if (cls == null) { + break; + } + if (cls.getBaseInterfaces() != null) { + for (String iface : cls.getBaseInterfaces()) { + pending.add(JavascriptNameUtil.sanitizeClassName(iface)); } } + String base = cls.getBaseClass(); + current = base == null ? null : JavascriptNameUtil.sanitizeClassName(base); } - String base = cls.getBaseClass(); - return base == null ? null - : walkInterfaces(JavascriptNameUtil.sanitizeClassName(base), name, desc, visited); - } - - private BytecodeMethod interfaceMethod(String ifaceName, String name, String desc, Set visited) { - if (ifaceName == null || !visited.add(ifaceName)) { - return null; - } - ByteCodeClass iface = byName.get(ifaceName); - if (iface == null) { - return null; - } - for (BytecodeMethod m : iface.getMethods()) { - if (!m.isEliminated() && !m.isAbstract() - && name.equals(m.getMethodName()) && desc.equals(m.getSignature())) { - return m; + while (!pending.isEmpty()) { + String ifaceName = pending.poll(); + if (ifaceName == null || !visited.add(ifaceName)) { + continue; } - } - if (iface.getBaseInterfaces() != null) { - for (String up : iface.getBaseInterfaces()) { - BytecodeMethod found = interfaceMethod( - JavascriptNameUtil.sanitizeClassName(up), name, desc, visited); - if (found != null) { - return found; + ByteCodeClass iface = byName.get(ifaceName); + if (iface == null) { + continue; + } + for (BytecodeMethod m : iface.getMethods()) { + if (!m.isEliminated() && !m.isAbstract() + && name.equals(m.getMethodName()) && desc.equals(m.getSignature())) { + return m; + } + } + if (iface.getBaseInterfaces() != null) { + for (String up : iface.getBaseInterfaces()) { + pending.add(JavascriptNameUtil.sanitizeClassName(up)); } } } From 0eecc012c43bb79a53b5229cbf8ff892617fa565 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Thu, 10 Sep 2026 12:57:23 +0300 Subject: [PATCH 10/11] The interpreter path devirtualizes too, and a clean exit is not a measurement Two more findings, both real. appendInvokeInstruction's inline arity 0-4 path picked _dv*/_dw* and its yield* from isInvokeSuspending() alone -- the call site's signature-wide verdict. The straight-line and compact paths were changed to follow the devirtualized TARGET; this one was missed, so a method complex enough to be emitted through the interpreter rather than structured could still yield* a plain function or drive a generator synchronously at exactly the sites monomorphicSuspending was added for. The benchmark runner treated a zero exit as a result. A workload process that reaches BENCHSUITE without emitting its BENCH row leaves that workload out of the JSON, and the baseline-vs-new check added earlier cannot help: it compares against the baseline, so a workload missing from the BASELINE is invisible to every later comparison. The row is asserted per process now, where the omission is still local. Proven non-vacuous with a fixture whose ``run("ghostWorkload", 99)`` is parsed out of the source but sits behind an unreachable branch: the runner exits 1 with "workload ghostWorkload exited cleanly but emitted no BENCH row". vm/tests: 305 tests, 0 failures. SpotBugs 0 findings. The benchmark still runs its twelve workloads clean. Co-Authored-By: Claude Opus 5 (1M context) --- scripts/run-javascript-throughput-benchmark.sh | 10 ++++++++++ .../tools/translator/JavascriptMethodGenerator.java | 9 +++++++++ 2 files changed, 19 insertions(+) diff --git a/scripts/run-javascript-throughput-benchmark.sh b/scripts/run-javascript-throughput-benchmark.sh index 26c1e4b688e..66024f25baa 100755 --- a/scripts/run-javascript-throughput-benchmark.sh +++ b/scripts/run-javascript-throughput-benchmark.sh @@ -220,6 +220,16 @@ for workload in $WORKLOADS; do sed -n '1,40p' "$WORK_DIR/stderr.txt" >&2 exit 1 fi + # Exiting 0 is not the same as producing a measurement. A process that + # reaches BENCHSUITE without emitting its BENCH row leaves that workload out + # of the JSON, and the baseline-vs-new check cannot catch a workload that was + # missing from the BASELINE in the first place -- every later comparison is + # then blind to it. Assert the row here, where the omission is still visible. + if [ "$filter" != "" ] && ! grep -q "^BENCH id=$workload " "$RAW"; then + bench_log "workload $workload exited cleanly but emitted no BENCH row" + sed -n '1,20p' "$WORK_DIR/stderr.txt" >&2 + exit 1 + fi done cat "$RAW" >&2 diff --git a/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptMethodGenerator.java b/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptMethodGenerator.java index 7458c1aa6b1..fed2a486679 100644 --- a/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptMethodGenerator.java +++ b/vm/ByteCodeTranslator/src/com/codename1/tools/translator/JavascriptMethodGenerator.java @@ -7145,6 +7145,15 @@ private static void appendInvokeInstruction(StringBuilder out, Invoke invoke, in // would resolve to a now-missing m: entry ("Missing virtual // method ..."). String monoImpl = monomorphicDispatch == null ? null : monomorphicDispatch.get(dispatchId); + // A devirtualized site follows its TARGET, exactly as the + // straight-line and compact paths do. This one was left reading + // the call site's signature-wide verdict, so a complex method -- + // one emitted through the interpreter rather than structured -- + // could still ``yield*`` a plain function or drive a generator + // synchronously at precisely the sites this map exists for. + if (monoImpl != null) { + susp = isDevirtualizedInvokeSuspending(dispatchId, susp); + } String iv = monoImpl != null ? (susp ? "_dv" : "_dw") : (susp ? "_v" : "_w"); String ivSecond = monoImpl != null ? monoImpl : ("\"" + dispatchId + "\""); String yk = susp ? "yield* " : ""; From e177ddf8f11f5e0be82784185178411a54e8a22e Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Thu, 10 Sep 2026 13:13:50 +0300 Subject: [PATCH 11/11] The benchmark's process isolation never worked, and it hid a phantom regression ``public static String only = ""`` compiles to a store in , and the generated main wrapper initializes the class before its body runs -- so the filter the harness wrote was overwritten with the empty string and every "isolated" process ran the entire suite. Measured: twelve processes emitted 144 BENCH rows instead of 12. The per-process BENCH-row assertion added in the previous commit could not see it either, because every process emitted every row. The field now has no initializer at all (nothing to overwrite it) and the harness forces the class initializer before writing, so the ordering cannot bite again. Verified: 12 rows, 12 processes. This matters beyond tidiness, because it retracts a result. Every number this branch reported as isolated was a shared-process number, and the one that looked worst was the artefact: iteratorWalk was reported as a reproducible +13.7% regression, twelve times its noise floor, in a method whose emitted body and all nineteen of whose iterator callees were byte-identical between the arms. That was never a regression. Re-measured with isolation that actually isolates, interleaved, best of three: hashCodeHeavy -60.6% iteratorWalk -1.1% toStringHeavy -24.0% monoVirtual -0.1% mapChurn -9.0% stringOps +0.3% polyVirtual -3.9% arithControl +0.5% (control) megaVirtual -3.7% suspendControl +0.3% (control) ifaceDispatch -2.5% equalsHeavy -2.4% Both controls flat, and the sync-dispatcher probe reads 0 on both arms. The absolute times also fall across the board -- iteratorWalk 14.4ms to 8.6ms, hashCodeHeavy 13.6 to 8.6 -- which is what it looks like when a workload stops inheriting eleven other workloads' heap and JIT state. Co-Authored-By: Claude Opus 5 (1M context) --- scripts/run-javascript-throughput-benchmark.sh | 4 ++++ vm/benchmarks/javascript/JsThroughputBench.java | 12 ++++++++++-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/scripts/run-javascript-throughput-benchmark.sh b/scripts/run-javascript-throughput-benchmark.sh index 66024f25baa..7de9eaaa242 100755 --- a/scripts/run-javascript-throughput-benchmark.sh +++ b/scripts/run-javascript-throughput-benchmark.sh @@ -171,6 +171,10 @@ if (typeof global.__parparInstallNativeBindings === 'function') { // run nothing else. See the comment on JsThroughputBench.only. const only = process.argv[3] || ''; if (only) { + // Run the class initializer FIRST. Writing the filter before means + // gets the last word, which is how every "isolated" process ended + // up running the whole suite. + jvm.ensureClassInitialized(process.argv[4]); const cls = jvm.classes[process.argv[4]]; if (!cls || !cls.staticFields) { throw new Error('cannot reach bench class to set filter'); } cls.staticFields['only'] = jvm.createStringLiteral(only); diff --git a/vm/benchmarks/javascript/JsThroughputBench.java b/vm/benchmarks/javascript/JsThroughputBench.java index 712c52a120d..9c734f0a1db 100644 --- a/vm/benchmarks/javascript/JsThroughputBench.java +++ b/vm/benchmarks/javascript/JsThroughputBench.java @@ -64,7 +64,15 @@ public class JsThroughputBench { /** * Set by the harness before {@code main} runs (a plain static write into - * the class's field table). Empty means "run everything". + * the class's field table). Null or empty means "run everything". + * + * Deliberately has NO initializer. {@code = ""} compiles to a store in + * {@code }, and the generated {@code main} wrapper initializes the + * class before its body runs -- so the harness's value was overwritten + * with the empty string and every "isolated" process silently ran the + * whole suite. That is not a small bias: it made twelve processes emit + * 144 rows instead of 12, and every number reported as isolated was a + * shared-process number. * * Per-workload isolation is not a convenience, it is a correctness * requirement for this suite. Measured in one process, a workload inherits @@ -74,7 +82,7 @@ public class JsThroughputBench { * regression in code that is byte-for-byte identical. That happened, and * it cost an hour to disprove. */ - public static String only = ""; + public static String only; public static void main(String[] args) { run("monoVirtual", 1);