ParparVM: make the translator self-hosting, and fix what that exposed - #5766
ParparVM: make the translator self-hosting, and fix what that exposed#5766shai-almog wants to merge 26 commits into
Conversation
javac lowers a primitive class literal to a read of the boxed type's own TYPE
field, so `TYPE = int.class` inside Integer's initializer compiles to
`getstatic TYPE; putstatic TYPE` -- it reads the field it is initializing and
leaves it null. Integer, Long, Byte, Character and Double all declared TYPE that
way and all had a null one; Short, Boolean and Float had no TYPE at all; and
Void.TYPE was java.lang.Void, the wrapper, rather than void.
Nothing threw. Measured on a translated binary before this change:
TYPE Integer null=true TYPE Double null=true TYPE Void name=java.lang.Void
map size 2 of 6 m.get(Integer.TYPE) -> "JAVA_DOUBLE"
A Map keyed on them collapses onto the single null key, so every lookup answers
with whatever was stored last. The translator's own Util.ctypeMap/sigTypeMap are
exactly that shape, keyed on all nine, which is how this surfaced: it would have
typed every primitive alike and emitted syntactically valid C with every
primitive type wrong.
The JDK declares a native for this for the same reason, and so does this:
- nine scalar `struct clazz` objects in cn1_globals.m, with designated rather
than positional initializers so a future field added to struct clazz cannot
silently shift every value the way it would in the generated ones beside them
- java_lang_Class_getPrimitiveClass, taking an int code rather than the JDK's
String name -- this runs inside the wrapper class initializers, which are
among the earliest code in the process, and decoding a Java String here would
drag String.getBytes and the charset machinery into Integer's own clinit
- isAssignableFrom and isInstance now test primitiveType before calling
instanceofFunction, which indexes tables by classId; a primitive class
carries a sentinel classId that no table has a row for
__codenameOneParentClsReference has to be set to class__java_lang_Class as the
generated clazz objects do. CN1_CLASS_OF reads it to find the vtable when a clazz
is used as an ordinary object, which is what happens the moment one becomes a Map
key -- leaving it zero segfaults on the first hashCode(), well away from anything
that names it.
PrimitiveTypeIntegrationTest compares a translated run against a real JVM rather
than a hard-coded expectation, because the failure was self-consistent and silent:
only an independent reference catches it. Confirmed to fail when TYPE = int.class
is put back.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ByteCodeTranslator's own bytecode, plus ASM's, translates to C and compiles into
a native binary that performs real translations. vm/selfhost/build-selfhost.sh
builds it and verify-selfhost.sh compares its output against the JVM-hosted
translator's.
The point is validation. The translator is a ~37k-line program that exercises
collections, strings, file I/O, exceptions and the GC at scale, so running both
builds over the same input and diffing the emitted C is an end-to-end conformance
test of the whole VM -- one whose corpus grows on its own as the translator does.
It has already earned that: three defects fell out of it, each invisible to every
existing test because each was self-consistent on HotSpot.
- C label names came from identity hash codes. ASM's Label.toString() is
"L" + System.identityHashCode(this), so the emitted C was irreproducible; and
on ParparVM, whose identity hash is the object pointer narrowed to int and so
often negative, it emitted label_L-180306432001, which C reads as a
subtraction. Every method with a try/catch failed to compile. Labels are now
numbered per method in bytecode order.
- C local-variable declarations were emitted in HashSet iteration order, so the
same input produced different C. debugVarEntries had already had to learn this
for the debug side-table; the declarations had the same defect and now share
its comparator.
- Class.getResourceAsStream returned a hard-coded null on every ParparVM target.
It now consults resources linked into the executable through a weakly-defined
cn1FindResource -- which the generated resource table overrides on targets that
embed them -- and then a search path from CN1_RESOURCE_PATH.
JavaAPI grows only where ASM's bytecode forces it, because ASM is a jar we cannot
edit: Integer.rotateLeft, Double.doubleToRawLongBits, Float.floatToRawIntBits,
the three-argument Class.forName, and TypeNotPresentException. Everything the
translator's own source needed was removed from the translator instead:
- String.split/replaceAll are gone from it entirely (Util.splitLiteral,
splitWhitespace, collapseWhitespace, rewriteLocalObjectRefs). Declaring them in
JavaAPI would have collided with BytecodeComplianceMojo, which rewrites those
calls onto com.codename1.util.regex precisely because JavaAPI lacks them.
UtilStringHelperTest holds each replacement against the JDK original over ~6000
fuzzed inputs; it caught one real divergence, that String.split returns { s }
when the pattern never matches instead of dropping the trailing empty.
- The 24 two-argument System.getProperty calls go through Util.getProperty,
which uses the one-argument form JavaAPI does have and falls back to getenv.
That also makes the knobs work in a native build, which no -D can.
- The ~110 java.nio.file calls are plain java.io again. Parser and
ConcatenatingFileOutputStream already used those constructors under the
zero-findings SpotBugs gate, so both idioms already coexisted.
- java.util.zip is confined to ArchiveClassScanner and DebugSymbolCompressor,
and NativeSignatureVerifier's command-line half moved to
NativeSignatureVerifierCli. JavaAPI cannot gain java.util.zip: it is mirrored
by Ports/CLDC11, where the package does not belong. Splitting out the CLI also
removes the second main() that made ByteCodeClass refuse the translation with
"Multiple main classes".
vm/selfhost/stubs holds no-op replacements used only by the native build, for the
JavaScript target and those two zip users.
Gate D (the native translator against itself) passes. Gate A (JVM against native)
is at 245 of 247 files byte-identical, and binaries built from the two trees
produce identical output. The remaining two files are java_util_HashMap.c/.h,
where the native pass culls seven more methods and emits them as stubs; both
trees compile, link and run correctly, but the two runtimes should not disagree.
Not nondeterminism (gate D passes on both) and not identity-hash order (tested by
re-running the JVM under -XX:hashCode=2, byte-identical output). Written up in
vm/selfhost/README.md.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… memory
bench-selfhost.sh runs both translators over the same corpus, interleaved, taking
the minimum wall clock and the peak phys_footprint (never ps rss). It refuses to
print ratios unless the two emitted identical C.
On the self-hosting corpus -- ASM plus the translator's own classes, ~570 classes
-- in the documented release shape (-O3 -flto=thin), against JDK 8:
wall clock (min of 3) parpar 7.06s jdk8 1.17s jdk8 6.0x faster
peak phys_footprint parpar 1434MB jdk8 509MB jdk8 2.8x smaller
That is the opposite of the expectation on both axes, so it is worth being clear
that it is a real measurement rather than a mistake. /usr/bin/time -l independently
reports 1328 MB and 501 MB, agreeing with the sampled vmmap figures; building at
-O1 rather than -O3 -flto=thin changes nothing measurable, so code quality is not
the bottleneck; and the corpus is large enough that JVM startup is not carrying the
result.
The user-versus-real split locates most of the gap:
parpar 6.29 real 7.31 user -> ~1.2x parallelism
jdk8 1.13 real 6.09 user -> ~5.4x parallelism
The two burn comparable CPU. HotSpot spends it across cores on JIT compiler
threads and parallel GC, while the translated program is single-threaded, so the
6x is mostly concurrency ParparVM does not have rather than per-instruction code
quality.
Fixing an early error in the harness, since it is the kind that reads as a result:
the memory sampler took $! from a subshell wrapper and reported the wrapper's
~1.3 MB footprint for both arms. It now execs the translator so the pid is the
process being measured.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…entical
The self-hosted translator emitted seven java.util.HashMap methods as empty stubs
that the JVM-hosted one emitted in full. The cause was not the VM: it was the
blanket "Javascript*" exclusion in build-selfhost.sh, which stubbed
JavascriptNativeRegistry along with the two classes that genuinely cannot compile
against JavaAPI.
That class compiles fine, and -- as the comment at its call site in Parser says --
it is consulted on EVERY target, not just JavaScript, because the C natives use
some of the same methods as fallbacks. Its RUNTIME_DELEGATE_TARGETS lists
java_util_HashMap's getImpl, putImpl, removeImpl, containsKeyImpl and clearImpl;
answering false for them let the dead-code pass cull all five, and with them
cn1PutSlot and cn1MaybeGrow, which nothing else calls.
Found by instrumenting the cull decision and diffing the two runs: the five showed
up as "examined jvm=0x parpar=1x" -- the JVM never even reached the cull check for
them, because isRuntimeDelegateTarget had already made it `continue`.
The stub list is now driven by what is actually in stubs/ rather than by a name
pattern, so only the sources that cannot compile are replaced.
Gate A (JavaAPI corpus, 247 files) byte-identical
Gate A (self-hosting corpus, 797 files / 21.6 MB) byte-identical
Gate D (native against itself), both corpora pass
The second of those is the bootstrap gate, and it is a stronger statement than
GCC's three-stage comparison: there is no foreign compiler in the loop, so the
program really is identical and only the runtime executing it changed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ot the GC
The wall-clock gap to JDK 8 is almost entirely the allocator's backpressure
throttle. `sample` on a default run puts 64% of the process's samples in a single
stack, and the mutator is not marking or sweeping -- it is asleep:
Ldc.getValueAsString -> cn1BibopAlloc -> cn1BibopMaybeGc
-> cn1PacingPark (3491 of 5476 samples)
-> usleep -> nanosleep -> __semwait_signal (3475)
CN1_LOG_PACING_PARKS reports only TWO park events for the whole run, so each one
is seconds long.
Isolated by A/B, translating ~570 classes on a 64GB / 16-core host, release shape:
as shipped 6.7-8.7s 6 cycles 2 parks 1434MB
CN1_GC_TRIGGER_MB=32768 (no GC) 1.42s 3 cycles 0 parks
CN1_GC_PACING_CAP_MB=4096 1.45s 4 cycles 0 parks
growth clamp disarmed 1.39-1.52s 4 cycles 0 parks 1467MB
Collection itself is nearly free: with the clamp disarmed the collector still runs
its four cycles and the time matches disabling GC outright. Against JDK 8 that is
1.19x, ordinary AOT-versus-warmed-JIT territory, instead of 6x.
The mechanism, from cn1BibopPacingCap: it computes fm/8 -- 4GB on this host -- and
then clamps to `trigger * CN1_BIBOP_GC_MAX_CAP_MULTIPLIER` once
cn1PacingPastGrowthFloor() is true, which is a process footprint over
CN1_PACING_GROWTH_FLOOR_BYTES (512MB). Early in the run the trigger is still at its
own 24MB floor, so the ceiling is 24 * 8 = 192MB, matching the observed
minCapKb=196608 exactly. A program whose live set is ~1.4GB cannot stay inside a
192MB allocation window, so it parks against a collector that can never get under
it. That bound is calibrated for phone-sized heaps and has no scaling for a 64GB
host: it costs 5x throughput to save 2% of peak footprint here. Left alone, since
what it should scale with is a policy call for the VM owners; the reproduction is
one -DCN1_PACING_GROWTH_FLOOR_BYTES, documented in vm/selfhost/README.md.
One real defect found alongside it IS fixed: cn1RefreshFreeMemCache() had exactly
one caller, inside the mark cycle, so cn1CachedFreeMem stayed 0 until the first
collection and the cap sat at its 72MB floor through the window with the least
reason to throttle anything. Primed in cn1BibopDoInit now.
ProcessBudgetPacingIntegrationTest's control arm reports minCapKb=4194304 with the
fix and the 72MB floor without it, and its budget-bounded arm still engages
backpressure (legacyParks=52, boundedChecks=771), so the ceiling that bound exists
to enforce is untouched.
Gates A and D still pass byte-identical on both corpora after the change.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…pool
Two fixes take the self-hosted translator from 6.0x slower than JDK 8 to 1.18x
slower than JDK 25 -- and 1.24x FASTER than JDK 8 -- on the self-hosting corpus.
Benchmarks now use JDK 25 as the reference; JDK 8 is kept only because it is what
the builders fork.
parpar 1.84s / 1443MB jdk25 1.56s / 516MB jdk8 2.27s / 502MB
1. The mutator slept instead of allocating, and would on any machine.
cn1BibopPacingCap computes a cap from available memory (fm/8, 4GB here) and then
clamps it to trigger * 8 once cn1PacingPastGrowthFloor() is true. That floor was a
flat 512MB. Early in a run the trigger is still at its own 24MB floor, so the
ceiling was 192MB -- confirmed by minCapKb=196608 -- and a program with a ~1.4GB
live set cannot stay inside a 192MB allocation window. It parked against a
collector that could never get under it: `sample` put 64% of samples in
cn1PacingPark -> usleep, from just two park events, each seconds long.
A fixed 512MB says the process has grown; it does not say the machine is under
pressure, and the bound exists for pressure. The floor is now
max(512MB, availableMemory/4). Where cn1_available_memory is the flat 100MB
placeholder (Linux, Windows, non-Apple fallback) the absolute floor still wins and
behaviour is bit-for-bit unchanged, and the floor can only rise, never fall, so no
constrained host becomes more permissive than it was.
This is the no-per-process-ceiling path only. Where a ceiling exists -- iOS's
dirty-memory limit, or an explicit budget -- cn1PacingPark takes the bounded branch
and never reaches cn1BibopPacingCap. ProcessBudgetPacingIntegrationTest covers both
halves and still passes: control arm minCapKb=4194304 with zero parks, bounded arm
holding a 120MB limit at a 60MB peak across 427 parks.
cn1RefreshFreeMemCache() also had exactly one caller, inside the mark cycle, so
cn1CachedFreeMem was 0 until the first collection and both the cap and this floor
sat at their absolute minimums during the window with the least reason to throttle.
Primed in cn1BibopDoInit.
2. Parser.addToConstantPool was O(n^2).
With pacing out of the way the main thread's profile was dominated by
constantPool.indexOf(s) -- a String.equals against every string already interned,
and the pool holds ~200k of them on a self-hosting translation. It was 32% of
main-thread samples across String.equals (11.2%), the list iterator (10.3%),
indexOf (6.2%) and ArrayList.get (5.1%). A HashMap side index answers the same
question; the list stays the source of truth so the emitted indices are unchanged.
Gates A and D still pass byte-identical on both corpora after both changes.
Still open: peak footprint is 2.8x JDK 25's. It is retained data, not garbage --
sweeping the trigger from 8MB to 256MB moves peak less than 15% -- and it scales
with the object graph rather than being a fixed cost (2.37x on a hello-world
corpus, 2.73x on the full one). The 16-byte object header and BiBOP size-class
rounding do not account for it, and compact strings are not the explanation either
since JDK 8 has none and still fits in ~500MB.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
cn1HeapAccounting and cn1AllocCensus were written but called from nowhere, so
nothing in the tree could answer "what is the footprint actually made of". They
now run post-sweep (the same world-consistent point the GC verifier uses) and once
at exit, under -DCN1_ALLOC_CENSUS plus CN1_HEAP_REPORT at run time, so an ordinary
build is untouched. A batch program usually ends between collections, hence the
atexit report as well as the per-cycle ones.
The forward declarations sit outside the CN1_GC_VERIFY block: putting them next to
cn1GcVerifyHeap looked natural and compiled to nothing in a census build, since
that block is off.
build-selfhost.sh: -O3 now implies -flto=thin. That IS the documented release
shape, and measured over five interleaved rounds it is the only rung that beats
-O1 -- 1.45s against 1.61s for -O1, 1.70s for -O2 and 1.73s for bare -O3.
Benchmarking a plain -O3 binary and calling it the release build understates it,
which is too easy to do when the flag is left to the caller to remember.
First results on the self-hosting corpus (~570 classes), at exit:
bibop pages=12029 reserved=751.81MB live=749.62MB slack=2.19MB
legacy objects=729174 bytes=110.85MB
JAVA TOTAL live=860.47MB process peak phys_footprint=1467MB
Two things fall out immediately. Page-pool slack is 2.19MB, so fragmentation is
not the memory story. And the Java heap is 860MB of a 1467MB process, so roughly
600MB is not the Java heap at all and needs its own answer.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two defects found by the per-class allocation census, on a self-hosting
translation of the ParparVM translator.
IdentityHashMap allocated an Entry on EVERY next(), including key and value
iteration, where the entry existed only to have one field read back out of it and
then dropped: 1,366,140 of them, 43.7MB, all garbage. java.util.HashMap already
had separate key/value/entry iterators for exactly this reason -- its key iterator
reads the flat table directly -- and this map had been left on the older shape.
It now has the same split, with the generic MapEntry.Type callback used only for
entrySet, which is the one view where a caller can observe an Entry at all.
ArrayList's no-arg constructor called this(10), so every list allocated a
128-byte slot up front, including one that is never added to. It now shares a
zero-length array until the first growth. That growth allocates exactly ten, not
the twelve the general growth path picks: ten keeps a one-to-ten element list in
the size class it already occupied, and growing to twelve would have traded a win
on empty lists for a loss on the common case.
Measured together on the self-hosting corpus:
allocations 10,160,401 objects / 991MB -> 8,706,929 / 940MB
legacy-heap objects 729,174 -> 444,783
Java live heap 860MB -> 770MB
process peak 1467MB -> 1324MB
CollectionSemanticsIntegrationTest holds both against a real JDK rather than a
hand-written expectation, since these fail at the edges and are invisible when
they work: empty-list operations, all three growth paths, identity semantics, null
keys and values through each of the three views, iterator removal, and a rehash.
Confirmed to fail when the key iterator stops mapping the table's NULL_OBJECT
sentinel back to null.
HashMap was investigated and deliberately left alone. It eagerly allocates three
arrays at capacity 16 and looks like the same defect, but the maps in this
workload are populated rather than empty, so the table is not waste. Rebuilding
with a default capacity of 1 -- the cheapest probe for how much of it is wasted --
made everything worse, because the maps then regrow repeatedly:
default capacity 16 Object[] 1,324,987 int[] 213,725 live 770MB
default capacity 1 Object[] 1,802,249 int[] 452,356 live 882MB
Its growth is also post-insert by design, so the shared-empty-table trick that
works for ArrayList would leave the put path writing into the shared table.
Also recorded in vm/selfhost/README.md: String's `long nsString` field, which
backs the Apple targets' direct NSString mapping, costs nothing anywhere else.
sizeof(obj__java_lang_String) is 48 with the field at offset 40, and the fields
before it end at 36, so half of those eight bytes were padding already. Without it
the struct is 40 bytes, and BiBOP's size classes are 32, 48, 64 -- both land in the
same 48-byte slot. Removing it would save zero bytes per String and cost the Apple
targets a side table and a lookup.
Full vm/tests suite: 36 classes, no failures.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
cn1AllocCensus answers churn -- what was allocated, which is what costs CPU. There
was nothing that answered retention -- what is still here, which is what costs
memory -- and those are different questions: an iterator allocated a million times
retains nothing, a cache allocated once retains everything.
cn1LiveCensus walks the BiBOP pages and the legacy heap after a sweep and reports
the heap by class: occupied bytes, object count, bytes each, and how many of them
the last mark proved reachable. Objects are charged what they OCCUPY -- a whole
size-class slot, a whole malloc block -- so the rows add up to the footprint and
rounding waste lands on the class that causes it. Classes are collected in a local
pointer-keyed table rather than read from cn1ClazzSet, which only exists under
CN1_CONSERVATIVE_GC_ROOTS.
Occupied and reachable are reported separately on purpose: "a million live
iterators" and "a million dead iterators still holding slots" call for opposite
fixes. Reachability is only meaningful in the post-sweep report -- it means
"carries the current mark", so at exit, long after the last cycle, almost
everything reads as unreachable whether it is or not. That trap is real: the exit
report says 6% reachable and the last post-sweep says 75%.
What it says about a self-hosting translation, all of it recorded in
vm/selfhost/README.md:
- The heap is genuinely live, not uncollected garbage: 295MB occupied and 75%
reachable at the last sweep. The run then ends at 769MB because only three or
four cycles complete in 1.4s while the mark thread sits at 97% CPU.
- Collecting harder does not fix it. -DCN1_GC_MARK_THREADS=4 more than doubles
the cycles (4 -> 9) and is slightly faster, but peak moves 1256MB -> 1243MB.
- vmmap puts essentially all of the process in malloc'd heap: MALLOC_LARGE
435.8MB dirty (the BiBOP arenas), MALLOC_SMALL 111.7MB, and 50.2MB of
MALLOC_LARGE (empty) -- freed but not returned. An earlier claim in the README
that ~600MB was "not the Java heap" was wrong; it compared an exit-time census
against the whole-run peak.
Two leads it opens without settling: per-object width against the JDK (String 73B
here against ~32B there), and 295,907 SimpleListIterator objects reading 72%
reachable at a fresh sweep, which a stack-local iterator should never be --
conservative stack scanning is on unconditionally and would explain it, but that
is a hypothesis and not yet measured.
Compile-gated on CN1_ALLOC_CENSUS and run-gated on CN1_HEAP_REPORT, so an ordinary
build is untouched. Gates A and D still byte-identical.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…riments
The live census reported "reachable", which conflated two different things and got
the answer backwards. Read post-sweep, an object stamped live by the sweep's grace
rule is indistinguishable from one the mark actually traced -- so a heap full of
fresh garbage read as a heap full of live data.
The census now runs PRE-sweep, the only point where the four reasons a slot is
still occupied are still distinguishable, and reports all four per class: traced
(the mark reached it), fresh (allocated since the mark, kept by grace), aging
(known dead, kept one more cycle) and dead (this sweep returns it).
On a self-hosting translation, at the last cycle:
occupied 4,441,347 objects 349MB
traced 47% fresh 30% aging 14% dead 9%
Only 47% of the occupied heap is traced live; the rest is held by collector policy
rather than by the program. Per class it is sharper -- char[] is 5% traced and 76%
fresh, almost pure churn caught between cycles.
experiments/PinProbe establishes the mechanism directly: three arms allocate and
drop 200,000 objects each -- shallow, under a 400-deep recursion, and with the
stack scrubbed -- and a dead object needs THREE cycles to have its slot returned
(grace while fresh, then aging, then reclamation). A translation completes three or
four cycles in 1.4s, so most of what it allocates is never eligible to be freed and
the heap grows towards total allocation volume: 940MB allocated, 1.3GB peak,
150-300MB genuinely live.
The same probe rules OUT the hypothesis it was built to test. Conservative stack
scanning does not pin dead objects: the marks are precise and all three arms behave
identically, so depth and stale stack words make no difference. Fragmentation is
ruled out too, at ~2MB of page-pool slack in 715MB.
Collecting faster helps but does not change the ratio, since the grace rule keeps
everything allocated since the last mark whatever the rate:
1 mark thread 3 cycles peak 1320MB
-DCN1_GC_MARK_THREADS=4 8 cycles peak 1172MB
4 threads + CN1_GC_TRIGGER_MB=24 7 cycles peak 1259MB
So the dominant lever is allocation churn, and cutting one allocation removes about
three cycles of occupancy rather than one object. The [ALLOC] census names where it
is: char[] 368MB, Object[] 196MB, String 77MB, SimpleListIterator 40MB.
Gates A and D still byte-identical; the census is compile-gated on CN1_ALLOC_CENSUS
and run-gated on CN1_HEAP_REPORT, so an ordinary build is untouched.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A dead object needs three GC cycles to have its slot returned, because the sweep keeps it twice: once as fresh (never marked) and once as aging (mark == V-1, not traced this cycle). The first is load-bearing. The second is not obviously anything, and the history says what it is -- November 2014, 31528ec: - if(o->__codenameOneGcMark != currentGcMarkValue) { + if(o->__codenameOneGcMark < currentGcMarkValue - 1) { with the message "Delayed GCing of elements to prevent them from being collected due to a race condition with the GC thread". That collector had NO SATB barrier -- zero matches for satb or snapshot in the file at that commit -- so a mutator could hide a reference from the mark, and keeping an extra generation made the resulting lost-object race improbable rather than impossible. It has been inherited ever since, including by the BiBOP sweep, without a rationale anywhere in the tree. The cases that would need it today have their own guards. A page missing from the page index for one cycle is covered by the fresh grace rule, since its objects are mark == -1; the repeated miss that aging could not save either is exactly what cn1GcPageIndexStale skips the whole reclaim for, and that comment says so. CN1_GC_NO_AGING compiles the second cycle out. Evidence: - run-gc-verify.sh GREEN, and its three self-tests still detect their injected faults -- including the injected EARLY-FREE fault, which is precisely the failure this change could cause, so the gate is not vacuous for it - run-gauntlet.sh GREEN: 12 torture suites byte-identical to the host JVM, plus GC stress in cooperative and forced-signal thread-stop modes - self-hosting gates A and D byte-identical over 793 files - peak footprint 1334 -> 1322 MB and 1349 -> 1302 MB, about 2-3% Deliberately NOT the default. The win here is small because aging is only 14-16% of the occupied heap while fresh is 26-36%, so removing the second cycle moves those objects one cycle earlier in a run that only has three or four; a long-running application whose heap reaches a steady state would see closer to the full 15%. And vm/CLAUDE.md is explicit that a green verifier is necessary rather than sufficient around the SATB window: it could not open the residual window even with the barrier deliberately compiled out. Also recorded, since dropping it for the non-GUI Apple targets is an obvious thing to try: String's `long nsString` costs nothing to keep. sizeof(obj__java_lang_String) is 48 with it and really does fall to 40 without, but BiBOP's size classes are 32/48/64 so both land in the same 48-byte slot. Three runs each way put peak at 1240/1379/1340 MB with the field and 1290/1345/1341 MB without -- ranges that overlap completely. There is no effect to find, and removing it would cost the Apple targets a side table and a lookup for nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…rth 2-3% Backs out d12892d entirely -- both the CN1_GC_NO_AGING switch and the change behind it. Keeping a compile switch for this was the wrong shape regardless: it is not debug code, so it would just be a second collector policy nobody runs. The history in that commit still stands: the second grace cycle is a 2014 pre-SATB workaround (31528ec, "Delayed GCing of elements to prevent them from being collected due to a race condition with the GC thread") and nothing in the tree records a reason for it. What the switch missed is that four later mechanisms have since been built ON the rule: - two java.lang.ref clearing sites that must use EXACTLY the sweep's liveness test; their comment spells out the failure as "FAILING to clear one the sweep frees hands get() a dangling pointer" - the fast-sweep page shortcut, whose gcGraceEpoch < V-1 bound is derived from the per-slot rule, and whose comment records issue 5425 when the two disagreed: "testing != V let it drop whole pages holding V-1 slots... 26,924 slots in one run. That is what left kept objects pointing into reclaimed memory" - the legacy and BiBOP sweeps ageing in step, so a matured Hashtable.Entry at V-1 is never kept while its page-resident payload at V-1 has already been freed The measurement that made it look safe was itself inconsistent: it changed the sweep and left the ref-clearing sites on the old rule, which IS the dangling-get() bug, and run-gc-verify.sh still came back green. So the verifier does not cover this coupling and a green result there was never sufficient evidence. Removing the rule properly means changing all four together and re-deriving the fast-sweep bound, for a measured 2-3% of peak. Not worth it; the allocation churn is where the memory is. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…n it Companion to the revert. The rule is vestigial in origin -- a 2014 pre-SATB workaround -- but the java.lang.ref clearing sites, the fast-sweep page bound and the legacy/BiBOP pairing have all been built on it since, and issue 5425 is what happened when two of them disagreed. Measured upside for removing it was 2-3% of peak. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Translating ByteCodeTranslator with itself turns a 37.6k-line real program into
a VM conformance test: the emitted C is a byte-exact expected value that costs
nothing to maintain, because it is whatever the JVM produced from the same
inputs. A defect that changes behaviour rather than crashing shows up as a diff
instead of passing silently. `.github/workflows/parparvm-selfhost.yml` runs it
nightly, on demand, and on a PR carrying the `selfhost` label.
The gates found a real VM bug and the profiler found several API implementations
that were slow for reasons that had nothing to do with the VM.
CORRECTNESS
Every generated __STATIC_INITIALIZER_X was textbook-broken double-checked
locking: __X_LOADED__ read with a plain load, written with a plain store OUTSIDE
the monitor, and class__X.initialized read the same way by inline guards that
skip the initialiser entirely and so never take the monitor. On arm64 a second
thread can observe the flag set while the stores that filled the vtable and the
classToInterfaceMap rows are still invisible, and then dispatch through a NULL
row. Observed as three identical SIGSEGVs at
classToInterfaceMap_java_util_NavigableMap[classId] + 0x8, reached from
TreeSet.clear; the translator is single-threaded in its own code but shares the
process with the GC thread, which also runs Java and so also runs initialisers.
Both flags are now release-stored and acquire-loaded, across all 391 classes.
The interface maps are calloc'd rather than malloc'd so a class id with no row
reads NULL instead of whatever the allocator last left there.
THROUGHPUT (5782-class corpus, min of 3 interleaved reps, phys_footprint)
GC run-ahead ceiling in cn1BibopPacingCap. The cap was a fraction of AVAILABLE
MACHINE RAM, and the trigger-derived clamp sat at 192MB (24MB x 8) for most of
a run, so the mutator parked on a cycle it could not help finish. Bounded from
both ends near 1GB, where the benefit saturates: 62.8s -> 27.4s, kernel time
33.4s -> 9.9s. Proportionate, so a phone or container is unaffected.
Class-init checks were unconditional CALLS at 74% of 2116 sites; the callee's
own first line already returns when the flag is set. Inline-guarded now that
the flag has acquire/release: 7.21% -> 0.12% of mutator self-time.
javac's `a + b` StringBuilder idiom is lowered to String.cn1ConcatN, the fused
path invokedynamic concat already used. Two allocations and no byte<->char
conversion against the builder's four plus two conversions. Only JDK 9+ emits
the indy form, so everything built at source 8 -- the core, every port, every
cn1lib -- reached none of it. 898 chains fused, StringBuilder allocation sites
3519 -> 2041.
API IMPLEMENTATION
AbstractList.SimpleListIterator.next had a try/catch per element to turn one
exception into another. ParparVM has no zero-cost exception tables, so that is
a setjmp per element in the hottest loop in the program, on top of virtual
size() and get() calls and an index recomputed as size() - numLeft.
ArrayList now has a direct-array iterator: iteration path 25.5% -> 12.4% of
mutator self-time, ArrayList.get 7.42% -> 0.55%, _setjmp to zero.
IdentityHashMap's iterator reached checkConcurrentMod() and hasNext() through
two more non-inlined calls per element, making four with the interface
dispatches. Inlined: checkConcurrentMod 2.69% -> 0.00%.
StringBuilder grew by 1.5x (inherited from Harmony) where OpenJDK doubles.
Growing to N chars costs N*r/(r-1) in abandoned arrays: 3N against 2N.
String.equals and String.compareTo had their fast path INVERTED -- memcmp only
when BOTH strings were UTF-16, the rare case, while two compact ASCII strings
took a per-character loop calling a helper that re-derived the base pointer and
re-tested the backing array's class every character. Corrected. Measured no
improvement: the cost there is call overhead, not the comparison. Kept because
the old structure was backwards, not because it is a win.
ByteCodeClass.generateCCode built each file in a fresh StringBuilder, and 95
copies of x.replace('/','_').replace('$','_') re-mangled the same owner per
emitted instruction. Reused buffer and a memo: char[] 1530MB -> 837MB.
ALSO
BytecodeInstructionIntegrationTest reflected on readFileAsStringBuilder, which
no longer exists: replaceInFile works on a String since the translator had to
compile against ParparVM's own JavaAPI, whose StringBuilder has no
indexOf/replace. Pointed at readFileAsString. 45/45 green.
VERIFIED
Gate D, Gate A and the negative control pass on the 797-file self-hosting
corpus, byte-identical. GC heap verifier clean over ~1e9 references.
check-cast-semantics and check-native-signatures clean.
KNOWN, NOT FIXED
A rare (~1/14) crash remains in interface dispatch, now a clean NULL-row fault
rather than silent garbage because of the calloc above. It is a bad class id
reaching a registered-row lookup, it predates this branch as far as the
evidence goes, and it is not reproducible under a debugger. Tracked separately.
The remaining iterator cost is two interface dispatches per element, each a
four-load pointer chase. Removing those means not making the calls -- lowering
for-each to an indexed loop in the translator -- which is a control-flow
rewrite and is deliberately left for its own change.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 118044dc12
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Native Windows port (x64)Compared 166 screenshots: 155 matched, 11 missing actuals.
Benchmark ResultsDetailed Performance Metrics
|
…, keep the pacing bound off placeholder hosts
Three defects, two of them found by review.
CLASS-INIT GUARDS TESTED THE WRONG FLAG (P1, review)
class__X.initialized is stored BEFORE __CLINIT__ runs, because it doubles as the
recursion guard for a <clinit> that touches its own statics. A guard on it can
therefore skip the initializer while <clinit> is still executing and hand back a
default for a static field that has not been assigned yet -- and the guards were
added to the static accessors, which is exactly where that is observable.
__X_LOADED__ is stored after __CLINIT__ returns and is the only flag meaning
"finished". Every guard emitted from ByteCodeClass and BytecodeMethod now tests
it. It is file-local, so generateCCode forward-declares it above the accessors;
the initializer block later in the same file is the definition.
The two guards emitted from TypeInstruction and FusedConstructor still test
initialized: they name a DIFFERENT class, whose flag is not visible from the
emitting translation unit. That is pre-existing, it is now written down where the
guard is emitted, and closing it needs a globally visible completion flag on
struct clazz.
THE CONCAT MATCHER TRUSTED THE OWNER, NOT THE STACK (P1, review)
It recognised appends by owner rather than by tracking which object was on the
stack, so it accepted
new StringBuilder(); POP; return existing.append(a).append(b).toString();
-- valid bytecode -- and took the appends on `existing` for appends on the
builder it had just allocated. Removing the allocation and the appends would then
leave the POP: an operand-stack underflow and a concat of the wrong operands.
The whole DUP/POP/SWAP family now ends the chain. Refused rather than reasoned
about, because a missed fusion is slower and a wrong one is memory corruption.
Cost: one site out of 898.
THE RUN-AHEAD BOUND SCALED OFF A NUMBER THAT IS NOT A MEASUREMENT (CI)
cn1_available_memory answers a flat 100MB wherever it cannot measure -- Linux,
Windows, the non-Apple fallback. cn1PacingGrowthFloorBytes only ever RAISES its
floor from that value, so a placeholder host is bit-for-bit unchanged. The new
run-ahead bound only ever LOWERS the cap, so scaling it by the placeholder
tightened pacing on precisely the hosts we know nothing about.
BibopPageFloorIntegrationTest went red on arm64 Linux, where fm/8 is 12.5MB,
while the same code passed on macOS where fm is real.
The bound now applies only where fm is a genuine reading, and answers 0 --
"leave the cap alone" -- elsewhere. macOS keeps the measured win: 25.1s.
ALSO
ArrayList.java joins the copyright exclusions as Apache Harmony source retaining
its Apache-2.0 notice, beside the other Harmony files. Swapping in the Codename
One header, which is what the gate was asking for, would have relicensed
third-party code. The three genuinely new files got the real header.
VERIFIED
Gate D, Gate A and the negative control pass byte-identical on 797 files.
BibopPageFloor and GcHeapIntegrity green. Copyright, control-character and
ASCII gates clean.
✅ Continuous Quality ReportTest & Coverage
Static Analysis
Generated automatically by the PR CI workflow. |
Cloudflare Preview
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 01643b78c9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (SB.equals(ci.getOwner())) { | ||
| if ("append".equals(ci.getName()) && APPEND_STR.equals(ci.getDesc())) { | ||
| appends.add(Integer.valueOf(j)); |
There was a problem hiding this comment.
Fresh evidence in the current code is that rejecting stack-shuffle opcodes still permits ordinary bytecode such as consume(new StringBuilder(), existing.append(a).append(b).toString()): the first argument's new builder remains below existing, and this owner-only check mistakes the calls on existing for calls on that allocation. The rewrite then deletes the first argument and the append calls, so consume receives existing and concat(a, b) instead of the new builder and existing's full appended text. Track the receiver identity rather than accepting every later StringBuilder invocation.
Useful? React with 👍 / 👎.
| const unsigned char* data = cn1FindResource(n, &len); | ||
| if(data == 0 || len <= 0) { | ||
| return JAVA_NULL; |
There was a problem hiding this comment.
On a normal Linux or Windows translated application, SomeClass.class.getResourceAsStream("/theme.res") still returns null even though this commit embeds that resource. This native calls cn1FindResource, but the repo-wide generated tables define cn1LinuxFindResource and cn1WinFindResourceId; the only definition of cn1FindResource is the weak null-returning fallback immediately above. The port-specific Implementation.getResourceAsStream methods use their tables, but direct Class.getResourceAsStream calls do not, so the new Java API path needs an adapter or matching generated symbol.
Useful? React with 👍 / 👎.
Native Windows port (arm64)Compared 166 screenshots: 145 matched, 21 missing actuals.
Benchmark ResultsDetailed Performance Metrics
|
|
Compared 181 screenshots: 181 matched. |
✅ ByteCodeTranslator Quality ReportTest & Coverage
Benchmark Results
Static Analysis
Generated automatically by the PR CI workflow. |
|
Compared 160 screenshots: 160 matched. Benchmark Results
Detailed Performance Metrics
|
…s native handleAppleOutput copied the runtime native to srcRoot/java_io_File.m and then called Parser.writeOutput, which on an Apple target emits one <class>.m per surviving class. A retained java.io.File therefore lands on exactly the name the copy had just used and CLOBBERS the native. What is left declares exists() calling existsImpl and has no definition of it, so the link fails: Undefined symbols: _java_io_File_existsImpl___java_lang_String_R_boolean, referenced from _java_io_File_exists___R_boolean in java_io_File.o on build-ios, build-ios-tv and build-ios-metal. build-macos passed in the same run, which is what identified the mechanism: MacOSNativeBuilder sets -DconcatenateFiles=true, routing class output into one buffer so the colliding name is never written. IPhoneBuilder sets it only under ios.superfastBuild, so the collision is live by default there. The comment in MacOSNativeBuilder has described this hazard for as long as that flag has been passed; the flag hides it rather than fixing it. The clean target already writes the same resource as java_io_File_runtime.c precisely so the two can coexist. This does the same on the Apple path. The generated class keeps java_io_File.m; the native becomes java_io_File_runtime.m; both are compiled, and the symbols resolve. Nothing else needed changing. The Xcode project collects sources by extension rather than from a fixed list, so the renamed file is picked up. And NativeSignatureVerifier reads the RESOURCE "/java_io_File.m" off the classpath, not this output path, so its scan is unaffected -- deliberately, per the comment on bundledRuntimeSources. VERIFIED Translating for the ios target now emits BOTH java_io_File.m (the generated class, referencing existsImpl) and java_io_File_runtime.m (the native, defining it); before, only the former survived. Gate D, Gate A and the negative control still pass byte-identical on the 797-file self-hosting corpus.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a57b3ecdbc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| String className = getName(); | ||
| int lastDot = className.lastIndexOf('.'); | ||
| absolute = lastDot < 0 ? "/" + name | ||
| : "/" + className.substring(0, lastDot).replace('.', '/') + "/" + name; |
There was a problem hiding this comment.
For nested classes or classes whose names contain underscores, getName() cannot identify the package correctly because generated class metadata converts both /, $, and _ into dots (for example, p/Outer$Inner becomes p.Outer.Inner). Consequently, Outer.Inner.class.getResourceAsStream("file.txt") searches /p/Outer/file.txt instead of /p/file.txt and returns null despite the resource being present. Derive the package from preserved binary-name metadata rather than the lossy generated class name.
Useful? React with 👍 / 👎.
The guards tested __X_LOADED__, which is the only flag that means "<clinit> finished" -- that part was right, and the review comment that prompted it stands. What is wrong is what happens when a class never reaches the store. The initializer returns early, WITHOUT setting __X_LOADED__, whenever it finds class__X.initialized already true: the re-entrant case, and the case where another thread is mid-<clinit>. It also never reaches the store if __CLINIT__ throws. Any class left in that state has __X_LOADED__ == 0 permanently, and with the guards in place EVERY subsequent static access and every allocation calls the initializer, takes the class monitor, finds initialized true and returns. Not a hang -- a monitor acquire on a path that used to be a predicted-not-taken load. MEASURED: the hello screenshot suite stops after 145 of 166 screenshots on Linux, in three runs across x64 and musl, at 699s and 1052s elapsed, with no crash, no OOM and no bad_alloc in the log. A passing master run reports CN1_HELLO_SUITE_PNGS=166. The stop lands in a different test each time but always at the same count, which is what a uniform slowdown looks like rather than a hang at one place. I had previously attributed those failures to the pre-existing flake in that workflow. That was wrong: the flake is real and does hit other branches, but it stops at a different count (82 on master), and matching on the symptom string hid a regression of my own. So the guards come out. What stays is the fix they were built on top of, which is independent and still wanted: __X_LOADED__ and class__X.initialized are release-stored and acquire-loaded in all 391 classes, and the interface maps are calloc'd. Those close the double-checked-initialization race that produced the SIGSEGV at classToInterfaceMap_java_util_NavigableMap[classId] + 0x8. The 7.2% of mutator self-time the guards were worth needs a design that cannot leave the flag unset -- a third state, or setting it on the already-initialized path once "another thread finished" can be told apart from "this thread is re-entrant". That belongs in its own change, with the suite as its gate. VERIFIED 0 inline guards emitted; 391 acquire fast paths and 391 release stores retained; 39 interface maps still calloc'd. Gate D, Gate A and the negative control pass byte-identical on the 797-file self-hosting corpus.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b236ee39c2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| public static java.lang.Class forName(java.lang.String className, boolean initialize, | ||
| ClassLoader loader) throws java.lang.ClassNotFoundException { | ||
| return forName(className); |
There was a problem hiding this comment.
When callers use Class.forName(name, true, loader) to trigger a static registration block, delegating to the one-argument implementation does not initialize the named class: forNameImpl only searches classesList and returns its descriptor. AOT linking does not make this safe because ParparVM still runs generated class initializers lazily, so frameworks relying on the true overload can continue before required static state exists. Explicitly invoke the target's initializer when initialize is true.
Useful? React with 👍 / 👎.
| .isArray = JAVA_FALSE, \ | ||
| .dimensions = 0, \ | ||
| .arrayType = 0, \ | ||
| .primitiveType = JAVA_TRUE, \ | ||
| .baseClass = 0, \ |
There was a problem hiding this comment.
For calls such as Integer.TYPE.newInstance(), these new primitive descriptors have a zero-initialized newInstanceFp, but java_lang_Class_newInstanceImpl___R_java_lang_Object unconditionally calls that pointer. Instead of returning null so the Java wrapper throws InstantiationException, a native target jumps through address zero and crashes. Special-case primitiveType in the native implementation, as the JavaScript runtime already does.
Useful? React with 👍 / 👎.
Two independent fixes in the allocation-pacing and collection paths.
cn1PacingPark tested threadBlockedByGC, then ran a mutator assist, then
looped with a bare `continue`. The assist can take a long time, and a GC
that requests a stop while it is running found the thread still marked
active with no safepoint ahead of it on that path -- so the collector
waited out its 250ms timeout and force-stopped the thread instead. That
is what the iOS packaging leg was reporting: the screenshot suite ran to
completion and then could not emit SUITE:FINISHED, with
[GC] force-stopped thread 3 after 250000us at a safepoint it never
reached (2 so far) ... (16 so far)
and the child finally killed with SIGTERM. Re-check the flag once the
assist returns and park properly if it is set.
Separately, ArrayListIterator was declared private. A private inner class
whose constructor is reached from the outer class makes javac synthesise
an access bridge and an ArrayList$1 marker type, so every iterator() paid
an extra class plus an aconst_null for the bridge argument. Package
private is invisible outside java.util either way. The self-hosting
corpus drops from 797 emitted files to 795 -- the .c/.h pair for the
synthetic that no longer exists.
Gates D and A stay byte-identical over the 795-file corpus, with the
negative control still detecting an injected corruption.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 28e62d6692
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| absolute = lastDot < 0 ? "/" + name | ||
| : "/" + className.substring(0, lastDot).replace('.', '/') + "/" + name; | ||
| } | ||
| byte[] embedded = cn1EmbeddedResource(absolute); |
There was a problem hiding this comment.
When a JavaScript-targeted application calls Class.getResourceAsStream, this new call resolves to an uncategorized native because cn1EmbeddedResource is neither bound in parparvm_runtime.js nor listed in JavascriptNativeRegistry; the generated stub therefore throws Missing javascript native method instead of preserving the previous null result. Even a null-returning binding would then reach System.getenv, which the registry explicitly marks unsupported for JavaScript, so this path needs a JavaScript resource implementation or a backend-safe null fallback.
Useful? React with 👍 / 👎.
| if (!name) { | ||
| throw new Error("getPrimitiveClass: unknown primitive type code " + typeCode); | ||
| } | ||
| return classObjectForName(name); |
There was a problem hiding this comment.
When the JavaScript target initializes any newly fixed wrapper TYPE field through this binding, the returned class object is later handled by the existing Class.getName native via descriptorClassName, which maps JAVA_INT to "I", JAVA_VOID to "V", and so on. Consequently calls such as Integer.TYPE.getName() and Void.TYPE.getName() now return JVM descriptor letters instead of the required "int" and "void", unlike the native implementation and the new primitive integration expectations; scalar primitive getName should use PRIMITIVE_INFO.javaName while array descriptors retain the descriptor mapping.
Useful? React with 👍 / 👎.
A for-each compiles to Iterator.hasNext()/next() through INVOKEINTERFACE, the most expensive dispatch this VM has -- a lookup in the owning class's interface map before the vtable read -- and it runs twice per element. Neither the emitter's closed-world devirtualization nor ThinLTO can touch it, because both start from a concrete owner and an interface call has none: java.util.Iterator has 27 implementors here. The concrete type is recoverable locally even though the translator has no general stack-type inference. When the collection's iterator() has exactly one reachable implementation, and that implementation's whole body is `return new T(...)`, the object the following ASTORE writes is a T -- no inference required. Retyping the Iterator calls on that local to INVOKEVIRTUAL on T is then enough by itself: the existing devirt in Invoke.appendInstruction carries any virtual call with no reachable override the rest of the way to a direct call, which ThinLTO can inline. Soundness rests on the local being assigned exactly once. A slot written twice could hold an iterator of another class at the same ALOAD, and a virtual call on the wrong class reads fields out of an object that does not have them -- silent here, since ParparVM's CHECKCAST is unchecked. allocatedReturnType() is deliberately strict for the same reason: a body that could return an object it did not just allocate is rejected rather than guessed at. Like the concat fusion, the pass runs before the unused-method cull so its new edges exist while reachability is computed. Yield is bounded by what the static types admit, and it is worth being explicit about: over the translator's own corpus, 294 for-each sites, 11 resolve, 7 lower. 190 are java/util/List and 74 more are Set or Collection -- all interface-typed receivers, which closed-world analysis cannot reduce to one implementation. Reaching those needs a runtime guarded specialization, which is a separate and much larger change. This pass is the part that is provable, and it fires for any collection whose iterator() is monomorphic rather than for a hard-coded list of types. Also guard gate A against a stale JVM side. It runs ByteCodeTranslator/target/classes, which neither build-selfhost.sh nor verify-selfhost.sh builds, so a translator edit that has not been through `mvn package` makes the gate compare the new translator against the old one and report the intended change as a VM divergence -- which is exactly what it did here, pointing convincingly at java_util_ArrayDeque.c. Maven is no help: it answered "Nothing to compile - all classes are up to date" for a source three hours newer than its class, so the guard compares the trees itself. CI was never affected; it runs mvn package first. Gates D and A byte-identical over 795 files, negative control still detecting an injected corruption, and vm/tests 562 passed 0 failed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f0718e6339
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| int slot = ((VarOp) store).getIndex(); | ||
| if (countStoresTo(slot) != 1) { | ||
| continue; | ||
| } | ||
| retypeIteratorUses(slot, itType); |
There was a problem hiding this comment.
For valid bytecode where this local slot initially contains an Iterator method parameter and is later reused by the single ASTORE for a concrete collection iterator, countStoresTo(slot) == 1 accepts the optimization even though the slot has two values. retypeIteratorUses() then scans the entire method and rewrites interface calls occurring before the store as calls on itType; if the parameter is another iterator implementation, generated C dispatches to methods that read the wrong object layout. Restrict rewriting to uses dominated by this store, or track the stored receiver's identity.
Useful? React with 👍 / 👎.
| String relative = absolute.substring(1); | ||
| int from = 0; | ||
| while (from <= path.length()) { | ||
| int end = path.indexOf(java.io.File.pathSeparatorChar, from); | ||
| String root = end < 0 ? path.substring(from) : path.substring(from, end); | ||
| if (root.length() > 0) { | ||
| java.io.File candidate = new java.io.File(root, relative); |
There was a problem hiding this comment.
When CN1_RESOURCE_PATH is set, an absolute resource name such as /../secret leaves relative as ../secret, and new File(root, relative) allows the subsequent FileInputStream to traverse outside the configured resource root. Thus code that passes a user-controlled resource name can read any process-accessible file instead of only resources under the search path; normalize/canonicalize the candidate and reject it unless it remains beneath the canonical root.
Useful? React with 👍 / 👎.
|
Compared 217 screenshots: 217 matched. |
|
Compared 144 screenshots: 144 matched. |
The suite stops after exactly 145 of 166 screenshots on every target
except glibc-x64 -- arm64, musl and Windows x64/arm64 all short by the
same 21 -- and master is green on the same jobs (10 consecutive runs of
the Windows leg). The app log shows what stops it, and it is not a clean
failure:
AIOOBE 89 at Display.callSeriallyOnIdle:1174 (pendingIdleSerialCalls.add)
AIOOBE -1 at Display.edtLoopImpl:1813
NPE at java_util_ArrayList.get:443
get() is array[firstIndex + location], so an NPE there means the backing
array reference itself is null. That is corrupt list state, not a bounds
mistake.
Two things are ruled out rather than assumed. The list logic is correct:
a differential fuzz of this exact source against java.util.ArrayList ran
3000 seeds x 200 random add/insert/remove/set/clear/trim/ensureCapacity/
iterate/iterator-remove/addAll operations with no divergence. And there
is no Java-level race: every access to pendingIdleSerialCalls in Display
is inside synchronized(lock). The GC's lifetime rules are also unchanged
by this PR -- the cn1_globals.m diff is 433 added lines of census behind
CN1_ALLOC_CENSUS and three removed pacing lines.
So the corruption is below Java, and the change that most alters what
the runtime's most-allocated class does is withdrawn here: the lazy
default-capacity allocation replaced an eager new Object[10] with a
process-wide SHARED static zero-length array, which also gave
java.util.ArrayList a <clinit> it had never had (master's only static is
a compile-time serialVersionUID, so no static initializer was emitted at
all). Verified back to zero <clinit> after this change. The generated
initializer sets class.initialized before running __CLINIT__, so a class
that newly acquires one newly acquires that window too.
This is an isolating experiment, not a proven mechanism: if the suite
goes green the cause is established, and if it does not, a major suspect
is eliminated and the run now produces evidence. The iterator -- the
change that carried the measured win, iteration 25.5% -> 12.4% of
mutator self-time -- is kept.
The evidence half, because the last round produced none:
- The musl leg had NO crash wiring whatsoever, so a suite that died
mid-run was indistinguishable from one that hung. It now enables core
dumps (core_pattern is global to the host kernel, but the kernel writes
the file in the crashing process's mount namespace, so an absolute path
under the bind-mounted /cn1 lands in the uploaded workspace), installs
gdb, post-mortems any core, and states explicitly when no core was
written.
- Both legs now ship the UNSTRIPPED binary beside the logs. A core or a
raw backtrace is addresses and nothing else once the runner is gone.
- The live-stack dump swallowed an absent gdb through a catch-all and
left hang-stacks.txt holding nothing but its sample headers -- which
reads as "we sampled and all was well". It now records why it could not
collect, and the gdb install is verified rather than silenced.
Gates D and A stay byte-identical over 795 files with the negative
control still detecting an injected corruption.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 68503d5f1b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| .primitiveType = JAVA_TRUE, \ | ||
| .baseClass = 0, \ | ||
| .baseInterfaces = EMPTY_INTERFACES, \ | ||
| .baseInterfaceCount = 0, \ | ||
| .initialized = JAVA_TRUE \ |
There was a problem hiding this comment.
When translated code calls Array.newInstance(Integer.TYPE, 4) (equivalently, Array.newInstance(int.class, 4)), this designated initializer leaves the trailing arrayClass field zero. java_lang_reflect_Array_newInstanceImpl explicitly rejects that value at nativeMethods.m:643-648, so every reflective primitive-array allocation now throws RuntimeException instead of returning an int[]. Populate each primitive descriptor with its matching class_array1__JAVA_* metadata and ensure the reflective allocator uses the primitive element width rather than sizeof(JAVA_OBJECT).
Useful? React with 👍 / 👎.
… unrooted
The core dump the previous commit's wiring collected names the fault
exactly:
#0 cn1_set_array_element_int cn1_globals.h:2537
((JAVA_ARRAY_INT*)(*(JAVA_ARRAY)array).data)[index] = value;
#1 com_codename1_ui_Display_edtLoopImpl__ Display.c:4072
#2 com_codename1_ui_Display_mainEDTLoop__
Display.c:4072 is Display.java:1813,
actualStack[actualStack.length - 1] = Integer.MAX_VALUE;
which is also the line the Windows run reported as AIOOBE -1. Index -1
means actualStack.length read 0. That array is inputEventStackTmp, which
is new int[1000] and is only ever swapped with another int[1000] or a
new int[qt.length] -- a zero length is unreachable in correct Java, so
the array header itself was clobbered. The -1 write is the consequence,
not the cause, and on Linux -O3 (unchecked stores) it segfaults instead
of throwing.
The cause is in this PR's own fused concat natives. cn1FusedConcat2..5
take raw interior pointers into the source Strings' byte[]s, and hold
those plus the JAVA_OBJECT arguments in plain C locals, ACROSS
cn1FusedLatin1Begin -- which allocates, and therefore can collect. None
of those are roots: CN1_CONSERVATIVE_GC_ROOTS is defined by no build in
this tree (it survives only in one comment), so the native stack is not
scanned and enteringNativeAllocations() is live rather than a no-op. A
collection inside that allocation can reclaim the byte[]s the copy loop
then reads, and the freed block can be handed to another allocation
while the loop is still walking it -- which is how an unrelated int[]
ends up with a zeroed header.
Every other native allocation path in this file brackets for exactly
this reason, including cn1ConcatFallback and newStringFromAsciiLen two
functions above. The fused concat was the one that did not. The bracket
now spans the whole body, since the references are held across both the
fused attempt and the fallback, and cn1ConcatFallback no longer sets it
itself -- doing so would clear it on return while its caller still holds
those pointers.
How the alternatives were eliminated rather than assumed: the ArrayList
logic is correct (3000 seeds x 200 random operations differentially
fuzzed against java.util.ArrayList, no divergence); there is no
Java-level race (every access to the corrupted list and to the event
stack is inside synchronized(lock)); the GC's lifetime rules are
unchanged by this PR (433 added lines in cn1_globals.m are census behind
CN1_ALLOC_CENSUS, the 3 removed lines are pacing); the classId == -1
primitive-class sentinel only reaches an array subscript under
DEBUG_GC_OBJECTS_IN_HEAP; and withdrawing the lazy ArrayList allocation
in the previous commit did not fix it.
Gates D and A stay byte-identical over 795 files -- a corpus that builds
strings constantly -- with the negative control still detecting an
injected corruption.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2ad038a claimed the fused concat natives allocate with their source Strings unrooted, on the grounds that CN1_CONSERVATIVE_GC_ROOTS "is defined by no build in this tree". That is false. cn1_globals.h:72 does #ifndef CN1_DISABLE_CONSERVATIVE_GC_ROOTS #define CN1_CONSERVATIVE_GC_ROOTS #endif so conservative roots are ON by default, exactly as vm/CLAUDE.md says in its first paragraph on tagged immediates. I reached the opposite conclusion by grepping for the symbol with .h files EXCLUDED, which filtered out the one line that defines it. With conservative roots the native stack is scanned, so a, b and the interior pointers in p[] are roots like any other local, and enteringNativeAllocations() expands to do {} while(0). The commit was therefore a no-op that compiled to nothing, and its comments asserted the opposite of what the build does -- worse than useless in a file where the next reader has no PR thread to check it against. Measured rather than argued: a torture driving all four fused arities (cn1Concat2..5 all present in the generated C, ~400k calls per round) while re-verifying 256 live int[1000] arrays element by element reports corrupt=0 and the HotSpot checksum, identically with and without the bracket. So the 145-screenshot stop is still unexplained, and the suspect list is back to what it was before 2ad038a. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a5ec1879df
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| String relative = absolute.substring(1); | ||
| int from = 0; | ||
| while (from <= path.length()) { | ||
| int end = path.indexOf(java.io.File.pathSeparatorChar, from); |
There was a problem hiding this comment.
When a native Windows app sets CN1_RESOURCE_PATH to a normal absolute path such as C:\assets, this splits at the drive-letter colon because the translated JavaAPI hard-codes File.pathSeparatorChar to ':' (vm/JavaAPI/src/java/io/File.java:8). The fallback consequently searches roots such as C and \assets instead of the configured directory, so filesystem resources cannot be loaded on Windows; use Windows-aware semicolon parsing without treating drive-letter colons as separators.
Useful? React with 👍 / 👎.
| public final class Integer extends Number implements Comparable<Integer> { | ||
|
|
||
| public static final Class<Integer> TYPE = int.class; | ||
| public static final Class<Integer> TYPE = Class.getPrimitiveClass(Class.CN1_PRIM_INT); |
There was a problem hiding this comment.
Now that this line makes Integer.TYPE a real primitive class object, Integer.TYPE.getClassLoader() reaches Class.getClassLoader() (Class.java:42-44), which unconditionally returns the non-null system loader. Primitive classes must report a null class loader, and reflection code commonly uses that result to distinguish bootstrap-defined types; special-case primitive descriptors in getClassLoader().
Useful? React with 👍 / 👎.
Not a fix. The concat fusion and the iterator lowering are the only changes in this PR that rewrite bytecode, and the failure is heap corruption: an int[] whose header reads length 0, which then takes an unchecked [-1] store in Display.edtLoopImpl and segfaults at cn1_set_array_element_int under -O3. A wrong stack depth or a wrong receiver type out of a rewrite is the most plausible way to produce that, so this splits the suspect space -- green means the cause is an emitted-code rewrite, red means it is in the JavaAPI or the C runtime. Both passes get restored once the answer is in. Reaching for this because the cheap local instruments are exhausted and came back clean: - run-gauntlet.sh GREEN (MapTorture, IdmTorture, HtTorture, SbTorture, StrCmp, FusedTest, IbpTest, ExcTest, ThreadChurn, SoeTest, TaggedSync, BoxEdge, GcStress+MtStress on both stop modes). - run-gc-verify.sh GREEN, with all three injected-fault self-tests still firing. One full-gate run reported ThreadChurn as a VACUOUS failure -- 0 verify passes, the workload completing no GC cycle under load -- and it is clean with 2 verify passes when run alone, so that was the gate correctly refusing to score a run that proved nothing, not a finding. - A torture driving all four fused arities (~400k calls per round, cn1Concat2..5 all confirmed present in the generated C) while re-verifying 256 live int[1000] arrays element by element: corrupt=0. - Self-hosting gates D and A byte-identical over 795 files, vm/tests 562 passed. A local macOS reproduction is not available: build-macos-app.sh needs the workspace toolchain, and tools/env.sh points into a TMPDIR that has been reaped. setup-workspace.sh would rebuild it, but under a shared $TMPDIR/codenameone-tools path that the sibling checkouts also use, so it is not safe to run from here. Worth noting separately that build-macos-app.sh exits 0 when it finds neither Xcode nor the toolchain -- a build step that reports success having done nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7d1b503d0c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| public final class Integer extends Number implements Comparable<Integer> { | ||
|
|
||
| public static final Class<Integer> TYPE = int.class; | ||
| public static final Class<Integer> TYPE = Class.getPrimitiveClass(Class.CN1_PRIM_INT); |
There was a problem hiding this comment.
Now that this initializes TYPE to a real primitive descriptor, Integer.TYPE.toString() reaches Class.toString() (Class.java:321-323), which unconditionally returns getName() + " class"; the result is therefore "int class" (and Void.TYPE similarly returns "void class") instead of the required primitive name alone. Special-case primitive class objects when formatting them.
Useful? React with 👍 / 👎.
Not a fix, and it stacks on probe 1 (both bytecode-rewriting passes are
still off). vm/JavaAPI, cn1_globals.{m,h} and nativeMethods.m go back to
0cd4328. What remains of the PR is the translator plumbing, the
self-hosting harness and the CI work.
Probe 1 answered only half a question. build-ios-tv went from
pass=6/fail=117 to fully green with the rewrites off, but every Linux
and Windows leg failed again with the SAME signature as before --
pngs=145, stopping in AccessibilityTest (x64, musl) or
MutableImageReadbackTest (arm64). An unchanged signature under a changed
input means the rewrites are not what those legs are dying on.
That the stop point is fixed at 145 while the AIOOBE index varies (69 on
Windows, 89 on Linux arm64) says the corruption itself is
nondeterministic but its trigger is not: 145 is simply where
AccessibilityTest sits in the run order.
If this probe is green the cause is in the reverted runtime, and the
remaining candidates are small enough to bisect one at a time: the
primitive TYPE statics added to nine wrapper classes (Boolean, Short and
Float gain the field outright, and vm/CLAUDE.md notes Byte and Boolean
are deliberately NOT tagged), the IdentityHashMap iterator inlining, the
StringBuilder growth change, the String.equals/compareTo fast path, and
the pacing edits. If it is red the cause is in the translator plumbing
that this probe keeps.
Recorded so it is not re-litigated: master is green 10/10 on the Windows
leg, so that leg is a clean signal; the Linux leg has its own
pre-existing flake on master (5 failures in 20 runs) whose signature
differs run to run (pngs=136, NativeMapFallbackScreenshotTest), which is
NOT this.
JavaAPI and the translator both compile at 0 errors after the revert.
The self-hosting gates cannot run in this state -- Util's ctype map needs
the primitive TYPE statics -- and that job is label-gated and not running
on this PR anyway.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 718aa7b19b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (false) { | ||
| fuseMtd.fuseStringBuilderConcat(); | ||
| fuseMtd.lowerIteratorCalls(); | ||
| } |
There was a problem hiding this comment.
When optimization is enabled—the default—every method still reaches this literal false, so neither fuseStringBuilderConcat() nor lowerIteratorCalls() can ever execute. As a result, Java 8 string concatenations retain their allocation-heavy StringBuilder path and foreach loops retain interface dispatch, leaving the newly added rewrite code and its reported performance benefits entirely inactive; remove this bisect probe or replace it with an explicit opt-out flag.
Useful? React with 👍 / 👎.
|
Compared 166 screenshots: 166 matched. Benchmark ResultsDetailed Performance Metrics
|
|
Compared 166 screenshots: 166 matched. |
|
Compared 166 screenshots: 166 matched. |
Probe 2 answered the question it was pushed for. With vm/JavaAPI,
cn1_globals.{m,h} and nativeMethods.m back at the merge base, the legs
that had been failing all went green:
Linux native build + run (GTK3, x64 + arm64) success
ParparVM Java Tests (Windows) success
Windows cross-build + run success
The only failure was vm-tests, and that is this probe's own collateral:
PrimitiveTypeIntegrationTest is a test THIS PR adds for the primitive
class objects, and probe 2 reverted the code under it, so its
cmake --build step fails. Not a signal.
So the cause is on the runtime side, and the two bytecode-rewriting
passes are exonerated -- probe 1 had already shown that turning them off
left the Linux and Windows signature completely unchanged at pngs=145.
They are restored here, which also answers the P2 review comment on the
literal `false`: it was a deliberate, temporary bisect probe, and it is
gone.
This probe restores everything on the runtime side EXCEPT
vm/JavaAPI/src/java/util, leaving ArrayList and IdentityHashMap at the
merge base. Green means the cause is the ArrayList iterator or the
IdentityHashMap iterator inlining; red means it is in java/lang (the
primitive TYPE statics on nine wrapper classes), the primitive class
structs in cn1_globals, the String.equals/compareTo fast path, or the
pacing edits.
Restoring java/lang also puts PrimitiveTypeIntegrationTest back on its
own code, so vm-tests should go green again and stop masking the result.
Gates D and A byte-identical over 797 files (797 rather than 795 because
the ArrayList$1 access bridge exists again at the merge base), negative
control still detecting an injected corruption.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 036daff184
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if(data == 0 || len <= 0) { | ||
| return JAVA_NULL; |
There was a problem hiding this comment.
When an embedded resource exists but has zero bytes, the generated table returns its address with len == 0, and this condition treats it as absent. Consequently, Class.getResourceAsStream() returns null instead of a valid empty stream for empty packaged resources; only a null data pointer should indicate that lookup failed.
Useful? React with 👍 / 👎.
This is the defect. The specialised ArrayListIterator added in this PR
does
if (i >= size) throw new NoSuchElementException();
return array[firstIndex + i];
and the size test is only a bounds check while the list's
firstIndex + size <= array.length invariant holds. The iterator it
replaced could not read out of range at all: it went through get(),
which bounds-checks, inside a try that turned IndexOutOfBoundsException
into NoSuchElementException. Removing that try was the entire point --
ParparVM has no zero-cost exception tables, so it was a setjmp per
element in the hottest loop in the program -- but it also removed the
only bounds check on the read, and ParparVM does NOT check an array read
in a release build. A recoverable exception became an out-of-bounds read
of the heap.
OpenJDK's own ArrayList.Itr.next() carries the identical guard,
if (i >= elementData.length) throw new ConcurrentModificationException();
so omitting it is the whole bug, and restoring it costs one compare
against a hoisted local while the measured win stays.
Bisected rather than guessed, over three CI probes:
1. both bytecode-rewriting passes off -- Linux and Windows signature
completely unchanged at pngs=145, so the rewrites are exonerated
(they are restored).
2. the entire runtime side reverted to the merge base -- Linux native,
ParparVM Java Tests (Windows) and Windows cross-build all green, so
the cause is in the runtime.
3. the runtime restored EXCEPT vm/JavaAPI/src/java/util -- the Windows
cross leg went from 100+ failures to pass=185 fail=3, the three
being Media360Panorama, VRStereoScene and VideoIODecodedFrames,
which are media/VR tests that produced no output and are a separate
question.
That isolates java/util, and of the two changes there IdentityHashMap is
sound -- it hoists elementData into a local and tests p < len against
that same snapshot, which is strictly safer than what it replaced.
Why it presented as corruption rather than an exception: the read walks
off the end of the backing array into whatever object follows it in the
BiBOP page, so the damage surfaces somewhere else entirely. The core
dump named an int[] whose header read length 0 -- Display's
inputEventStackTmp, which is new int[1000] and can never legitimately be
empty -- and the unchecked [-1] store that followed segfaulted in
cn1_set_array_element_int. The AIOOBE index varied run to run (69, 89)
while the stop point did not, because 145 is simply where
AccessibilityTest sits in the run order.
3000 differential fuzz seeds against java.util.ArrayList still clean, so
the guard changes nothing for correct single-threaded use. Gates D and A
byte-identical over 795 files.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Delete stale embedded-resource artifacts
When a reused Windows output directory is rebuilt after its last classpath resource is removed, this branch writes nothing but also leaves the previous cn1_resources.rc and staged blobs in place. writeCmakeProject() then sees the stale .rc and continues compiling it, so resources removed from the application remain embedded in the executable; the analogous Linux branch leaves cn1_resources_data.S behind as well. Explicitly delete each platform's generated resource artifact when resources is empty, as emitBundledSqlite() already does for conditional outputs.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
2 participants
Rationale is in the commit message and in code comments; this body is a pointer only.
What this is. ByteCodeTranslator now translates itself, and
.github/workflows/parparvm-selfhost.ymlcompares the C it emits against the Cthe JVM-hosted translator emits from the same inputs. A 37.6k-line real program
becomes a VM conformance test whose expected value costs nothing to maintain.
Nightly,
workflow_dispatch, and opt-in on a PR via theselfhostlabel -- afull run builds the translator twice and translates the corpus several times, so
it does not belong on every PR.
A real VM bug fell out of it. Every generated
__STATIC_INITIALIZER_Xwasbroken double-checked locking --
__X_LOADED__plain-loaded and plain-storedoutside the monitor, and
class__X.initializedread the same way by inlineguards that skip the initialiser and so never take the monitor. On arm64 a
second thread can see the flag set while the vtable and
classToInterfaceMaprows are still invisible. Observed as three identical SIGSEGVs at
classToInterfaceMap_java_util_NavigableMap[classId] + 0x8fromTreeSet.clear.Acquire/release on both flags across all 391 classes; interface maps
calloc'd.Measured (5782-class corpus, min of 3 interleaved reps,
phys_footprint):62.8s -> 27.4s wall, kernel time 33.4s -> 9.9s. Class-init checks 7.21% -> 0.12%
of mutator self-time; iteration path 25.5% -> 12.4%;
checkConcurrentMod2.69% -> 0.00%;
char[]1530MB -> 837MB.Verified. Gate D, Gate A and the negative control pass byte-identical on the
797-file corpus.
vm/testsgreen.check-cast-semanticsandcheck-native-signaturesclean. GC heap verifier clean over ~1e9 references.Known, not fixed. A rare (~1/14) crash remains in interface dispatch -- a bad
class id reaching a registered-row lookup. The
callocabove makes it a cleanNULL-row fault instead of silent garbage. It predates this branch as far as the
evidence goes, and it is not reproducible under a debugger.
Deliberately not here. Lowering for-each to an indexed loop in the
translator. The remaining iterator cost is two interface dispatches per element,
each a four-load pointer chase; removing those means not making the calls, which
is a control-flow rewrite and belongs in its own change.
Not rebased on #5741. Both touch
cn1_globals.mGC policy in differentfunctions (that PR trims the SATB log's retained buffers; this one bounds
run-ahead in
cn1BibopPacingCap).