ParparVM collector: long-shift codegen fix, live-set heap goal, and an arm64 parallel-mark run

.github/workflows/parparvm-parallel-mark.yml

@@ -0,0 +1,138 @@
+# Runs the collector's own tests against a PARALLEL marker on real arm64 Linux.
+#
+# gcMarkResolveThreadCount compiles the mark pool out by default, and the comment
+# there reads as a standing verdict: arm64 Linux corrupted the heap with the pool
+# on, and "a SECOND ordering hole remains somewhere in the branch-only
+# parallel-GC work". Read in isolation that says parallel marking is broken.
+#
+# The history says something narrower. Inside PR #5327 the sequence was:
+#
+#   Jul 3  30257f5234  acquire-load the mark word in parallel marking (arm64 corruption)
+#   Jul 3  3ad67971e9  default parallel marking to serial -- "arm64 isolation experiment"
+#   Jul 4  78752a21a2  gcMarkObject must reject freed BiBOP slots
+#   Jul 5  ff7a1f1947  SATB write barrier, closing the concurrent-mark cross-thread race
+#   Jul 5  017168581c  drain grace-object subtrees before sweep (swept-while-reachable)
+#   Jul 5  60ef4c7d85  belt pass: guarantee mark-drain completeness before sweep
+#   Jul 5  4888b66cd0  stop-the-world final mark, looped to a fixpoint
+#   Jul 6  c6144beb7a  object-bearing frameless OFF: "unsound under conservative GC on arm64"
+#   Jul 10 ff93b1c415  clazz-registry invariant (arm64 SIGSEGV in cn1GcRegisterClazz)
+#
+# The experiment that produced the comment ran on Jul 3. Every mechanism that
+# makes concurrent marking sound arrived after it -- the SATB barrier most of
+# all, which did not exist when the conclusion was drawn -- and at least one
+# other arm64 heap corruptor was found and disabled on Jul 6 that had nothing to
+# do with the mark pool. Parallel marking was never re-tested at any point after
+# Jul 3, and gcMarkDrainParallel, gcMarkObject, gcMarkFlushLocal and
+# gcMarkWorklistPush have all been reworked since.
+#
+# So the comment records what was believed on one day, before the fixes, and the
+# switch under it was never revisited. This workflow is how that gets settled
+# with evidence instead of archaeology.
+#
+# The cost of leaving it alone is measured. On the GcPause benchmark -- one
+# mutator, 20M short-lived objects, a 4096-node live set -- the worst mutator
+# pause is 2.2-3.3s with one marker and 0.3-0.9s with four, against Go's 20ms on
+# the identical loop. Median and p99 already match Go at 32ns/64ns, so the single
+# marker is a large part of why the tail loses.
+#
+# It could not be reproduced on an Apple-silicon podman guest: GcPause ran clean
+# with four markers, and a multi-threaded stress with graph rewiring and
+# cross-thread resurrection ran clean too -- but that second result proves
+# nothing, because the same stress passes with the SATB barrier compiled out
+# (-DCN1_DISABLE_SATB) and under -DCN1_GC_VERIFY at 6.6M references checked. A
+# four-CPU hypervisor guest is a weak generator of store interleavings. These
+# runners are native arm64 hardware, which is where the corruption was seen.
+#
+# NOTHING HERE CHANGES A DEFAULT. The pool is switched on for this workflow only,
+# through CN1_TEST_EXTRA_CFLAGS; every other build still compiles it out.
+name: ParparVM parallel mark (arm64 Linux)
+
+on:
+  workflow_dispatch:
+  pull_request:
+    paths:
+      - 'vm/ByteCodeTranslator/src/cn1_globals.m'
+      - 'vm/ByteCodeTranslator/src/cn1_globals.h'
+      # One entry per test the matrix below runs, plus the helper that injects
+      # CN1_TEST_EXTRA_CFLAGS. Keep this list and the -Dtest list in sync: a glob
+      # is what went wrong here, because Gc*.java matched three of the six tests
+      # and silently let the other three -- and every change to the flag plumbing
+      # every arm depends on to select its marker count -- land without the
+      # workflow that exercises them ever running.
+      - 'vm/tests/src/test/java/com/codename1/tools/translator/GcHeapIntegrityIntegrationTest.java'
+      - 'vm/tests/src/test/java/com/codename1/tools/translator/GcOverflowSpiralIntegrationTest.java'
+      - 'vm/tests/src/test/java/com/codename1/tools/translator/GcUncooperativeThreadIntegrationTest.java'
+      - 'vm/tests/src/test/java/com/codename1/tools/translator/LargeArrayGcIntegrationTest.java'
+      - 'vm/tests/src/test/java/com/codename1/tools/translator/LowMemoryThrottleIntegrationTest.java'
+      - 'vm/tests/src/test/java/com/codename1/tools/translator/BibopPageFloorIntegrationTest.java'
+      - 'vm/tests/src/test/java/com/codename1/tools/translator/CompilerHelper.java'
+      - '.github/workflows/parparvm-parallel-mark.yml'
+
+concurrency:
+  group: parparvm-parallel-mark-${{ github.ref }}
+  cancel-in-progress: true
+
+jobs:
+  gc-suite:
+    name: GC suite (${{ matrix.markers }} marker(s), ${{ matrix.arch }})
+    runs-on: ${{ matrix.runner }}
+    timeout-minutes: 90
+    strategy:
+      fail-fast: false
+      matrix:
+        include:
+          # The control. A failure here is not the parallel path and the arm below
+          # cannot be read as evidence of anything until this one is green.
+          - arch: arm64
+            runner: ubuntu-24.04-arm
+            markers: 1
+            cflags: ''
+          - arch: arm64
+            runner: ubuntu-24.04-arm
+            markers: 4
+            cflags: '-DCN1_GC_MARK_THREADS=4'
+          # x64 as well, so a failure can be attributed to the architecture rather
+          # than to the pool itself.
+          - arch: x64
+            runner: ubuntu-latest
+            markers: 4
+            cflags: '-DCN1_GC_MARK_THREADS=4'
+    steps:
+      - name: Check out repository
+        uses: actions/checkout@v6
+
+      - name: Set up JDK 8
+        uses: actions/setup-java@v4
+        with:
+          distribution: temurin
+          java-version: '8'
+
+      - name: Install clang and cmake
+        run: bash scripts/ci/apt-get-install.sh clang cmake
+
+      # Package first, then test with -am: the integration tests reload the
+      # translator from its PACKAGED jar, and the reactor is what resolves the
+      # dependency. Packaging alone without -am (and without install) leaves
+      # nothing for the tests module to resolve against, which is how the first
+      # revision of this workflow failed on all three arms including the control.
+      # This mirrors parparvm-tests.yml.
+      - name: Build JavaAPI and the translator
+        run: mvn -B clean package -pl JavaAPI -am -DskipTests
+        working-directory: vm
+
+      - name: GC suite
+        env:
+          CN1_TEST_EXTRA_CFLAGS: ${{ matrix.cflags }}
+        run: |
+          mvn -B test -pl tests -am \
+            -Dsurefire.failIfNoSpecifiedTests=false \
+            -Dtest='GcHeapIntegrityIntegrationTest,GcOverflowSpiralIntegrationTest,GcUncooperativeThreadIntegrationTest,LargeArrayGcIntegrationTest,LowMemoryThrottleIntegrationTest,BibopPageFloorIntegrationTest'
+        working-directory: vm
+
+      - name: Surefire reports
+        if: always()
+        uses: actions/upload-artifact@v4
+        with:
+          name: gc-suite-${{ matrix.arch }}-${{ matrix.markers }}marker
+          path: vm/tests/target/surefire-reports/
+          if-no-files-found: ignore

vm/ByteCodeTranslator/src/cn1_globals.h

@@ -855,16 +855,48 @@ else if (IS_DOUBLE_WORD(-1)) SP=BC_DUP2_X2_DSS(SP);\
 
 #define BC_I2C() SP[-1].data.i = (SP[-1].data.i & 0xffff)
 
-#define BC_ISHL() SP--; SP[-1].data.i = (SP[-1].data.i << (0x1f & (*SP).data.i))
-#define BC_ISHL_EXPR(val1, val2) (val1 << (0x1f & val2))
-#define BC_LSHL() SP--; SP[-1].data.l = (SP[-1].data.l << (0x3f & (*SP).data.l))
-#define BC_LSHL_EXPR(val1, val2) (val1 << (0x3f & val2))
+// Java defines << as two's-complement wraparound. C does not: shifting a signed
+// value left so that the result is not representable -- 1 << 31, or any negative
+// left operand -- is UNDEFINED, not merely implementation-defined, so the
+// optimizer is entitled to assume it never happens. Shifting through the
+// unsigned type of the same width gives Java's answer with no undefined step,
+// and the conversion back is the ordinary two's-complement reinterpretation.
+//
+// The signed RIGHT shifts below are left alone deliberately: a negative >> n is
+// implementation-defined rather than undefined, and every compiler this VM is
+// built with defines it as the arithmetic shift Java specifies.
+#define BC_ISHL() SP--; SP[-1].data.i = (JAVA_INT)(((unsigned int)SP[-1].data.i) << (0x1f & (*SP).data.i))
+#define BC_ISHL_EXPR(val1, val2) ((JAVA_INT)(((unsigned int)(JAVA_INT)(val1)) << (0x1f & (val2))))
+#define BC_LSHL() SP--; SP[-1].data.l = (JAVA_LONG)(((unsigned long long)SP[-1].data.l) << (0x3f & (*SP).data.l))
+/* val1 is CAST, and that cast is the whole point.
+ *
+ * The translator emits a long constant as a bare C literal, so LCONST_1 reaches
+ * here as `BC_LSHL_EXPR(1, n)` -- and in C `1` is an int, which makes this an
+ * int shift no matter what the 0x3f mask says. The result was silently wrong for
+ * every shift of a long CONSTANT by 31 or more:
+ *
+ *     1L << 31  gave -2147483648   (int overflow, then sign-extended)
+ *     1L << 32  gave 1             (int shift counts are masked to 5 bits)
+ *     1L << 33  gave 2
+ *
+ * `x << n` for a long VARIABLE was always right, which is why this survived: the
+ * variable carries JAVA_LONG into the macro and the constant does not. Found by a
+ * histogram whose bucket labels came out negative.
+ *
+ * BC_LUSHR_EXPR below already casts, so this class of bug was fixed once for the
+ * unsigned shift and not carried across to its two siblings. */
+// The inner JAVA_LONG cast is load-bearing and separate from the unsigned one:
+// the translator emits long constants as bare C literals, so without it LCONST_1
+// arrives as an int and 1L << 32 evaluates to 1. See the LongShift benchmark.
+#define BC_LSHL_EXPR(val1, val2) ((JAVA_LONG)(((unsigned long long)(JAVA_LONG)(val1)) << (0x3f & (val2))))
 
 #define BC_ISHR() SP--; SP[-1].data.i = (SP[-1].data.i >> (0x1f & (*SP).data.i))
 #define BC_ISHR_EXPR(val1, val2) (val1 >> (0x1f & val2))
 
 #define BC_LSHR() SP--; SP[-1].data.l = (SP[-1].data.l >> (0x3f & (*SP).data.l))
-#define BC_LSHR_EXPR(val1, val2) (val1 >> (0x3f & val2))
+/* Cast for the same reason as BC_LSHL_EXPR above: a long constant arrives as an
+ * int literal and would otherwise be shifted 32 bits wide. */
+#define BC_LSHR_EXPR(val1, val2) (((JAVA_LONG)(val1)) >> (0x3f & (val2)))
 
 #define BC_IUSHL() SP--; SP[-1].data.i = (((unsigned int)SP[-1].data.i) << (0x1f & ((unsigned int)(*SP).data.i)))
 #define BC_IUSHL_EXPR(val1, val2) (((unsigned int)val1) << (0x1f & ((unsigned int)val2)))
@@ -1778,6 +1810,22 @@ extern long long totalAllocations;
 #define CN1_BIBOP_FLUSH_BYTES(ts) do {} while(0)
 #endif
 
+#ifdef CN1_GC_CONFORM
+// Defined in cn1_globals.m. Declared here because the BiBOP fast paths are inline
+// in this header and are the route MOST small objects take -- profiling only
+// codenameOneGcMalloc would miss them and blame whatever little reaches it.
+//
+// There are FOUR entry points, and the profile is only honest if every one of
+// them records exactly once: codenameOneGcMalloc, cn1BibopFastAlloc (what
+// CN1_FAST_NEW calls), cn1BibopFastAllocNoZero, cn1AllocFused and
+// cn1FusedLatin1Begin. cn1BibopAlloc is deliberately NOT hooked -- it is an
+// internal callee of three of those and hooking it would double-count.
+// Each hook sits on the SUCCESS return rather than at function entry: a fast
+// path that returns 0 falls back to __NEW_X -> codenameOneGcMalloc, so an
+// entry-side hook counts that allocation twice.
+void cn1RecordAllocation(struct clazz* parent, int size);
+#endif
+
 // Inlined bump fast path. Returns 0 (slow path: page full / free-list present /
 // ineligible / oversized) -> caller falls back to __NEW_X / codenameOneGcMalloc.
 static inline JAVA_OBJECT cn1BibopFastAlloc(CODENAME_ONE_THREAD_STATE, int size, struct clazz* parent, int ci) {
@@ -1859,6 +1907,9 @@ static inline JAVA_OBJECT cn1BibopFastAlloc(CODENAME_ONE_THREAD_STATE, int size,
             // allocationsSinceLastGC / totalAllocations (the isHighFrequencyGC heuristic)
             // are now bumped in bulk by CN1_BIBOP_FLUSH_BYTES once per page-acquire, not
             // per object -- removing two global-counter stores from the hot path.
+#ifdef CN1_GC_CONFORM
+            cn1RecordAllocation(parent, size);
+#endif
             return o;
         }
     }
@@ -1883,6 +1934,7 @@ static inline JAVA_OBJECT cn1BibopFastAlloc(CODENAME_ONE_THREAD_STATE, int size,
 // memset" note in cn1BibopFastAlloc and OVERFLOW RESCAN in cn1_globals.m). The
 // header (parentCls / mark / heapPosition) is still initialized here; ONLY the
 // body zero is elided.
+
 static inline JAVA_OBJECT cn1BibopFastAllocNoZero(CODENAME_ONE_THREAD_STATE, int size, struct clazz* parent, int ci) {
     if(ci < 0) return (JAVA_OBJECT)0; // oversized: folded away for big types
     if(__builtin_expect(threadStateData->bibopBypassRemaining[ci] > 0, 0)) {
@@ -1937,6 +1989,9 @@ static inline JAVA_OBJECT cn1BibopFastAllocNoZero(CODENAME_ONE_THREAD_STATE, int
             __atomic_store_n(&p->gcAllocedSinceSweep, JAVA_TRUE, __ATOMIC_RELAXED);
 #endif
             CN1_BIBOP_ACCOUNT_BYTES(threadStateData, p->slotSize);
+#ifdef CN1_GC_CONFORM
+            cn1RecordAllocation(parent, size);
+#endif
             return o;
         }
     }