From 8ef8642e35e5e4db1407c127c4cfb4fcfbd7a431 Mon Sep 17 00:00:00 2001 From: Jim Schaff Date: Sun, 23 Aug 2026 13:17:50 -0400 Subject: [PATCH 1/6] docs(geometry): consolidate the geometry-memory incident, measurements and options Planning document, deliberately not a fix. The 2026-08-22 prod OOM produced three prototype branches in quick succession (#2022, #2023, #2024) and the understanding changed three times while they were being written -- which is the signal that this needs planning rather than merging. Records what happened, what was measured, what turned out to be WRONG, and the options with their costs. Section 4 is the part worth reading: four beliefs that were acted on and then disproved, including two of my own that reached open PRs. The most consequential correction: XML read does NOT rebuild surfaces. XmlReader:2086 assigns the parsed surface description to a variable named 'dummy', but the method MUTATES the geometry it is passed and only the return value is discarded, so getGeometricRegions() is non-null and precomputeAll skips updateAll entirely. The incident was eleven image decodes and eleven retained pixel arrays (~680 MB against a 1000 MB heap), not eleven surface rebuilds -- which matches both the measured 661 MB and the ~370 ms spacing of the prod warnings. Also records that a pixel-class limit was implemented, measured and abandoned (memory is flat in class count; region count is the predictor), that enforcing the image-size veto on the load path would have broken the very model it protected, and that the first duplicate-row query over-matched by 13x. Supersedes docs/geometry-memory-notes.md from #2023. Refs #2021, #2022, #2023, #2024 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_018kr8SbzXtwW3gMVUgMfDDt --- docs/geometry-memory.md | 428 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 428 insertions(+) create mode 100644 docs/geometry-memory.md diff --git a/docs/geometry-memory.md b/docs/geometry-memory.md new file mode 100644 index 0000000000..d22e836f4d --- /dev/null +++ b/docs/geometry-memory.md @@ -0,0 +1,428 @@ +# Geometry memory: the incident, what we measured, and what to do about it + +**Status: planning document. Nothing here is committed to.** +Three prototype branches exist (#2022, #2023, #2024). They are evidence that the measurements are +real and the fixes are feasible — **not** merge candidates. Decisions belong in this document +first. + +Written 2026-08-23 after the 2026-08-22 prod incident. Supersedes `docs/geometry-memory-notes.md` +from #2023, which contains an error corrected below. + +--- + +## 1. What happened + +At 06:36:57Z on 2026-08-22, two production `api` pods died within seven seconds of each other: + +``` +06:36:57.750 challenge response was null <- unauthenticated request +06:36:58.244 WARNING: image size 61920000 pixels exceeded limit of 4000000 + ... 10 more, ~370 ms apart ... + java.lang.OutOfMemoryError: Java heap space + Heap dump ... [1024595375 bytes] + Terminating due to java.lang.OutOfMemoryError +api-76b48c8cdb-4l5jz 06:36:57Z exit 3 +api-76b48c8cdb-crsfw 06:37:04Z exit 3 +``` + +The request was `GET /api/v0/biomodel/101963252/simulation/98916046`, issued by **PetalBot**, a +search-engine crawler walking links from `/api/v0/biomodel/101963252`. The model has 15 simulation +contexts and a 61,920,000-pixel geometry. The api heap is **1000 MB**. + +An identical, non-fatal repeat at 07:48 completed in 4022 ms, which tells us the request is +survivable when the pod is otherwise idle — this is a margin problem, not an impossibility. + +Two things are worth stating plainly: + +- **This is remotely reachable and needs no credentials.** Any crawler, or any user, who fetches a + published model can do this. It is a robustness issue, not a nuisance warning. +- **#1899 is why we can see it at all.** Before the heap was sized to fit the container, the kernel + OOM-killed the pod (exit 137) while the Java heap sat mostly free, so `HeapDumpOnOutOfMemoryError` + could never fire. This is the first time that machinery produced evidence. + +--- + +## 2. Why it costs so much: the VCML shape + +`Xmlproducer.java:1416` writes the **complete `` element, image included, inside every +``**, and `XmlReader.java:6024` parses each copy independently. + +So a model with 11 spatial applications over one geometry carries eleven copies of the same image +on the wire and decodes eleven copies into the heap. At 62 MP that is **11 × 62 MB ≈ 680 MB of +retained pixel arrays**, on a 1000 MB heap that is also serving everything else. + +That is the incident, in one sentence. + +--- + +## 3. What we measured + +Instrumentation lives on #2023: `GeometryMemoryProfiler` (per-phase peak / retained / allocated, +with a warmup pass and explicit retention holds) and `XmlGeometrySharingBenchmark`. A heap class +histogram was taken with `jcmd GC.class_histogram` during a hold window. + +### 3.1 Parsing a document with N applications + +11 applications over one 7872×7872 image (61,968,384 px — the incident's pixel count): + +| | peak | retained | time | Geometry objects | VCImage objects | +|---|--:|--:|--:|--:|--:| +| share nothing (today) | 787.1 MB | 661.4 MB | 2689 ms | 11 | 11 | +| share decoded images | **162.1 MB** | **60.4 MB** | **287 ms** | 11 | 1 | +| share whole geometries | 162.1 MB | 60.3 MB | 273 ms | 1 | 1 | + +Retained falls by exactly the application count. **Sharing images alone captures the entire win**; +sharing the mutable `Geometry` on top adds nothing measurable. + +### 3.2 Building a RegionImage from scratch (a different operation — see §4.1) + +`GeometryMemoryProfiler`, one image geometry: + +| pixels | peak | retained | allocated | time | +|--:|--:|--:|--:|--:| +| 262,144 | 40.2 MB | 7.9 MB | 22.6 MB | 47 ms | +| 2,097,152 | 101.7 MB | 53.5 MB | 101.8 MB | 99 ms | +| 4,096,000 | 151.6 MB | 73.5 MB | 185.9 MB | 193 ms | +| 62,099,136 | **1,431 MB** | 810 MB | 1,962 MB | 1930 ms | + +Region finding is volume-linear at ~4 B/px retained. Surface cost scales with interface *area*, so +its share per pixel falls as the image grows while its absolute cost dominates. + +### 3.3 Where the bytes are + +Heap histogram at 62 MP, retained: + +| structure | instances | bytes | +|---|--:|--:| +| `int[]` | 2,121 | **497.7 MB** | +| `byte[]` | 22,869 | 130.4 MB | +| `RegionImage$MembraneEdgeNeighbor` | 1,920,800 | 61.5 MB | +| `RegionImage$MembraneElementIdentifier` | 1,919,138 | 46.1 MB | +| `Object[]` | 482,989 | 31.8 MB | +| `surface.Node` | 479,866 | 23.0 MB | +| `Node[]` | 480,201 | 17.3 MB | +| `ArrayList` | 480,458 | 11.5 MB | +| `RegionImage$SurfAndFace` | 480,200 | 11.5 MB | +| `surface.Quadrilateral` | 480,200 | 11.5 MB | + +Two facts fall out: + +- The 497.7 MB of `int[]` in 2,121 arrays is `mapImageIndexToLinkRegion` — **one `int` per pixel, + 236.9 MB per RegionImage** — against a measured **16,706 link regions, which fit in two bytes**. +- **480,200 quadrilaterals carry ~180 MB of objects, ~375 bytes per quad**, where a quad's + geometric content is four node indices (16 bytes). + +### 3.4 How VCell compares to a mature labeller + +Prompted by the suggestion to look at `cv2.connectedComponents`. Same synthetic volume, one process +per case (`ru_maxrss` is a monotonic high-water mark, so several cases in one process cannot be +measured independently — an earlier attempt that ignored this reported cv2 at 1.04 B/px, an +artefact): + +| | pixels | added peak | time | B/px | +|---|--:|--:|--:|--:| +| `scipy.ndimage.label` 3D | 62,099,136 | 580.6 MB | 725 ms | 9.80 | +| `cv2.connectedComponents` 2D | 61,968,384 | 335.5 MB | 74 ms | 5.68 | +| `scipy.ndimage.label` 2D | 61,968,384 | 280.2 MB | 445 ms | 4.74 | +| VCell region finding, 3D | 62,099,136 | 452 MB peak | 472 ms | 8.32 alloc | + +**VCell's connected-component labelling is competitive with scipy and not the problem.** Everyone +pays ~4–5 B/px because everyone materialises a 32-bit label array. cv2 is much faster in 2D but +uses more memory, and is 2-D only. The conclusion is that the labelling *algorithm* is not where +VCell is losing — which redirects effort to §5.3 and §5.5. + +### 3.5 Pixel classes are the wrong thing to limit + +Tested because a limit of ~16 pixel classes was proposed as a way to reject unsegmented images. +256³ volume, one subvolume per concentric shell, regions only: + +| pixel classes | 2 | 4 | 16 | 32 | 64 | 128 | +|---|--:|--:|--:|--:|--:|--:| +| regions | 2 | 4 | 16 | 32 | 64 | **14,050** | +| peak | 124 MB | 196 MB | 244 MB | 209 MB | **300 MB** | **1,652 MB** | + +**Memory is flat in pixel-class count.** A 64-subvolume geometry is unremarkable. The jump at 128 is +not the class count: at that resolution the shells fall below one voxel thick and **fragment**, and +it is the 14,050 resulting regions that cost 1.65 GB. Measured separately, a **random 2-class image +costs 2,111 MB at under 1 MP**. + +So a class limit would reject the cheap 64-subvolume case and still admit the expensive fragmented +one. **The predictor is region count, not class count.** This also confirms the observation that +doing N binary passes would not blow up — VCell's labelling already does not scale with class count. + +### 3.6 Prod database: duplicate image rows + +Read-only queries against prod (see §7 for the account rule that governs this). + +A first pass grouped on "name with trailing digits stripped" and reported 12,629 surplus rows. +**That was wrong** — it collapsed `N1E for PIP2` / `PIP3` / `PIP4`, which are deliberately distinct +images. Discarded. + +Tightened: `TokenMangler.getNextRandomToken` appends `abs(random × Integer.MAX_VALUE)`, effectively +always 6+ digits, which humans do not type. Combined with same owner, same dimensions, and written +within one save: + +| | groups | surplus rows | +|---|--:|--:| +| copies written **≤ 5 s apart** (one save) | 453 | **944** | +| …requiring ≥1 machine-mangled name | 445 | 936 | +| copies **> 5 s apart** (ordinary save-as, not a bug) | 2,196 | 8,965 | + +Of 21,978 images, 14,083 carry a mangled name, but most of that is twenty years of legitimate +save-as collisions. + +The mechanism is confirmed by the reference counts — image rows vs. geometries referencing them: + +``` +sus_arj_10um_nov 17 image rows 17 geometries +img_20190130_ 15 15 +10spines 15 15 +img_20180606_ 12 12 +fix_simple 11 11 +img_20250616_ 8 8 <- October 2025 +``` + +**Exactly one image row per referencing geometry**, which is what N inserts inside one +`saveBioModel` produces and not what save-as produces. + +Cause: `saveBioModel` keys `memoryToDatabaseHash` by the image object, and `VCImage` overrides +neither `equals` nor `hashCode`. The XML round-trip inside the save produces N distinct-but-equal +objects for one image, so a **new** image (`key == null`) falls into the insert branch N times. + +Not yet confirmed: that the referencing geometries all belong to a single BioModel. One more query +would settle it. + +--- + +## 4. What we got wrong along the way + +Recorded deliberately — each of these was believed, acted on, and then disproved by measurement. +They are the reason this document exists instead of three merged PRs. + +### 4.1 "Surfaces are rebuilt on every XML read" — false + +`XmlReader:2086` assigns the parsed surface description to a variable named `dummy`, which looks +like the document's stored surfaces are discarded and recomputed. They are not. +`getGeometrySurfaceDescription(sd, newgeometry)` **mutates the geometry it is passed** — +`setVolumeSampleSize`, `setFilterCutoffFrequency`, `setGeometricRegions`. Only the *return value* +is discarded. Afterwards `getGeometricRegions()` is non-null, so: + +```java +// Geometry:457 +if (getDimension()>0 && (bForcePrecomputeSurfaces || getGeometrySurfaceDescription().getGeometricRegions()==null)) +``` + +is false and `updateAll()` never runs. **A saved model reuses its stored surfaces.** + +Consequence: the 1,431 MB in §3.2 is the real cost of `new RegionImage(...)`, but **an XML read of a +stored model does not pay it**. The incident was 11 image decodes and 11 retained pixel arrays — +which matches both the measured 661 MB and the ~370 ms spacing of the prod warnings +(hex-decode-plus-inflate time, not the ~1.1 s a 62 MP RegionImage takes). + +### 4.2 "One parse needs 1.4 GB, so the pods were doomed" — overstated + +Followed from 4.1. One *read* of that model is affordable; eleven copies of its pixels are not. + +### 4.3 "Enforce the image-size veto" — would have broken the model it protects + +`GeometrySpec.vetoableChange` fires when **deserializing**, and the constructor converts +`PropertyVetoException` into `RuntimeException`. Any enforced ceiling there makes stored geometries +above it impossible to **open**. BioModel 101963252 is 61,920,000 px and loads today; an enforced +50 M ceiling would have broken exactly the model the fix was aimed at. This is why enforcement has +to move to the submission boundary (§5.4). + +It is also, almost certainly, why the throw was commented out in 2017 (`1bf1b2a551`) rather than +fixed: at the historical limit of 4,000,000 px — only 159³, or 2000×2000 in 2D — enforcement +rejects ordinary models. + +### 4.4 "Limit pixel classes" — measured to be the wrong quantity + +See §3.5. Implemented, measured, abandoned. + +### 4.5 The first duplicate-row query over-matched + +See §3.6. 12,629 → ~944. + +--- + +## 5. Options + +Ordered by measured value, not by appeal. Each states what it buys, what it costs, and what is +unresolved. + +### 5.1 Share the decoded `VCImage` within one document parse — **recommended first** + +Cache decoded images by digest of their `` element, for the life of one `XmlReader` (one +document). + +- **Buys:** the whole measured win — peak 787 → 162 MB, retained 661 → 60 MB, 8.7× faster (§3.1). +- **Risk:** low. A `VCImage` is immutable payload: compressed pixels are `final`, + `VCPixelClass implements Immutable` with no setters, and everything editable — subvolumes, their + names, extent, origin, surfaces — lives on the per-application `GeometrySpec`. +- **Also fixes** the duplicate-row insert (§3.6), because the identity-keyed hashtable then sees one + object. +- **Prototype:** #2024. `MathGen_IT` 1045 tests green. + +**Explicitly not** sharing the `Geometry` object. Five applications sharing one mutable `Geometry` +would see each other's subvolume renames — unexpected, and measured to buy nothing on top of image +sharing. The prototype keeps it behind an off-by-default property for read-only server use only. + +### 5.2 Stop writing the geometry once per application (VCML format) + +Emit geometries once at BioModel level and reference them by key. + +- **Buys:** ~11× smaller VCML for these models, on top of the parse win. Removes the root cause + rather than compensating for it. +- **Costs:** a format change. Old clients must still read new documents, and the migration story + needs design. Bigger than anything else on this list. +- **Unresolved:** whether the reference should be a database key or a document-local id; what + happens for a model whose applications genuinely differ. + +### 5.3 Narrow the per-pixel label array + +`mapImageIndexToLinkRegion` is `int[N]` (237 MB at 62 MP) against 16,706 labels. + +- **Option A — width promotion.** `RegionImage:86-146` already contains a commented-out + `CompactUnsignedIntStorage` doing byte → short → int promotion. 237 → 118 MB (or 59 MB). + Someone reached this conclusion before; it was never wired in. +- **Option B — run-length encoding.** Segmented volumes are highly run-coherent, so RLE could be + orders of magnitude smaller, not 2×. `isIndexInRegion(index)` is the only random-access consumer + and a binary search over runs serves it. Riskier, much larger payoff. +- **Note:** `RegionImage:855` already throws above 65535 *distinct* regions, so 16 bits is already + the working assumption downstream — though that cap is on post-merge regions, not the pre-merge + labels this array holds. + +**Do this before any tiling work** (§5.5): streaming the computation while still emitting a +full-size output array saves peak but not retention. + +### 5.4 Limit new submissions, grandfather what is stored + +Enforce at `ServerDocumentManager.saveGeometry / saveBioModel / saveMathModel`, keyed on +`image.getKey() == null` — content that has never been persisted. + +- **Buys:** the problem stops growing, without breaking a single existing model (§4.3). +- **Limits:** image size, and **region count** rather than pixel classes (§3.5). Region count is + free at save time — the geometry already computed a `RegionImage` while parsing. +- **Prototype:** #2022, defaults 16,000,000 px and 2,000 regions, both properties. +- **Unresolved, and the reason this is not ready:** *both numbers are guesses calibrated against + today's implementation, not against what the science needs.* See §6. + +### 5.5 Reduce the surface representation + +~375 bytes per quadrilateral for 16 bytes of content (§3.3). A struct-of-arrays layout — +`int[4*numQuads]` node indices, `float[3*numNodes]` coordinates — is ~28 B/quad, a >10× reduction, +and turns 2 M scattered objects into a handful of arrays G1 never has to trace. + +- **Costs:** ripples through `SurfaceCollection`, `OrigSurface`, `Polygon`, `TaubinSmoothing`, the + STL exporter and the VTK path. Worth costing before committing. +- **Smaller, separable first step:** the `edgeMap` + (`HashMap>>`, `RegionImage:1744`) is + built for every edge of every quad and used once. A sorted `long[]` of packed `(node1, node2)` + keys replaces the whole structure. + +### 5.6 Tiled / slab-wise region finding + +Two variants were discussed. + +**Slab-wise inside `RegionImage`.** The algorithm is already two-pass connected components — a +z-major labelling pass plus equivalence resolution — and it is already **6-connected** (forward x, +y, z only), so face-only reconciliation suffices and no halo is needed. The surface pass already +streams two z-planes of nodes (`mapImageIndexToNode[2][numY+1][numX+1]`). The structure is largely +there; what blocks it is the full-volume state: `regionPixels` `int[N]`, three `BitSet(N)`, and the +retained label map. **Sequence after §5.3.** + +One concrete inefficiency to fix along the way: equivalences go into `Vector[]` with a +linear `contains()` scan per pixel-pair (`createLink`), then a clone-and-BFS merge. That is not +union-find, and union-find over ~16,706 labels is trivially small. + +**Tiling whole geometries and stitching them.** Appealing because it reuses `RegionImage` unchanged +per tile. Three specific obstacles, all verified: + +1. **Taubin smoothing is global.** `RegionImage:930` smooths the entire `SurfaceCollection`. + Smoothing per tile then stitching gives *different node positions* near seams — a correctness + difference, not a cosmetic seam. +2. **Watertight stitching has bitten this codebase before.** #1890 / #1895 / #1896: independently + processed pieces produced a non-watertight surface with 464 exposed interior-grid-plane walls. + Nodes on a shared face are created twice with different global indices, and membrane adjacency + across the seam must be recomputed. +3. **Membrane element ordering is persisted.** Stitching changes quad ordering, and membrane element + indices map stored simulation results. + +Also worth weighing: at 62 MP the peak was 1.4 GB but retained only 810 MB, so tiling bounds the +working set, not the result. + +### 5.7 Lazy surface generation + +Low value, given §4.1 — stored models already skip it. Only helps documents with no stored +`` (older VCML, some geometry-only documents). + +### 5.8 Clean up the 944 duplicate image rows + +Separate from the code fix, and a data change: needs an owner, a dry run, and a decision about +whether rows referenced by geometries can be repointed and collapsed. + +### 5.9 Operational levers + +Independent of all the above, and cheap: + +- **Crawler traffic.** `/api/v0/biomodel/{id}/simulation/{id}` is being walked by PetalBot. A + `robots.txt`, or rate limiting on the legacy api, removes the trigger without touching geometry + code. It does not fix the underlying fragility. +- **api heap.** 1000 MB, sized in #1899. Whether that is still right is a separate question from + whether one request should need 680 MB. + +--- + +## 6. Open questions — decisions needed before implementation + +1. **What image size should a new geometry be allowed to have?** The prototype says 16 M px, derived + from today's memory profile against a 1000 MB heap. That is an implementation artefact, not a + scientific limit. **Needs a survey of image sizes actually in the database.** +2. **What region count?** Same problem; 2,000 is a guess. A tissue image with hundreds of separate + cells is ordinary science. **Needs a survey of region counts.** +3. **Is the VCML format change (§5.2) on the table at all?** Everything else is compensation for it. +4. **Sequencing.** Suggested: §5.1 → §5.4 (with real numbers) → §5.3 → §5.5 → §5.6. §5.9 can happen + immediately and independently. +5. **Who owns the duplicate-row cleanup**, and should it wait for §5.1 to stop new ones first? +6. **Do the duplicate images in a group belong to one BioModel?** One query settles it; needs + `vcell_dev` credentials. + +Items 1, 2 and 6 are all answerable with read-only queries once someone runs them. + +--- + +## 7. Working notes + +**Database access.** Investigations use the **`vcell_dev`** account. Not `vcell_service` — that is +for running services only, and locking it would take down every environment — and not `vcell`, the +schema owner. Its password is **not** in any Kubernetes secret (the cluster secrets hold the +`vcell_service` password); pointing a script at `vcell_dev` while sourcing that secret is a failed +login, which is the exact lockout risk. Supply it explicitly and try once. + +**Prototype branches — do not merge as-is:** + +| PR | Branch | Contains | +|---|---|---| +| #2022 | `fix/geometryspec-image-veto` | Submission-time limits (§5.4), grandfathering | +| #2023 | `perf/geometry-memory-profiler` | `GeometryMemoryProfiler` + the notes file this supersedes | +| #2024 | `perf/parse-geometry-once` | Image sharing (§5.1) + `XmlGeometrySharingBenchmark` | + +The instrumentation in #2023 is the piece most worth keeping regardless of which direction is +chosen: every number in this document came from it, and the corrections in §4 were only possible +because it existed. + +**Reproducing the measurements:** + +```bash +mvn test-compile -pl vcell-core -am +mvn -q -pl vcell-core exec:java -Dexec.classpathScope=test \ + -Dexec.mainClass=cbit.vcell.geometry.GeometryMemoryProfiler +mvn -q -pl vcell-core exec:java -Dexec.classpathScope=test \ + -Dexec.mainClass=cbit.vcell.xml.XmlGeometrySharingBenchmark +``` + +Add `-Dprofiler.hold=120` to keep structures reachable for `jcmd GC.class_histogram`. + +**Related:** #2021 (the issue — note its framing of "eleven parses exhausted the heap" is corrected +by §4.1/§4.2), #1899 (heap sizing, why this was diagnosable), #1890/#1895/#1896 (the watertightness +precedent), virtualcell/vcell-fluxcd#51 (the suspended restart cron this was found under). From 4d8b7946e8cec2fa7a5bd89a9dc8d6008525ed46 Mon Sep 17 00:00:00 2001 From: Jim Schaff Date: Sun, 23 Aug 2026 18:27:25 -0400 Subject: [PATCH 2/6] docs(geometry): measure the compressed-and-rehydrate idea; it holds up Jim's suggestion: VCImageCompressed already keeps compressed pixels strongly and the uncompressed copy as a transient cache, so let the uncompressed copy become reclaimable, and consider holding other large objects compressed too. Measured before writing it up, and the numbers are better than expected. To the direct question first: RegionImage is NOT a VCImage. It implements Serializable, takes a VCImage as a constructor argument and keeps only the dimensions. Its heavy state is its own int[] label map plus the SurfaceCollection. So the idea applies in two separate places, not one. Real geometry images out of the VCML test corpus -- 62 of them, using the CompressedSize already recorded in , so no synthetic stand-in whose compressibility would be wrong by construction: all 62 images 23.3x .. 73.6x median .. 287.7x the 32 >= 1 MP 53.6x .. 63.5x median .. 287.7x aggregate 83.8 MB raw -> 1.1 MB compressed (78.8x) Rehydration is cheap: 676-951 MB/s across four real images, so a 62 MP image re-inflates in roughly 65-90 ms against the ~2.7 s the eleven-application parse takes today. The result that was not obvious: the DERIVED label array compresses too, 53-93x as int[]. Scaled to 62 MP, mapImageIndexToLinkRegion -- the single largest retained object at 236.9 MB -- becomes 2.5-4.5 MB. One correction that decides whether the idea works: use SoftReference, not WeakReference. A weak reference is cleared at the next GC regardless of memory pressure, so a hot geometry would re-inflate on essentially every collection -- 90 ms of CPU repeatedly for no benefit. Soft references are cleared only under actual pressure. Three verified blockers for a naive version: GeometrySpec.uncompressedPixels holds a SECOND strong reference to the same array and would pin it regardless; VCImageUncompressed has no compressed form at all (final byte[]), which matters because the sampled image is one; and a compressed label map is not randomly accessible while isIndexInRegion needs random access, so it wants block-wise compression or RLE. Feasibility rests on an audit of getPixels(), which returns the internal array and has 178 call sites. A narrow check of cbit.image and cbit.vcell.geometry found no mutation, but that is a small fraction of the surface. Also concedes a point: the earlier objection that tiling bounds the working set while the finished artefact still has to fit in memory is weakened by this. If the result is held compressed, size stops being the binding constraint. The remaining obstacles to stitching are about correctness, not size. Refs #2021, #2025 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_018kr8SbzXtwW3gMVUgMfDDt --- docs/geometry-memory.md | 105 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 101 insertions(+), 4 deletions(-) diff --git a/docs/geometry-memory.md b/docs/geometry-memory.md index d22e836f4d..a70adb903f 100644 --- a/docs/geometry-memory.md +++ b/docs/geometry-memory.md @@ -192,6 +192,52 @@ objects for one image, so a **new** image (`key == null`) falls into the insert Not yet confirmed: that the referencing geometries all belong to a single BioModel. One more query would settle it. +### 3.7 How compressible this data actually is + +Measured after the question was raised: if the expensive things can be *held* compressed and +rehydrated on demand, the size of the finished artefact stops being the constraint it looked like. + +**Real geometry images, straight out of the VCML test corpus** (the `` element carries +`CompressedSize`, so the deflate ratio VCell already achieves is readable without decompressing +anything). 62 distinct images: + +| | min | median | max | +|---|--:|--:|--:| +| all 62 images | 23.3× | **73.6×** | 287.7× | +| the 32 images ≥ 1 MP | 53.6× | **63.5×** | 287.7× | + +Aggregate: 87,874,955 px would be **83.8 MB raw, 1.1 MB compressed — 78.8×**. + +Synthetic images get this property badly wrong in both directions, which is why these are real ones. + +**Rehydration cost**, four real images, best of seven inflates with the cache cleared each time: + +| image | pixels | ratio | inflate | throughput | +|---|--:|--:|--:|--:| +| 564×160×31 | 2,797,440 | 234.3× | 3.9 ms | 676 MB/s | +| 208×153×83 | 2,641,392 | 63.5× | 2.6 ms | 951 MB/s | +| 600×300×22 | 3,960,000 | 114.0× | 4.9 ms | 775 MB/s | +| 256×256×34 | 2,228,224 | 106.3× | 2.4 ms | 888 MB/s | + +**A 62 MP image rehydrates in roughly 65–90 ms** — against the ~2.7 s the eleven-application parse +takes today. + +**The derived label array compresses too**, which is the more surprising result. Same four images, +`mapImageIndexToLinkRegion` deflated: + +| image | entries | max label | regions | as `int[]` | as `short[]` | +|---|--:|--:|--:|--:|--:| +| 564×160×31 | 2,797,440 | 1,523 | 2 | 93.2× | 71.0× | +| 600×300×22 | 3,960,000 | 3,557 | 3 | 86.6× | 63.6× | +| 256×256×34 | 2,228,224 | 2,305 | 3 | 76.8× | 55.0× | +| 208×153×83 | 2,641,392 | 3,830 | 6 | 53.1× | 36.8× | + +Scaled to 62 MP, **the 236.9 MB `int[]` — the single largest retained object in §3.3 — becomes +2.5–4.5 MB.** Deflating it costs 13–23 ms per 2–4 M entries. + +Incidentally, the corpus corroborates §2 directly: `biomodel_27192717.vcml` embeds the same image +element **nine times**, `biomodel_26455186.vcml` eight times. + --- ## 4. What we got wrong along the way @@ -295,6 +341,52 @@ Emit geometries once at BioModel level and reference them by key. **Do this before any tiling work** (§5.5): streaming the computation while still emitting a full-size output array saves peak but not retention. +### 5.3b Hold it compressed, rehydrate on demand — **most promising unexplored direction** + +Raised as: `VCImageCompressed` already keeps compressed pixels strongly and the uncompressed copy +as a `transient` cache, so take it one step further and let the uncompressed copy be reclaimable. + +§3.7 says the arithmetic works, and works better than expected. Real geometry images compress +**50–100×**, rehydrate at **~700–950 MB/s**, and — the part that was not obvious — the derived +label array compresses **53–93×** as well. + +**Use `SoftReference`, not `WeakReference`.** This distinction decides whether the idea works. A +weak reference is cleared at the next GC *regardless of memory pressure*, so a hot geometry would +re-inflate on essentially every collection: 90 ms of CPU repeatedly, for no benefit. A soft +reference is cleared only when the heap is actually under pressure, and the JVM ages them by +`-XX:SoftRefLRUPolicyMSPerMB`. Soft is the tool that gives "does not contribute to memory +pressure" without giving up the cache. + +Three things block a naive version, all verified: + +1. **`GeometrySpec.uncompressedPixels` (line 67) holds a second strong reference to the very same + array** (`getImage().getPixels()` at line 962). Softening `VCImageCompressed.uncompressed` alone + changes nothing while that field pins it. They have to move together. +2. **`VCImageUncompressed` has no compressed form** — `private final byte pixels[]`. The sampled + image from `createSampledImage` is one of these, so it cannot be softened as-is; it would need + either a compressed backing or a documented recompute path (it *is* derivable from the image + plus subvolume handles). +3. **A compressed label map is not randomly accessible**, and `RegionInfo.isIndexInRegion(index)` + needs random access. Two ways out: compress in fixed blocks and cache a few decompressed blocks, + or use RLE, which supports random access by binary search over runs and is a natural fit for + run-coherent segmented volumes. The second doubles as option §5.3. + +**The audit that decides feasibility.** `getPixels()` returns the internal array and has **178 call +sites** across core, server and client. Two failure modes to look for: a caller that *retains* the +array in a field (harmless to correctness, but pins it and defeats the purpose) and a caller that +*mutates* it expecting persistence (a real correctness bug once a rehydration can replace the +array). A narrow check of the `cbit.image` and `cbit.vcell.geometry` packages found no mutation — +the one candidate, `RayCaster:1626`, writes into a freshly allocated array — but that is a small +fraction of the surface and the rest is unexamined. + +**Precedent worth knowing about:** `VCImageCompressed.nullifyUncompressedPixels()` already exists +and has exactly one caller, `GeomDbDriver:490`. Someone hit this problem before and solved it by +hand in a single place. + +**Why this is worth the deep investigation:** it attacks retained memory rather than peak, it needs +no format change, and correctness is verifiable against a large corpus of stored models — the +inflated bytes either match or they do not. + ### 5.4 Limit new submissions, grandfather what is stored Enforce at `ServerDocumentManager.saveGeometry / saveBioModel / saveMathModel`, keyed on @@ -348,8 +440,11 @@ per tile. Three specific obstacles, all verified: 3. **Membrane element ordering is persisted.** Stitching changes quad ordering, and membrane element indices map stored simulation results. -Also worth weighing: at 62 MP the peak was 1.4 GB but retained only 810 MB, so tiling bounds the -working set, not the result. +An earlier objection here — that tiling bounds the working set but the finished artefact still has +to fit in memory — is **weakened by §3.7**. If the accumulated result is held compressed (2.5–4.5 MB +for a 62 MP label map rather than 237 MB), the finished artefact is no longer the binding +constraint, and tiling becomes considerably more attractive than it first appeared. The three +obstacles above are about *correctness of the stitch*, not about size, and they still stand. ### 5.7 Lazy surface generation @@ -381,8 +476,10 @@ Independent of all the above, and cheap: 2. **What region count?** Same problem; 2,000 is a guess. A tissue image with hundreds of separate cells is ordinary science. **Needs a survey of region counts.** 3. **Is the VCML format change (§5.2) on the table at all?** Everything else is compensation for it. -4. **Sequencing.** Suggested: §5.1 → §5.4 (with real numbers) → §5.3 → §5.5 → §5.6. §5.9 can happen - immediately and independently. +4. **Sequencing.** Suggested: §5.1 → §5.4 (with real numbers) → §5.3/§5.3b → §5.5 → §5.6. §5.9 can + happen immediately and independently. §5.3b is the one worth a real investigation rather than a + quick implementation: the numbers are strong, and the risk is entirely in the 178-call-site + `getPixels()` audit rather than in the arithmetic. 5. **Who owns the duplicate-row cleanup**, and should it wait for §5.1 to stop new ones first? 6. **Do the duplicate images in a group belong to one BioModel?** One query settles it; needs `vcell_dev` credentials. From 527232f9255939fd476afbf5d952738f274246ff Mon Sep 17 00:00:00 2001 From: Jim Schaff Date: Sun, 23 Aug 2026 18:41:39 -0400 Subject: [PATCH 3/6] docs(geometry): do the getPixels() audit; the surface is far smaller than claimed Resolves the open item that decided whether the compressed-and-rehydrate idea in 5.3b is feasible. Method matters here, because grep cannot answer this question: getPixels() also exists on ByteImage, UShortImage, FloatImage, ShortImage and ImageDataset, and a text search cannot tell which receiver a call is on. The compiler already resolved every receiver, so the call sites were read out of the BYTECODE with javap across all 13 built modules and then located in source. (I also tried mapping bytecode offsets to lines via LineNumberTable; it put several call sites on lines with no call at all, so that step was dropped rather than trusted.) Corrects my own figure. This document said 178 call sites. That was a raw grep count dominated by the unrelated image classes. Real surface: 39 call instructions across 22 files, about 31 distinct source expressions. Classification: ~27 read-only and transient, 2 already defensively .clone(), 3 that retain the array, and exactly ONE that mutates it. The single mutation is DatabaseWindowManager:1004, and it is on a VCImageUncompressed built locally by createSampledImage for display scaling -- never a stored, compressed image. So it does not block softening VCImageCompressed.uncompressed. It does prove that returning the LIVE array is load-bearing: any variant returning a defensive copy silently loses those writes. Of the three retention sites only one matters -- GeometrySpec:962, which stores the stored image's array in a field. Now confirmed to be the only one of its kind. The other two (SourceDataInfo, MemoryImageSource) hold the SAMPLED image's array, which has no compressed form to soften against. The finding I did not expect is a hazard that is neither mutation nor retention: four loops call getPixels() in the loop header, and deeper than that, ImageSubVolume.isInside calls getUncompressedPixels()[index] for ONE PIXEL PER CALL from sampling loops. Under the memory pressure that clears a soft reference -- exactly the condition this is for -- that turns a pointer dereference into an inflate. That, not correctness, is the real design problem. Refs #2021, #2025 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_018kr8SbzXtwW3gMVUgMfDDt --- docs/geometry-memory.md | 89 +++++++++++++++++++++++++++++++++++++---- 1 file changed, 81 insertions(+), 8 deletions(-) diff --git a/docs/geometry-memory.md b/docs/geometry-memory.md index a70adb903f..3ac5c92649 100644 --- a/docs/geometry-memory.md +++ b/docs/geometry-memory.md @@ -238,6 +238,79 @@ Scaled to 62 MP, **the 236.9 MB `int[]` — the single largest retained object i Incidentally, the corpus corroborates §2 directly: `biomodel_27192717.vcml` embeds the same image element **nine times**, `biomodel_26455186.vcml` eight times. +### 3.8 The `getPixels()` audit + +**Method.** Grep cannot answer this: `getPixels()` also exists on `ByteImage`, `UShortImage`, +`FloatImage`, `ShortImage` and `ImageDataset`, and text search cannot tell which receiver a call is +on. The compiler already resolved every receiver, so the call sites were read out of the **bytecode** +(`javap -c` over all 13 built modules, matching `invoke*` whose receiver is `VCImage`, +`VCImageUncompressed` or `VCImageCompressed`), then located in source. An earlier attempt also +mapped bytecode offsets to line numbers via `LineNumberTable` and got several lines wrong; that step +was dropped rather than trusted, and class-level attribution — which is exact — was used instead. +Test sources are excluded. + +**Scale — an earlier figure in this document was wrong.** It said 178 call sites. That was a raw +grep count dominated by the unrelated image classes. The real surface: + +| | | +|---|--:| +| call instructions with a `VCImage` receiver | **39** | +| source files containing them | **22** | +| distinct source expressions | ~31 | + +**Classification.** + +| Category | Sites | Which | +|---|--:|---| +| Read-only, transient | ~27 | the large majority — copy out, iterate, write to a file, build a display buffer | +| Already defensive | 2 | `RayCaster:1643` and `VCImageUncompressed:31` call `.clone()` | +| **Retains the array** | 3 | see below | +| **Mutates the array** | **1** | see below | + +**The one mutation** is `DatabaseWindowManager:1004`: + +```java +currentValue.getPixels()[j] = (byte) (newPC[i].getPixel() & 0x000000FF); +``` + +`currentValue` is a `VCImageUncompressed` produced locally by `createSampledImage` for display +scaling — never a stored, compressed image. So it does **not** block softening +`VCImageCompressed.uncompressed`. It does mean **`getPixels()` returning the live array is +load-bearing**: any variant that returns a defensive copy silently loses these writes. + +**The three retention sites**, only one of which matters: + +- `GeometrySpec:962` — `uncompressedPixels = getImage().getPixels()` stores the **stored image's** + array in a field. This is the blocker already identified in §5.3b, now confirmed to be the only + one of its kind. +- `GeometryViewer:257` — the array is handed to `SourceDataInfo`, which keeps it in a + `private Serializable data` field for as long as the viewer is open. +- `GeometrySummaryPanel:395` — handed to `MemoryImageSource`, which retains it. + +Both of the latter are on the *sampled* image, which has no compressed form to be softened against. + +**A hazard specific to soft references, not visible as either category.** Four loops call +`getPixels()` in the loop header itself: + +```java +for (int i = 0; i < dbImage.getPixels().length; i++) // ClientRequestManager:1169 +for (int j = 0; j < currentValue.getPixels().length; j++) // DatabaseWindowManager:1002 +for (int i=0;i Date: Sun, 23 Aug 2026 22:11:13 -0400 Subject: [PATCH 4/6] docs(geometry): record 5.3b as implemented, and what it did not cover The compressed-and-rehydrate direction is built: #2026 removes the second strong reference GeometrySpec held to the image's pixel array, #2027 makes VCImageCompressed hold its inflated pixels through a SoftReference. Both are open, neither is merged, and the sequencing question in section 6 is still open. Adds section 3.9, the end-to-end check that section 3.7's arithmetic actually changes the outcome. Eleven images, 300^3 each, 256 MB heap: soft cache OFF OUT OF MEMORY after 9 of 11 images soft cache ON completed all 11 With it on the heap drops 246 MB -> 63 MB at image 10 as the collector reclaims, and 283 MB of pixels are re-read on demand. That is the incident and its fix in one run. Rewrites 5.3b as built rather than proposed, and is explicit about what it did NOT cover, because the heading alone would overstate it: - VCImageUncompressed still has no compressed form, so the SAMPLED image is still held strongly. Derived rather than stored, so a different problem. - The per-pixel path is unchanged. The loops that could see a compressed image were hoisted; VCImage.getPixel(x,y,z) and ImageSubVolume.isInside still fetch per call. Cold today, a problem only if either becomes hot. - The label array is untouched -- that is 5.3, and still the largest single retained object. Also corrects two places in section 3.8 that still read as open, updates the branch table with a state column now that drafts and open PRs are mixed, and expands the reproduction commands to include SoftPixelCacheDemo (which needs its own -Xmx, so it does not go through exec:java). Refs #2021, #2026, #2027 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_018kr8SbzXtwW3gMVUgMfDDt --- docs/geometry-memory.md | 173 ++++++++++++++++++++++++++++------------ 1 file changed, 121 insertions(+), 52 deletions(-) diff --git a/docs/geometry-memory.md b/docs/geometry-memory.md index 3ac5c92649..54d51fe6f8 100644 --- a/docs/geometry-memory.md +++ b/docs/geometry-memory.md @@ -1,9 +1,9 @@ # Geometry memory: the incident, what we measured, and what to do about it -**Status: planning document. Nothing here is committed to.** -Three prototype branches exist (#2022, #2023, #2024). They are evidence that the measurements are -real and the fixes are feasible — **not** merge candidates. Decisions belong in this document -first. +**Status: planning document. Nothing here is merged.** +Six branches exist. #2022, #2023 and #2024 are **drafts** — evidence that the measurements are real +and the fixes feasible, not merge candidates. #2026 and #2027 implement §5.3b and are open for +review. Decisions still belong in this document first. Written 2026-08-23 after the 2026-08-22 prod incident. Supersedes `docs/geometry-memory-notes.md` from #2023, which contains an error corrected below. @@ -281,8 +281,8 @@ load-bearing**: any variant that returns a defensive copy silently loses these w **The three retention sites**, only one of which matters: - `GeometrySpec:962` — `uncompressedPixels = getImage().getPixels()` stores the **stored image's** - array in a field. This is the blocker already identified in §5.3b, now confirmed to be the only - one of its kind. + array in a field. This was the blocker identified in §5.3b, and the audit confirmed it is the only + one of its kind. **Removed in #2026.** - `GeometryViewer:257` — the array is handed to `SourceDataInfo`, which keeps it in a `private Serializable data` field for as long as the viewer is open. - `GeometrySummaryPanel:395` — handed to `MemoryImageSource`, which retains it. @@ -307,9 +307,30 @@ why the `uncompressedPixels` cache exists. Softening it without giving those loo the array once would turn a pointer dereference into an inflate. **Verdict.** The correctness risk is much smaller than the raw grep suggested: one mutation, on a -class that cannot be softened anyway. The real work is not correctness but two design problems — -removing the second strong reference at `GeometrySpec:962`, and giving the per-pixel `isInside` path -a bulk or hoisted accessor so it cannot thrash. +class that cannot be softened anyway. The real work was not correctness but two design problems — +removing the second strong reference at `GeometrySpec:962` (**done, #2026**), and giving the +per-pixel `isInside` path a bulk or hoisted accessor so it cannot thrash (**still open**; the loops +that could see a compressed image were hoisted in #2027, but `getPixel(x,y,z)` and `isInside` still +fetch per call — cold today, and a problem only if either becomes hot). + +### 3.9 Does holding the pixels softly change the outcome? + +The arithmetic in §3.7 says it should. This is the end-to-end check, run after §5.3b was built: +`SoftPixelCacheDemo` recreates the shape of the incident — N images all reachable at once — against +a deliberately small heap. + +11 images, 300³ each, **256 MB heap**: + +| | result | +|---|---| +| soft cache **OFF** (previous behaviour) | **OUT OF MEMORY after 9 of 11 images** | +| soft cache **ON** | **completed all 11** | + +With it on, the heap drops from 246 MB to 63 MB at image 10 as the collector reclaims, and 283 MB +of pixels are then re-read on demand at the end. That is the incident and its fix in one run. + +It is a `main` rather than a test: driving a JVM to the edge of its heap does not belong in a shared +test run. --- @@ -420,45 +441,77 @@ Emit geometries once at BioModel level and reference them by key. **Do this before any tiling work** (§5.5): streaming the computation while still emitting a full-size output array saves peak but not retention. -### 5.3b Hold it compressed, rehydrate on demand — **most promising unexplored direction** +### 5.3b Hold it compressed, rehydrate on demand — **IMPLEMENTED** + +> **Status: built, open, not merged.** #2026 (prerequisite) and #2027 (the change itself, stacked +> on it). Both are ordinary PRs rather than drafts, but nothing here is merged and the sequencing +> question in §6 is still open. Raised as: `VCImageCompressed` already keeps compressed pixels strongly and the uncompressed copy as a `transient` cache, so take it one step further and let the uncompressed copy be reclaimable. -§3.7 says the arithmetic works, and works better than expected. Real geometry images compress -**50–100×**, rehydrate at **~700–950 MB/s**, and — the part that was not obvious — the derived -label array compresses **53–93×** as well. +§3.7 said the arithmetic works — real geometry images compress **50–100×** and rehydrate at +**~700–950 MB/s**. §3.9 confirms it end to end: at 256 MB, eleven images **OOM at 9 of 11** with +the old strong cache and **complete** with the soft one. + +**What was built** + +- `VCImageCompressed.softPixels` is a `SoftReference`; `getPixels()` re-inflates on demand. +- `vcell.image.softPixelCache=false` restores the old strong caching. +- The second strong reference at `GeometrySpec:962` is gone (#2026) — see below. +- Two loops hoisted; two deliberately left alone. -**Use `SoftReference`, not `WeakReference`.** This distinction decides whether the idea works. A +**`SoftReference`, not `WeakReference`** — the distinction that decides whether the idea works. A weak reference is cleared at the next GC *regardless of memory pressure*, so a hot geometry would -re-inflate on essentially every collection: 90 ms of CPU repeatedly, for no benefit. A soft -reference is cleared only when the heap is actually under pressure, and the JVM ages them by -`-XX:SoftRefLRUPolicyMSPerMB`. Soft is the tool that gives "does not contribute to memory -pressure" without giving up the cache. - -Three things block a naive version, all verified: - -1. **`GeometrySpec.uncompressedPixels` (line 67) holds a second strong reference to the very same - array** (`getImage().getPixels()` at line 962). Softening `VCImageCompressed.uncompressed` alone - changes nothing while that field pins it. They have to move together. -2. **`VCImageUncompressed` has no compressed form** — `private final byte pixels[]`. The sampled - image from `createSampledImage` is one of these, so it cannot be softened as-is; it would need - either a compressed backing or a documented recompute path (it *is* derivable from the image - plus subvolume handles). -3. **A compressed label map is not randomly accessible**, and `RegionInfo.isIndexInRegion(index)` - needs random access. Two ways out: compress in fixed blocks and cache a few decompressed blocks, - or use RLE, which supports random access by binary search over runs and is a natural fit for - run-coherent segmented volumes. The second doubles as option §5.3. - -**The `getPixels()` audit — done, and the surface is far smaller than feared.** See §3.8. - -**Precedent worth knowing about:** `VCImageCompressed.nullifyUncompressedPixels()` already exists -and has exactly one caller, `GeomDbDriver:490`. Someone hit this problem before and solved it by -hand in a single place. - -**Why this is worth the deep investigation:** it attacks retained memory rather than peak, it needs -no format change, and correctness is verifiable against a large corpus of stored models — the -inflated bytes either match or they do not. +re-inflate on essentially every collection: ~90 ms of CPU repeatedly, for no benefit. Soft +references are cleared only under real pressure, aged by `-XX:SoftRefLRUPolicyMSPerMB`. + +Because nothing about ordinary behaviour would look wrong if someone swapped the type, there is a +test whose only job is to fail if they do (`ordinaryGcDoesNotDiscardThePixels`), verified by +temporarily switching to `WeakReference` and watching it fail on a changed identity hash. + +**The three blockers, resolved** + +1. **`GeometrySpec.uncompressedPixels` held a second strong reference** to the same array, which + would have pinned it regardless. **Removed in #2026.** It turned out to be a *redundant* cache — + `VCImageCompressed.getPixels()` already caches and `VCImageUncompressed.getPixels()` returns its + field directly — feeding a cold path (the only caller reaching an `ImageSubVolume` is + `getSubVolume(x,y,z)`, whose sole caller samples 100 points on a curve). Its test asserts + *reachability*, not structure, so it still catches the problem if the reference reappears + elsewhere. +2. **`VCImageUncompressed` has no compressed form** — `private final byte pixels[]`. **Still true, + and deliberately untouched.** The sampled image from `createSampledImage` is one of these, so it + is not softened. It would need either a compressed backing or a documented recompute path (it + *is* derivable from the image plus subvolume handles). See "what remains". +3. **A compressed label map is not randomly accessible.** **Not addressed here** — that is + `mapImageIndexToLinkRegion`, which belongs to §5.3, not to the image cache. + +**One hazard that had to be designed out.** `getPixels()` takes a strong local reference once and +holds it for the whole method. Reading the `SoftReference` twice would let the collector clear it +between the null check and the return and hand the caller a **null array** — rare, +non-deterministic, and extremely unpleasant to diagnose. + +**Loop hoisting, and where it was declined.** `ClientRequestManager:1169` (reached as +`getGeometrySpec().getImage()` — the stored image) and `ImageFile:149/153` call `getPixels()` every +iteration and *can* receive a compressed image; under the very pressure that clears a soft +reference they would have re-inflated per iteration, so both were hoisted. +`ROIMultiPaintManager:2036` and `ITextWriter:544` were **not** touched: their receivers are locally +built `VCImageUncompressed` instances that never inflate, so a rewrite there would have been risk +for no benefit. + +**What remains** + +- **The per-pixel path.** `VCImage.getPixel(x,y,z)` is `getPixels()[index]`, and + `ImageSubVolume.isInside` goes through `GeometrySpec.getUncompressedPixels()` one pixel at a + time. Both are cold today (§3.8), but either becoming hot would want a bulk or hoisted accessor + rather than a per-call fetch. +- **The sampled image is still held strongly** (blocker 2 above). It is derived rather than stored, + so it is a different problem from the one solved here. +- **The label array** — §5.3, unchanged and still the largest single retained object. + +**Precedent worth knowing about:** `VCImageCompressed.nullifyUncompressedPixels()` already existed +with exactly one caller, `GeomDbDriver:490`. Someone hit this problem before and solved it by hand +in a single place. ### 5.4 Limit new submissions, grandfather what is stored @@ -549,10 +602,11 @@ Independent of all the above, and cheap: 2. **What region count?** Same problem; 2,000 is a guess. A tissue image with hundreds of separate cells is ordinary science. **Needs a survey of region counts.** 3. **Is the VCML format change (§5.2) on the table at all?** Everything else is compensation for it. -4. **Sequencing.** Suggested: §5.1 → §5.4 (with real numbers) → §5.3/§5.3b → §5.5 → §5.6. §5.9 can - happen immediately and independently. §5.3b is the one worth a real investigation rather than a - quick implementation: the numbers are strong, and the risk is entirely in the 178-call-site - `getPixels()` audit rather than in the arithmetic. +4. **Sequencing.** §5.3b is now built (#2026, #2027) and §5.1 is prototyped (#2024); neither is + merged. Remaining suggested order: §5.1 → §5.4 (with real numbers) → §5.3 → §5.5 → §5.6. §5.9 + can happen immediately and independently. **Whether §5.3b and §5.1 should land before the rest + is decided is itself an open question** — they are independent of each other and of the format + question in §5.2. 5. **Who owns the duplicate-row cleanup**, and should it wait for §5.1 to stop new ones first? 6. **Do the duplicate images in a group belong to one BioModel?** One query settles it; needs `vcell_dev` credentials. @@ -571,11 +625,14 @@ login, which is the exact lockout risk. Supply it explicitly and try once. **Prototype branches — do not merge as-is:** -| PR | Branch | Contains | -|---|---|---| -| #2022 | `fix/geometryspec-image-veto` | Submission-time limits (§5.4), grandfathering | -| #2023 | `perf/geometry-memory-profiler` | `GeometryMemoryProfiler` + the notes file this supersedes | -| #2024 | `perf/parse-geometry-once` | Image sharing (§5.1) + `XmlGeometrySharingBenchmark` | +| PR | Branch | Contains | State | +|---|---|---|---| +| #2022 | `fix/geometryspec-image-veto` | Submission-time limits (§5.4), grandfathering | draft | +| #2023 | `perf/geometry-memory-profiler` | `GeometryMemoryProfiler` + the notes file this supersedes | draft | +| #2024 | `perf/parse-geometry-once` | Image sharing (§5.1) + `XmlGeometrySharingBenchmark` | draft | +| #2025 | `docs/geometry-memory-plan` | this document | open | +| #2026 | `fix/geometryspec-pixel-retention` | removes the second strong reference (§5.3b prerequisite) | open | +| #2027 | `perf/soft-pixel-cache` | the SoftReference itself (§5.3b) + `SoftPixelCacheDemo`, stacked on #2026 | open | The instrumentation in #2023 is the piece most worth keeping regardless of which direction is chosen: every number in this document came from it, and the corrections in §4 were only possible @@ -585,13 +642,25 @@ because it existed. ```bash mvn test-compile -pl vcell-core -am + +# §3.2/§3.3 — per-phase peak/retained/allocated for one geometry (#2023) mvn -q -pl vcell-core exec:java -Dexec.classpathScope=test \ -Dexec.mainClass=cbit.vcell.geometry.GeometryMemoryProfiler + +# §3.1 — parse cost by sharing mode, N applications over one image (#2024) mvn -q -pl vcell-core exec:java -Dexec.classpathScope=test \ -Dexec.mainClass=cbit.vcell.xml.XmlGeometrySharingBenchmark + +# §3.9 — soft cache on vs off against a small heap; needs the -Xmx, so run (#2027) +# it directly rather than through exec:java +java -Xmx256m -Dvcell.image.softPixelCache=false -cp \ + cbit.image.SoftPixelCacheDemo 11 300 +java -Xmx256m -Dvcell.image.softPixelCache=true -cp \ + cbit.image.SoftPixelCacheDemo 11 300 ``` -Add `-Dprofiler.hold=120` to keep structures reachable for `jcmd GC.class_histogram`. +Add `-Dprofiler.hold=120` to the profiler to keep structures reachable for +`jcmd GC.class_histogram`. **Related:** #2021 (the issue — note its framing of "eleven parses exhausted the heap" is corrected by §4.1/§4.2), #1899 (heap sizing, why this was diagnosable), #1890/#1895/#1896 (the watertightness From fe2d879bf34e1a708fa67dddd22b2daa06ba5d2c Mon Sep 17 00:00:00 2001 From: Jim Schaff Date: Sun, 23 Aug 2026 22:57:03 -0400 Subject: [PATCH 5/6] docs(geometry): mark 5.3b merged #2026 (8a6e9f78e2) and #2027 (620ccd56db) landed on master 2026-08-24. Admin-merged because master requires a review and none was available, which bypasses the merge queue, so regression.yml was triggered manually against master. Everything else in this document is still open: #2022, #2023 and #2024 remain drafts and none of the decisions in section 6 have been made. Refs #2021, #2026, #2027 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_018kr8SbzXtwW3gMVUgMfDDt --- docs/geometry-memory.md | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/docs/geometry-memory.md b/docs/geometry-memory.md index 54d51fe6f8..2af6d88081 100644 --- a/docs/geometry-memory.md +++ b/docs/geometry-memory.md @@ -1,9 +1,10 @@ # Geometry memory: the incident, what we measured, and what to do about it -**Status: planning document. Nothing here is merged.** -Six branches exist. #2022, #2023 and #2024 are **drafts** — evidence that the measurements are real -and the fixes feasible, not merge candidates. #2026 and #2027 implement §5.3b and are open for -review. Decisions still belong in this document first. +**Status: planning document.** +§5.3b is **merged to master** (#2026 `8a6e9f78e2`, #2027 `620ccd56db`, 2026-08-24). Everything else +is still open: #2022, #2023 and #2024 are **drafts** — evidence that the measurements are real and +the fixes feasible, not merge candidates — and the decisions in §6 have not been made. Those +decisions still belong in this document first. Written 2026-08-23 after the 2026-08-22 prod incident. Supersedes `docs/geometry-memory-notes.md` from #2023, which contains an error corrected below. @@ -443,9 +444,9 @@ full-size output array saves peak but not retention. ### 5.3b Hold it compressed, rehydrate on demand — **IMPLEMENTED** -> **Status: built, open, not merged.** #2026 (prerequisite) and #2027 (the change itself, stacked -> on it). Both are ordinary PRs rather than drafts, but nothing here is merged and the sequencing -> question in §6 is still open. +> **Status: MERGED to master 2026-08-24.** #2026 (prerequisite, merge commit `8a6e9f78e2`) and +> #2027 (the change itself, `620ccd56db`). Admin-merged, so the merge queue was bypassed and +> `regression.yml` was triggered manually against master. Raised as: `VCImageCompressed` already keeps compressed pixels strongly and the uncompressed copy as a `transient` cache, so take it one step further and let the uncompressed copy be reclaimable. @@ -602,11 +603,9 @@ Independent of all the above, and cheap: 2. **What region count?** Same problem; 2,000 is a guess. A tissue image with hundreds of separate cells is ordinary science. **Needs a survey of region counts.** 3. **Is the VCML format change (§5.2) on the table at all?** Everything else is compensation for it. -4. **Sequencing.** §5.3b is now built (#2026, #2027) and §5.1 is prototyped (#2024); neither is - merged. Remaining suggested order: §5.1 → §5.4 (with real numbers) → §5.3 → §5.5 → §5.6. §5.9 - can happen immediately and independently. **Whether §5.3b and §5.1 should land before the rest - is decided is itself an open question** — they are independent of each other and of the format - question in §5.2. +4. **Sequencing.** §5.3b is **merged** (#2026, #2027). §5.1 is prototyped (#2024) and not merged. + Remaining suggested order: §5.1 → §5.4 (with real numbers) → §5.3 → §5.5 → §5.6. §5.9 can happen + immediately and independently of all of it. 5. **Who owns the duplicate-row cleanup**, and should it wait for §5.1 to stop new ones first? 6. **Do the duplicate images in a group belong to one BioModel?** One query settles it; needs `vcell_dev` credentials. @@ -631,8 +630,8 @@ login, which is the exact lockout risk. Supply it explicitly and try once. | #2023 | `perf/geometry-memory-profiler` | `GeometryMemoryProfiler` + the notes file this supersedes | draft | | #2024 | `perf/parse-geometry-once` | Image sharing (§5.1) + `XmlGeometrySharingBenchmark` | draft | | #2025 | `docs/geometry-memory-plan` | this document | open | -| #2026 | `fix/geometryspec-pixel-retention` | removes the second strong reference (§5.3b prerequisite) | open | -| #2027 | `perf/soft-pixel-cache` | the SoftReference itself (§5.3b) + `SoftPixelCacheDemo`, stacked on #2026 | open | +| #2026 | `fix/geometryspec-pixel-retention` | removes the second strong reference (§5.3b prerequisite) | **merged** `8a6e9f78e2` | +| #2027 | `perf/soft-pixel-cache` | the SoftReference itself (§5.3b) + `SoftPixelCacheDemo` | **merged** `620ccd56db` | The instrumentation in #2023 is the piece most worth keeping regardless of which direction is chosen: every number in this document came from it, and the corrections in §4 were only possible From 734628209096e5ded3ac0fc290e5c3be32f9ce5d Mon Sep 17 00:00:00 2001 From: Jim Schaff Date: Mon, 24 Aug 2026 09:01:15 -0400 Subject: [PATCH 6/6] docs(geometry): reject 5.1, make the explicit geometry list the direction Records why #2024 was closed rather than deferred, so nobody re-derives it. It is not needed. It was designed when the incident looked like 11 x 62 MB of unavoidable retention; 5.3b removed that premise. With the pixels held softly the retained floor for the same document drops from ~680 MB to ~11 MB, without deduplicating anything. The win was also overstated, and section 4.6 now records that as one of my own errors. The 11x figure came from a synthetic document built with eleven IDENTICAL copies -- the benchmark assumed the thing it was meant to test. Measured across 23 corpus models carrying 103 image elements (new section 3.10): today 103 decodes digest of the element 66 (36%) by database key 66 (36%) by pixel payload 52 (50%) biomodel_27192717 shows why: 9 image elements, 8 distinct names, 8 distinct keys, and only 2 distinct pixel payloads. The duplicate-insert bug had already renamed every copy and given each its own row, and the name and version sit INSIDE the digested element -- so the bug defeats its own detection. And the design was wrong regardless: the digest matched neither declared identity (KeyValue, which the save controller effectively uses) nor content identity, and sharing instances needs a client-side split on write that it had no mechanism for. 5.2 is rewritten from a VCML format change to giving a BioModel an explicit list of geometries, edited directly -- Jim's framing, and a better one. Modelling the sharing answers every objection that sank 5.1 at the source: declared identity for the recursive, non-atomic incremental save controller to key on, a visible home for copy-on-write, and it matches what the database already stores. VCML is the layer that discards the relationship by inlining. New 5.10 salvages the one useful part of #2024 as its own item: VCImage overrides neither equals nor hashCode while saveBioModel keys a hashtable by it, so a new image is inserted once per application. Includes the finding that SQLCreateAllTables.writeScript(POSTGRES, ...) already emits the whole schema, so a testcontainer can build the real thing rather than a stub -- there is currently no test of the document save path at all. Sequencing reordered accordingly: 5.9 (free) -> 5.10 (stop creating duplicates) -> 5.8 (clean up the existing ones) -> 5.4 -> 5.2. Refs #2021, #2024, #2025 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_018kr8SbzXtwW3gMVUgMfDDt --- docs/geometry-memory.md | 164 ++++++++++++++++++++++++++++++++-------- 1 file changed, 132 insertions(+), 32 deletions(-) diff --git a/docs/geometry-memory.md b/docs/geometry-memory.md index 2af6d88081..a09d6ac67a 100644 --- a/docs/geometry-memory.md +++ b/docs/geometry-memory.md @@ -1,10 +1,16 @@ # Geometry memory: the incident, what we measured, and what to do about it **Status: planning document.** -§5.3b is **merged to master** (#2026 `8a6e9f78e2`, #2027 `620ccd56db`, 2026-08-24). Everything else -is still open: #2022, #2023 and #2024 are **drafts** — evidence that the measurements are real and -the fixes feasible, not merge candidates — and the decisions in §6 have not been made. Those -decisions still belong in this document first. + +**Merged:** §5.3b, the compressed-and-rehydrate work (#2026, #2027) — this fixes the incident. +Plus the geometry regression suite (#2028, #2029), which is what makes further change in this area +checkable rather than assertable. + +**Rejected:** §5.1, parse-time image sharing (#2024 closed) — not needed once §5.3b landed, and +measured at 36% rather than the ~90% a synthetic benchmark suggested. + +**Still open:** #2022 and #2023 remain drafts, and the decisions in §6 have not been made. §5.2 is +now the recommended direction rather than a fallback. Written 2026-08-23 after the 2026-08-22 prod incident. Supersedes `docs/geometry-memory-notes.md` from #2023, which contains an error corrected below. @@ -333,6 +339,38 @@ of pixels are then re-read on demand at the end. That is the incident and its fi It is a `main` rather than a test: driving a JVM to the edge of its heap does not belong in a shared test run. +### 3.10 What "the same image" means, measured on real documents + +The parse-sharing idea (§5.1) assumed a document's repeated geometries are byte-identical. Synthetic +benchmarks were built that way. Real stored models are not. + +Across 23 corpus models carrying 103 `` elements: + +| identity notion | decodes | saved | +|---|--:|--:| +| today | 103 | — | +| digest of the whole `` element | 66 | 36% | +| by database key (`KeyValue`) | 66 | 36% | +| by pixel payload alone | 52 | **50%** | + +`biomodel_27192717` shows why: **9 image elements, 8 distinct names, 8 distinct keys — and only 2 +distinct pixel payloads.** + +The names and keys differ because of the duplicate-insert bug (§3.6): each copy was inserted +separately and renamed by `TokenMangler`, so the document carries +`purkinje9_3D_crop1017221890`, `…1846473978` and eight separate `KeyValue`s for what is really two +images. **The name and version sit inside the element**, so a content digest over the element is +defeated by the very bug it was meant to help. + +The three notions are not interchangeable: + +- **Declared identity** (`KeyValue`) is what `ServerDocumentManager` effectively uses — its + `memoryToDatabaseHash` is keyed by object reference standing in for row identity. Sharing on this + is correct but, on already-duplicated data, buys little. +- **Content identity** (payload) saves the most but *merges rows the database considers distinct*. + That is a data-model decision, not a parser optimisation, and on the next save it would collapse + two `vc_image` rows into one. + --- ## 4. What we got wrong along the way @@ -386,7 +424,13 @@ A raw grep count, quoted in this document before the audit was done. It counted `ByteImage`, `UShortImage`, `FloatImage` and `ImageDataset`, which are unrelated classes. The real figure is **39 call instructions in 22 files**. See §3.8. -### 4.6 The first duplicate-row query over-matched +### 4.6 "Sharing the decoded image gives an 11x win" + +Measured on a synthetic document with eleven *identical* copies, then quoted as the expected result. +On real corpus models the same mechanism eliminates **36%** of decodes, not ~90% (§3.10). The +benchmark was built out of the assumption it was meant to test. + +### 4.7 The first duplicate-row query over-matched See §3.6. 12,629 → ~944. @@ -397,33 +441,60 @@ See §3.6. 12,629 → ~944. Ordered by measured value, not by appeal. Each states what it buys, what it costs, and what is unresolved. -### 5.1 Share the decoded `VCImage` within one document parse — **recommended first** +### 5.1 Share the decoded `VCImage` within one document parse — **REJECTED** + +> **Closed as #2024.** Not deferred: the premise it rested on no longer holds, and the reasoning is +> recorded here so nobody re-derives it. + +The idea: cache decoded images by digest of their `` element, for the life of one parse. + +**Why it is not needed.** It was designed when the incident looked like 11 × 62 MB of *unavoidable* +retention. §5.3b removed that premise — with the pixels held softly the retained floor for the same +document drops from ~680 MB to **~11 MB** (§3.9), without deduplicating anything. + +**Why the win was smaller than advertised.** 36% of decodes on real models, not the ~90% the +synthetic benchmark suggested (§3.10, §4.6). -Cache decoded images by digest of their `` element, for the life of one `XmlReader` (one -document). +**Why the design was wrong regardless.** The digest matched neither declared identity (`KeyValue`, +which the save controller effectively uses) nor content identity (the payload). And sharing +instances requires a **client-side split on write**, which it had no mechanism for: even confined to +`VCImage`, `setName` / `setPixelClasses` / `setDescription` mutate the shared instance, and +`ServerDocumentManager` calls `setName` on images in four places during save (`saveBioModel:840`, +`saveGeometry:1577`, `saveMathModel:1717`, `saveSimulation:2128`). -- **Buys:** the whole measured win — peak 787 → 162 MB, retained 661 → 60 MB, 8.7× faster (§3.1). -- **Risk:** low. A `VCImage` is immutable payload: compressed pixels are `final`, - `VCPixelClass implements Immutable` with no setters, and everything editable — subvolumes, their - names, extent, origin, surfaces — lives on the per-application `GeometrySpec`. -- **Also fixes** the duplicate-row insert (§3.6), because the identity-keyed hashtable then sees one - object. -- **Prototype:** #2024. `MathGen_IT` 1045 tests green. +**The one thing worth salvaging** is the duplicate-insert fix, which #2024 got only as a side +effect. `VCImage` overrides neither `equals` nor `hashCode` while `saveBioModel` keys +`memoryToDatabaseHash` by it, so N distinct-but-equal objects become N inserted rows (§3.6). That is +a targeted defect with a small blast radius and belongs in its own PR — see §5.10. -**Explicitly not** sharing the `Geometry` object. Five applications sharing one mutable `Geometry` -would see each other's subvolume renames — unexpected, and measured to buy nothing on top of image -sharing. The prototype keeps it behind an off-by-default property for read-only server use only. +### 5.2 Give a BioModel an explicit list of geometries — **recommended direction** -### 5.2 Stop writing the geometry once per application (VCML format) +Rather than have a parser infer that two inlined geometries are one, **model the sharing**: a +BioModel owns a list of geometries, applications reference them, and the user edits that list +directly. -Emit geometries once at BioModel level and reference them by key. +This is a user-facing model change, not merely a serialization change, and that is the point. It +resolves the objections to §5.1 at the source instead of patching each one: -- **Buys:** ~11× smaller VCML for these models, on top of the parse win. Removes the root cause - rather than compensating for it. -- **Costs:** a format change. Old clients must still read new documents, and the migration story - needs design. Bigger than anything else on this list. -- **Unresolved:** whether the reference should be a database key or a document-local id; what - happens for a model whose applications genuinely differ. +- **Identity is declared.** `ServerDocumentManager`'s incremental save is recursive, iterative and + non-atomic: it decides what needs saving and propagates keys to parents when a child changes + (`saveBioModel:880` — when the image was saved, the geometry is re-pointed at the database + instance and forced to save, which pushes a new geometry key upward). That controller needs a real + notion of "same geometry" to key on. A declared reference gives it one; a parse-time digest does + not. +- **Copy-on-write gets a home.** Splitting a shared instance becomes a visible user action — copy + this geometry — rather than something a parser decides silently. +- **It matches the database.** Geometries and images are already keyed rows with + `vc_geometry.imageref`. VCML is the layer that discards the relationship by inlining a full copy + into every `` (`Xmlproducer:1416`). +- **It stops new duplicates at the source**, rather than deduplicating after the fact. + +**Costs.** A format change, with old clients still needing to read new documents, and UI work to +expose the list. The largest item on this page. + +**Open:** whether the reference is a database key or a document-local id; what happens to a model +whose applications genuinely have different geometries (they keep separate entries — the list is a +list, not a singleton); and the migration path for the ~944 already-duplicated rows (§5.8). ### 5.3 Narrow the per-pixel label array @@ -593,6 +664,27 @@ Independent of all the above, and cheap: - **api heap.** 1000 MB, sized in #1899. Whether that is still right is a separate question from whether one request should need 680 MB. +### 5.10 Fix the duplicate image inserts directly + +Salvaged from §5.1, on its own merits rather than as a side effect. + +`saveBioModel` keys `memoryToDatabaseHash` by the `VCImage` object, and `VCImage` overrides neither +`equals` nor `hashCode`. The XML round-trip inside the save produces N distinct-but-equal objects +for one image, so a **new** image (`key == null`) falls into the insert branch N times and is written +as N rows with mangled names — ~944 surplus rows in production, still accruing as of October 2025 +(§3.6). + +- **Buys:** stops the corruption that also defeats content-based identity (§3.10). A prerequisite + for §5.2 being effective on existing data. +- **Costs:** small and contained, but it is the document save path, which has **no test at all** + today. Wants the testcontainer harness below. +- **Testing:** `SQLCreateAllTables.writeScript(POSTGRES, bootstrapData, writer)` already emits the + entire VCell schema — every table, the sequence, indices and bootstrap rows. A PostgreSQL + testcontainer can therefore build the *real* schema rather than the stub schema + `DatabaseCleanupOracleSqlTest` uses, and PostgreSQL means it can run in the fast lane. Assertions + worth having: a new N-application model writes **one** `vc_image` row; re-saving unchanged writes + nothing; replacing one application's geometry leaves the others' rows alone. + --- ## 6. Open questions — decisions needed before implementation @@ -602,11 +694,17 @@ Independent of all the above, and cheap: scientific limit. **Needs a survey of image sizes actually in the database.** 2. **What region count?** Same problem; 2,000 is a guess. A tissue image with hundreds of separate cells is ordinary science. **Needs a survey of region counts.** -3. **Is the VCML format change (§5.2) on the table at all?** Everything else is compensation for it. -4. **Sequencing.** §5.3b is **merged** (#2026, #2027). §5.1 is prototyped (#2024) and not merged. - Remaining suggested order: §5.1 → §5.4 (with real numbers) → §5.3 → §5.5 → §5.6. §5.9 can happen - immediately and independently of all of it. -5. **Who owns the duplicate-row cleanup**, and should it wait for §5.1 to stop new ones first? +3. **Is the explicit geometry list (§5.2) on the table?** It is now the recommended direction rather + than a fallback: every objection that sank §5.1 — declared identity for the incremental-save + controller, a home for copy-on-write, matching what the database already stores — is answered by + modelling the sharing instead of inferring it. It is also the largest item here, and it is a + user-facing change, not just a format one. +4. **Sequencing.** §5.3b is **merged** (#2026, #2027) and fixes the incident. §5.1 is **rejected** + (#2024 closed). Suggested order from here: **§5.9** (free, removes the trigger) → **§5.10** (stop + creating duplicates, with the testcontainer harness) → **§5.8** (clean up the ~944 existing rows) + → **§5.4** with real numbers → **§5.2** as the real answer for multi-application models. §5.3, + §5.5 and §5.6 are memory work that nothing currently forces. +5. **Who owns the duplicate-row cleanup** (§5.8), and it should wait for §5.10 to stop new ones first — otherwise the cleanup is undone by the next multi-application save. 6. **Do the duplicate images in a group belong to one BioModel?** One query settles it; needs `vcell_dev` credentials. @@ -628,10 +726,12 @@ login, which is the exact lockout risk. Supply it explicitly and try once. |---|---|---|---| | #2022 | `fix/geometryspec-image-veto` | Submission-time limits (§5.4), grandfathering | draft | | #2023 | `perf/geometry-memory-profiler` | `GeometryMemoryProfiler` + the notes file this supersedes | draft | -| #2024 | `perf/parse-geometry-once` | Image sharing (§5.1) + `XmlGeometrySharingBenchmark` | draft | +| #2024 | `perf/parse-geometry-once` | Image sharing (§5.1) | **closed — rejected, see §5.1** | | #2025 | `docs/geometry-memory-plan` | this document | open | | #2026 | `fix/geometryspec-pixel-retention` | removes the second strong reference (§5.3b prerequisite) | **merged** `8a6e9f78e2` | | #2027 | `perf/soft-pixel-cache` | the SoftReference itself (§5.3b) + `SoftPixelCacheDemo` | **merged** `620ccd56db` | +| #2028 | `test/geometry-surface-goldens` | geometry golden suite, synthetic fixtures | **merged** `4246d369d9` | +| #2029 | `test/geometry-corpus-goldens` | corpus fixtures + the `Geometry_IT` group | **merged** `f9cd0c386a` | The instrumentation in #2023 is the piece most worth keeping regardless of which direction is chosen: every number in this document came from it, and the corrections in §4 were only possible