perf(imaging): route morphology through Leptonica's DWA kernels, exactly#33
Closed
P4suta wants to merge 17 commits into
Closed
perf(imaging): route morphology through Leptonica's DWA kernels, exactly#33P4suta wants to merge 17 commits into
P4suta wants to merge 17 commits into
Conversation
Nothing in the pipeline measured where a run's time went: ProgressEvents
carry no timestamps and the stage logs no durations, so optimization
work had no baseline to argue against. This adds the measurement layer:
- --timings: a StageTimingSink (composed in the CLI shell) prints a
stable, machine-parseable per-stage breakdown to stderr when a run
ends ("timing: <stage> = <seconds>s (<percent>%)"), including the
still-open stage on failure.
- PipelineRunner logs each stage directory's byte total, making the
intermediate I/O of every stage visible.
- benchPipeline: a Gradle task driving the installDist launcher with
--timings (PipelineBenchmark, test sources), measuring E2E wall, the
per-stage medians, peak RSS via /proc VmHWM, and output size, over a
-Pjobs sweep; writes pipeline/docs/perf-baseline.md.
- createSampleScan: a deterministic synthetic 600-dpi A5 scan book
(specks for despeckle, ±0.5° skew for deskew) so the benchmark needs
no copyrighted input and stays comparable across machines.
Baseline on the 200-page fixture (8 CPUs): conv 14.48s at -j8 —
despeckle 68%, register 22.6%, extract 7.9%, spread 1.5% — and a
3.44x scale-up from -j1, recorded in pipeline/docs/perf-baseline.md.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
pdfimages -tiff decodes every embedded G4 image into an uncompressed TIFF (~2.2 MB per 600-dpi page; ~434 MB of transient intermediates for a 200-page book) even though the typical self-scanned source is CCITT G4 end to end. (The originally planned `-tiffcompression g4` flag does not exist on pdfimages — it is a pdftoppm option.) The extractor now picks its mode from one pdfimages -list pass: when every embedded image is 1-bpp CCITT, each chunk dumps the raw G4 streams (-ccitt) and CcittTiffs wraps them verbatim into single-strip CCITT-G4 TIFFs — a pure remux: no decode/re-encode, intermediates drop ~60x, and the image's true ppi is stamped instead of pdfimages' default 72 dpi. Because PDF's EncodedByteAlign never reaches the dumped .params file, every wrapped page is decoded back once through Leptonica as verification; a chunk that deviates in any way (params shape, count, or a wrap that fails to decode) is re-extracted decoded, which is also the whole-run mode for any non-CCITT source. The photometric mapping (-B -> WhiteIsZero, -W -> BlackIsZero) is pinned empirically by a pixel-identical round trip test. Extraction chunks also shrink from total/jobs to ~12 pages (capped at 4*jobs): fast finishers free their pool slot early, and a future streaming source can consume pages chunk by chunk. Benchmark (200-page fixture, warm median of 3, vs the PR #28 baseline): extract 1.15s -> 0.46s at -j8 (4.57s -> 0.88s at -j1), conv 49.85s -> 45.98s (-7.8%) at -j1, intermediates ~434 MB -> ~7 MB. Output validated with qpdf --check (100 spreads, linearized, no errors). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The shared Tasks.awaitAll (and its two private copies in despeckle/ register) had three real defects: - No fail-fast: invokeAll waits for every page even after the first failure — a corrupt page at position 1 of 600 still ran the other 599. - Error kinds were destroyed at the pool boundary: a worker's domain exception (DespeckleException, RegisterException — RuntimeExceptions carrying their ErrorKind) was wrapped into a generic IOException, so ExceptionMapper saw INTERNAL instead of the real kind: wrong exit code, wrong RunFailed token in the webapp's SSE stream. - ProcessRunner leaked the child on interrupt: an InterruptedException from waitFor propagated without destroyForcibly, leaving the drainer close() hanging on a live child's pipes. Tasks is rewritten around a batch-owned executor (try-with-resources) with StructuredTaskScope.join() semantics on final-Java features: fail-fast with sibling interruption plus a gate that stops queued tasks a freed worker dequeues in the shutdownNow race window, quiescence before the failure propagates (so finally-deletes never race in-flight writers), and exception identity preserved (IOException/ RuntimeException unchanged, UncheckedIOException unwrapped). Two scheduling modes: Workers.platform(jobs) for CPU-bound Leptonica/FFM work (long downcalls pin virtual threads' carriers) and Workers.virtual(maxInFlight) for subprocess waits (pdfimages chunks, per-page jbig2), semaphore-bounded. ItemProgress now fires on the orchestrating thread in completion order, so per-page progress events are strictly ordered and the AtomicInteger counters at every call site are gone. All seven call sites migrate (G4EncodeStage, DespeckleService, RegistrationService both passes, PdfExtractSource, the shared extractor/assembler, Jbig2PackService, both PdfPipelineServices — whose redundant outer pools, idle through the whole middle step, are removed); the ExecutorService parameter disappears from the PdfImageExtractor/Jbig2Assembler ports; the dead NativeTools.awaitAll copies (zero callers) are deleted. pdfbook's Main also gains a shutdown hook that interrupts the run and waits (bounded 15s) for it to unwind, so Ctrl-C now kills the children, quiesces the workers, and deletes the p4suta-pipeline- work area instead of leaking it. Verified in-container: SIGINT to the process group aborts the run (exit 130), no work dir remains. Benchmark: no regression (conv 14.29s vs 14.23s at -j8 on the 200-page fixture, within noise). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
despeckle dominates pdfbook conversions (~72% of conv), but nothing measured WHERE inside clean() the ~50ms/page/core goes. benchCleaner times each Leptonica primitive the cleaner composes — read, the four selectBySize shapes (incl. the inverted-page variant whose giant background component is rendered back), the 43x43 dilate, the 7x7 open, the boolean ops, both counting passes, the G4 write — plus clean() end-to-end, on a deterministic synthetic 600-dpi A5 page, and writes the table to despeckle/docs/cleaner-baseline.md. The committed baseline (174.9ms clean(), sigma row covers 92.5%): dilate 43x43 = 21.5%, selectBySize on the inverted page x2 = 25.3% (22.2ms vs 15.2ms on the normal page — the background-component re-render penalty, measured), metrics-only countConnComp x2 = 13.5%, page selectBySize x2 = 17.4%, open 7x7 = 7.0%. Every following optimization is judged against this table. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
clean() counted 8-connected components before and after every page — two full connected-component labelings, 13.5% of the page's time per the committed baseline — yet the counts feed nothing but the HTML report's charts and the summary log line. The pdfbook pipeline never sets a report dir, so its hot path paid for numbers nobody read. ProcessResult's component counts become OptionalInt (a counted 4-int convenience constructor keeps every existing construction compiling unchanged; componentsRemoved() stays present-or-zero int, documented). ProcessOptions grows collectComponentStats (default true via the delegating 5-knob constructor) plus a withoutComponentStats() wither. DespeckleService turns counting off exactly when no reportDir is set — the report path is byte-identical, including its charts and totals. The black-pixel counts still run unconditionally: they feed the 3% over-removal guard. Summary logs switch to always-measured black-pixel terms when counting was skipped. Measured: clean() 161.8ms -> 139.5ms single-threaded (-13.8%, matching the baseline's countConnComp share); pipeline at -j8 despeckle stage 10.19s -> 9.57s (-6.1%, the saving partially absorbed by memory- bandwidth saturation), conv -4.4%. The benchmark gains a "clean() without component stats" row pinning the pipeline-path number. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The cleaner's two heaviest morphology ops ran on the generic rasterop bricks: the 43x43 isolated-dust dilation (37.5ms/page, the single hottest op in the baseline) and the 7x7 fill-holes opening (12.2ms). Leptonica ships word-accelerated DWA kernels for both, unbound. Binding them surfaced a real trap the planned equality sweep was built to catch: pixDilateBrickDwa silently diverges from the generic brick for sel sizes missing from the generated table (every prime above 15 — including the production 43), while pixOpenBrickDwa is exact at every size and dilate is exact up to 15. The shipped routing is therefore: - dilated(): a single DWA pass up to size 15, larger sizes composed from safe-size DWA passes — exact by Minkowski sum (brick(a) then brick(b) equals brick(a+b-1); clipping per pass changes nothing inside the image rectangle since L-inf paths between in-bounds points stay in bounds), and version-robust (only the always-present small sels are used). - opened(): DWA up to size 15 (production is 7x7), generic beyond (an opening does not compose from smaller passes). The generic variants stay as package-private oracles, and PixTest pins pixel-identity across radii 0..31 (sel 1..63) on border-touching ink plus degenerate pages. Rider: pixCountPixels now reuses one process-lifetime popcount table instead of rebuilding it per call. Measured: dilate 43x43 37.5 -> 14.1ms (-62%), open 7x7 12.2 -> 4.0ms (-67%); clean() on the pipeline path 139.5 -> 107.6ms; despeckle stage at -j8 9.57 -> 5.47s (-43%, the bandwidth relief compounds across workers), conv 13.61 -> 9.51s (-30%; -34% vs the original baseline). selectBySize is now ~70% of the remaining clean() — the measured gate for the follow-up selection restructuring. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The CI spell check flags PDFBox's COSName.DECODE_PARMS (the PDF spec's own key name, which the remux test must name verbatim) and a hyphenated coinage in the extractor's javadoc. Allowlist the spec identifier — the same precedent as the veraPDF en-GB names — and use plain words. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The test released its gate from inside the ok tasks, leaving a window where the failing task's completion could overtake an ok task's still-being-enqueued completion (countDown happens inside call(), the queue add after it returns) — the recorded sequence then read [1] instead of [1, 2]. The gate now opens from the progress callback on the orchestrating thread, so both successes are provably consumed before the failure is thrown. Also renames `failer` to `failing` for the spell check. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Squash merges orphan the stack's ancestry, so the benchmark documents (regenerated on every bench run) collide as add/add between this branch and main. Align them to main's version; the round's closing PR commits the final regenerated baselines, so no information is lost from the final state. The measured numbers this PR contributed remain in its commit message and PR description. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Owner
Author
|
Re-filed as a fresh branch off main (stacked-squash ancestry orphaning); content identical. |
This was referenced Jun 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
The cleaner's two heaviest morphology ops ran on Leptonica's generic rasterop bricks: the 43×43 isolated-dust dilation (37.5ms/page — the single hottest op in the committed baseline) and the 7×7 fill-holes opening (12.2ms). Leptonica ships word-accelerated DWA kernels for both; they were unbound.
The trap the equality sweep caught
Binding them surfaced exactly what the planned pixel-identity gate existed for:
pixDilateBrickDwasilently diverges from the generic brick for sel sizes missing from the generated table — empirically every prime above 15, including the production 43 — whilepixOpenBrickDwais exact at every size and dilate is exact up to 15. (The compositepix*CompBrickDwavariants diverge too; diagnosed per-size in-container, table in the test rationale.)The shipped routing (exact by construction)
dilated(): single DWA pass ≤ 15; larger sizes composed from safe-size DWA passes — exact by Minkowski sum (brick(a) ⊕ brick(b) = brick(a+b−1); per-pass clipping changes nothing inside the image rectangle because L∞ paths between in-bounds points stay in bounds), and version-robust (only the always-present small sels are used). 43×43 = three passes.opened(): DWA ≤ 15 (production is 7×7), generic beyond (an opening does not compose).PixTestpins pixel-identity across radii 0–31 (sel 1–63) on border-touching ink + degenerate (tiny/all-black/all-white) pages.pixCountPixelsreuses one process-lifetime popcount table (hygiene, <0.1%).Measured
selectBySizeis now ~70% of the remaining clean() — the measured gate for the follow-up selection restructuring (De Morgan flip + single-labeling fusion) is met.Verification
./gradlew checkgreen; ArchUnit FFM confinement untouched (new bindings inLeptonica, routing inPix).dilated/openedinternals (~10 lines); bindings and sweep stay harmless.🤖 Generated with Claude Code