0.9.0: Hot-path performance rewrite, restart-interval parallelism, acceleration-layer removal#117
Merged
Merged
Conversation
Measured baseline, hot-path attribution, phased work items with bit-exactness gates, and rejected directions. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Replace Foundation Data backing (38.9 ns/byte append) with a pre-reserved [UInt8] and widen the bit accumulator to UInt64 so a worst-case 7-bit residual plus a 32-bit write can never overflow. Unary/ones batching widened from 24 to 32 bits per call. Gate: 44 golden artifacts byte-identical; 1388 tests pass. Synthetic 16-bit 2048^2 encode: 215 -> 190 ms. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Resolve componentPixels[componentId] once per scan and current/previous row arrays once per line in all three interleave paths; compute causal neighbours inline from the hoisted rows (identical ITU-T.87 boundary semantics to JPEGLSPixelBuffer.getNeighbors). Run detection and the interruption pixel reuse the hoisted rows; sample-interleaved edge tracking switches from Dictionary to component-index arrays. Gate: 44 golden artifacts byte-identical; 1388 tests pass. Synthetic 16-bit 2048^2 encode: 190 -> 110 ms (baseline 215). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The scan loop already quantizes all three gradients for the run-mode test; pass them (as a fused contextIndex+sign) into a new encodePixel overload instead of recomputing them inside regularMode.encodePixel. computeContextIndexAndSign evaluates the sign chain once for both values. The d-based encodePixel overload remains and delegates. Gate: 44 golden artifacts byte-identical; 1388 tests pass. Synthetic 16-bit 2048^2 encode: 110 -> 97 ms (baseline 215). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Port the encoder's init-time gradient quantisation table to the decoder, built by evaluating the reference branch chain so boundary semantics (Table A.7, strict upper bounds) are identical by construction. Scan loops pass the already-quantized gradients into decodeSinglePixel and a new decodePixel overload, eliminating the double (scan-loop + pipeline) quantization per pixel. Gate: 44 golden artifacts byte-identical; 1388 tests pass. Synthetic 16-bit 2048^2 decode: 138 -> 114 ms (baseline 138). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
detectRunLength scans via withUnsafeBufferPointer (buffer is immutable during the scan). Lossless (near==0) uses a 4-way unrolled exact equality test instead of abs() per element; near>0 keeps the abs path. Gate: 44 golden artifacts byte-identical; tests pass (one wall-clock perf-regression test in the suite is flaky under parallel load, unrelated: reruns green). Synthetic 2048^2: 8-bit (run-heavy) encode 41 -> 12.7 ms; 16-bit encode 97 -> 94 ms. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1. encodeNoneInterleaved: only allocate the near-lossless 'reconstructed' buffer when near > 0 (every access is so guarded); lossless previously zeroed a full H*W [[Int]] per scan (~32 MB at 2048^2, ~136 MB for a 17 MP mammo frame) that was never read. 2. Decoder result skips the O(W*H) re-validation pass via an internal trusted init when samples are clamped by construction (uniform sampling, no mapping table, no colour transform). Added the ITU-T.87 C.2.4.1.1 parse-time check MAXVAL <= 2^P-1 that the validation pass was implicitly providing. 3. Parser records each scan body's byte range during its existing walk; the decoder slices data directly instead of re-walking the whole file (extractScanData kept as fallback for external parse results). Gate: 44 golden artifacts byte-identical; 1388 tests pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Rewrite JPEGLSBitstreamReader around a UInt64 window over [UInt8]: refill loads several bytes at a time applying the ISO 14495-1 9.1 stuff-bit rule per byte (0xFF + <0x80 contributes 8+7 bits; 0xFF before a marker contributes 8, replicating the historical fall-through), and unary prefixes decode via leadingZeroBitCount over the aligned window (readUnaryCount) instead of one readBits(1) call per bit. resetBitBuffer keeps its contract under eager refill by replaying the stuffing rule over the consumed bit region to land on the byte boundary after the last consumed bit. Parser byte reads also benefit from the [UInt8] backing (was per-byte Foundation Data subscripts). Also make the memory-usage benchmark test deterministic: it asserted RSS delta > 0, which legitimately fails now that encode allocates less. Gate: 44 golden artifacts byte-identical; 1388 tests pass. Synthetic 16-bit 2048^2 decode: 111 -> 98 ms (baseline 138). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
decodeComponent decodes into a contiguous UInt16 plane through one unsafe buffer scoped over the whole scan (no nested-array indirection, bounds checks, or per-row CoW; half the memory traffic of [[Int]]), with the run fill and interruption-sample decode inlined over the plane. Widens to the public [[Int]] once at scan end. encodeNoneInterleaved gains a lossless (NEAR=0) fast path over the same flat representation, including the 4-way unrolled exact-equality run scan against the row. Near-lossless keeps the general path; coding decisions are unchanged in both. Gate: 44 golden artifacts byte-identical; 1388 tests pass. Synthetic 16-bit 2048^2: encode 97 -> 88 ms, decode 98 -> 90 ms. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Replace the four parallel [Int] context arrays (A/B/C/N) with one [ContextRecord] array: updateContext loads the record once, mutates locals, and stores once (one bounds + one CoW check per pixel instead of ~10 across four arrays, and the whole record shares a cache line). RESET threshold and the B-update factor (2*NEAR+1) are hoisted to stored lets. New pixelCodingState(contextIndex:) returns bias C[Q], Golomb k, and the k=0 error correction from a single record load; encoder and decoder pipelines use it instead of three separate calls. Gate: 44 golden artifacts byte-identical; 1388 tests pass. Synthetic 16-bit 2048^2: encode 88 -> 82 ms, decode 90 -> 77 ms. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
processEncode/processDecode were 'not yet implemented' stubs sitting on top of a working semaphore-throttled worker pool. Encode auto-detects PGM/PPM/PNG/TIFF (mirroring 'jpegls encode' defaults, honouring --near); decode writes packed raw samples like 'jpegls decode --format raw'. Both codecs are stateless Sendable structs, so concurrent per-file use is safe — verified batch output is byte-identical to serial encodes. Dropped the stale --width/--height requirement that assumed raw input. Gate: 44 golden artifacts byte-identical; 1388 tests pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Bug fix: conformant streams containing restart markers previously FAILED to decode — both the parser's scan walk and extractScanData treated FFD0-FFD7 as scan terminators and truncated the body at the first RST. Both now recognise restart markers as part of the entropy-coded segment. Encoder: opt-in Configuration.restartInterval (lines; lossless non-interleaved scans) writes a DRI segment and emits RSTm markers cycling FFD0-FFD7. Per ITU-T.87 every interval restarts coding as at scan start (fresh contexts/run state, byte alignment, zero previous line), so intervals are independent and encode in parallel into per-interval buffers (DispatchQueue.concurrentPerform with a lock-protected result box). CLI: 'jpegls encode --restart-interval N'. Decoder: splits the scan body at validated RSTm markers and decodes the intervals concurrently as independent flat regions. 4096x4096 16-bit, interval 256: encode 0.84 -> 0.31 s wall, decode 0.69 -> 0.24 s, +0.03% size, pixels identical to non-restart. Also: CLI raw output accumulated per-byte into Data (~70x slower than [UInt8]) in decode/batch — fixed; plain 33 MB decode 2.0 -> 0.69 s. 11 new tests (round-trip 8/12-bit, run-heavy boundaries, marker cycling, DRI parse, determinism, multi-component, validation). Gate: 44 golden artifacts byte-identical; 1399 tests pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Remove Sources/JPEGLS/Platform/ (Metal, Vulkan, Accelerate, ARM64, x86_64 — ~4,400 lines), JPEGLSBufferPool, JPEGLSCacheFriendlyBuffer, JPEGLSTileProcessor, and PlatformProtocols, plus their test files (~5,000 lines): profiling showed zero production call sites — the shipping codec never invoked any of it — and the Metal/Vulkan kernels could not produce conformant streams (the entropy stage's bias correction and context adaptation are sequential by construction). Restart intervals (W3.2) are the supported parallelism mechanism. Docs: PERFORMANCE_TUNING.md rewritten around what actually ships (flat planes, 64-bit bitstream I/O, packed contexts, restart-interval parallelism, honest benchmarking guidance); deleted the Metal/Vulkan/ x86-64 guides; README claims, structure trees, and feature tables updated; stale tile-processor/buffer-pool examples in 8 guides replaced with restart-interval equivalents; CHANGELOG entry for the whole branch. Gate: 44 golden artifacts byte-identical; 1038 tests in 80 suites pass (suite runs in ~36 s, down from ~78 s, with the accel benchmarks gone). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Fixes from the adversarial branch review (every medium+ finding was independently verified before being accepted): Major: - decode(_:) rebases Data slices with non-zero startIndex before using zero-based scan ranges (slices previously decoded wrong bytes silently, or trapped; DICOM fragment extraction produces such slices) - Encoder rejects sub-sampled component planes up front: the flat scan paths iterate frame dimensions through unsafe buffers and would read out of bounds (silently, in release) on narrower planes - Decoder requires every frame component to have a scan (the trusted result init had dropped the count check the validating init provided) Minor: - Stray RSTm markers in scans without an active restart interval now throw instead of being absorbed as entropy data (a corrupted stuffed byte produced silent garbage; main failed loudly) - DRI is tracked per scan (T.81 B.2.4.4: a DRI between scans applies to following scans only); JPEGLSParseResult gains scanRestartIntervals - Interleaved streams declaring DRI >= height (zero actual markers) are decoded instead of rejected - Crafted LSE type-4 dimensions whose product overflows Int now throw invalidDimensions instead of trapping; undersized LSE segments throw instead of trapping in readBytes (count is now validated non-negative) - Encoder validates preset MAXVAL <= 2^P-1 (ITU-T.87 C.2.4.1.1): larger values trapped mid-encode or emitted undecodable streams - batch encode: --interleave/--colour-transform are now honoured for colour inputs; the never-functional raw-input options (--width, --height, --bits-per-sample, --components) are removed and the help text no longer claims raw support - Corrected the bitstream writer's bit-buffer invariant comment (high bits are stale, and the stuff-bit clear is load-bearing) and documented the reader's resetBitBuffer requirement when switching from bit-level to byte-level reads Adds 12 robustness regression tests (Data slices, malformed restart streams, hostile LSE segments, encoder validation). The external golden gate now also pins 6 restart-interval artifacts. Gate: 50 golden artifacts byte-identical (44 original + 6 restart); 1050 tests in 81 suites pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Promote the [Unreleased] section to a dated 0.9.0 release and bump the installation snippet's package version. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR delivers the v0.9.0 optimization plan for JLSwift’s JPEG-LS codec: a hot-path rewrite to substantially improve encode/decode throughput, adds standards-compliant restart-interval (DRI/RSTm) support enabling intra-image parallelism, and removes the previously unused Platform acceleration layer (Metal/Vulkan/Accelerate/arch-specific wrappers) along with associated docs/tests.
Changes:
- Add restart-interval (DRI/RSTm) support to the encoder/decoder + CLI flag, with a dedicated test suite.
- Rewrite core hot-path pieces (bitstream writer, run-mode scanning, context model packing, gradient quantization table, parser scan slicing) for performance while keeping output bit-exact (where stated).
- Remove Platform acceleration layer, buffer pool/tile processor/cache-friendly buffer, and update docs/README/changelog accordingly; implement
jpegls batch encode/decode.
Reviewed changes
Copilot reviewed 68 out of 69 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| Tests/JPEGLSTests/X86_64AcceleratorPhase14Tests.swift | Removes x86_64 accelerator-specific tests (platform layer removed). |
| Tests/JPEGLSTests/PlatformBenchmarks.swift | Removes platform-accelerator benchmark suite. |
| Tests/JPEGLSTests/Phase16OptimisationTests.swift | Removes buffer-pool optimisation tests (buffer pool removed). |
| Tests/JPEGLSTests/JPEGLSTileProcessorTests.swift | Removes tile processor tests (tiling removed). |
| Tests/JPEGLSTests/JPEGLSRestartIntervalTests.swift | Adds restart-interval round-trip/structure/validation tests. |
| Tests/JPEGLSTests/JPEGLSPerformanceBenchmarks.swift | Makes a memory benchmark assertion non-flaky; asserts encode result instead. |
| Tests/JPEGLSTests/JPEGLSCacheFriendlyBufferTests.swift | Removes cache-friendly buffer tests (type removed). |
| Tests/JPEGLSTests/JPEGLSBufferPoolTests.swift | Removes buffer pool tests (type removed). |
| Tests/JPEGLSTests/IntelMemoryOptimizerTests.swift | Removes Intel memory optimizer tests (platform layer removed). |
| Tests/JPEGLSTests/EdgeCasesTests.swift | Removes edge-case tests tied to removed pooling/tiling/buffer APIs. |
| Tests/JPEGLSTests/ARM64AcceleratorPhase13Tests.swift | Removes ARM64 accelerator-specific tests (platform layer removed). |
| Tests/JPEGLSTests/AppleSiliconMemoryOptimizerTests.swift | Removes Apple Silicon memory optimizer tests (platform layer removed). |
| Tests/JPEGLSTests/AccelerateFrameworkBenchmarks.swift | Removes Accelerate benchmark suite (platform layer removed). |
| Sources/jpeglscli/EncodeCommand.swift | Adds --restart-interval CLI option and wires it into encoder configuration. |
| Sources/jpeglscli/DecodeCommand.swift | Speeds up raw output writing by buffering in [UInt8] then creating Data once. |
| Sources/jpeglscli/BatchCommand.swift | Implements batch encode/decode and updates batch CLI semantics (auto-detect image formats). |
| Sources/JPEGLS/Platform/x86_64/X86_64Accelerator.swift | Removes x86_64 SIMD accelerator implementation. |
| Sources/JPEGLS/Platform/x86_64/IntelMemoryOptimizer.swift | Removes Intel-specific memory optimizer utilities. |
| Sources/JPEGLS/Platform/Vulkan/VulkanMemory.swift | Removes Vulkan acceleration scaffolding. |
| Sources/JPEGLS/Platform/Vulkan/VulkanDevice.swift | Removes Vulkan device enumeration scaffolding. |
| Sources/JPEGLS/Platform/Vulkan/VulkanCommandBuffer.swift | Removes Vulkan command buffer scaffolding. |
| Sources/JPEGLS/Platform/ARM64/AppleSiliconMemoryOptimizer.swift | Removes ARM64 memory-optimizer utilities. |
| Sources/JPEGLS/Encoder/JPEGLSRunMode.swift | Speeds up lossless run detection using unsafe buffer access + unrolling. |
| Sources/JPEGLS/Encoder/JPEGLSRegularMode.swift | Avoids duplicated context computations; adds an overload taking precomputed context/sign. |
| Sources/JPEGLS/Encoder/JPEGLSPixelBuffer.swift | Adds an internal “unchecked” initializer for decoder-produced components to skip O(W·H) validation. |
| Sources/JPEGLS/Decoder/JPEGLSRegularModeDecoder.swift | Adds init-time gradient quantization table + decode overload taking precomputed context inputs. |
| Sources/JPEGLS/Decoder/JPEGLSParser.swift | Records scan-body ranges and per-scan DRI; fixes restart-marker handling inside scan bodies; hardens LSE parsing. |
| Sources/JPEGLS/Core/PlatformProtocols.swift | Removes platform-accelerator protocol/selection API. |
| Sources/JPEGLS/Core/JPEGLSTileProcessor.swift | Removes tiling API. |
| Sources/JPEGLS/Core/JPEGLSContextModel.swift | Packs context stats into a record array; hoists constants; adds combined context index+sign and single-load coding-state accessor. |
| Sources/JPEGLS/Core/JPEGLSCacheFriendlyBuffer.swift | Removes cache-friendly buffer type and memory statistics type. |
| Sources/JPEGLS/Core/JPEGLSBufferPool.swift | Removes buffer pooling implementation and shared pool. |
| Sources/JPEGLS/Core/JPEGLSBitstreamWriter.swift | Reworks writer to [UInt8] + UInt64 accumulator; improves bulk writes and unary/ones batching. |
| README.md | Updates project positioning, features, and docs links for the new hot path + restart intervals; removes platform layer references. |
| Package.swift | Removes Metal shader resource processing (Metal layer removed). |
| docs/VULKAN_GPU_ACCELERATION.md | Removes Vulkan acceleration documentation (feature removed). |
| docs/TROUBLESHOOTING.md | Updates troubleshooting guidance to match removed acceleration APIs and new restart-interval parallelism. |
| docs/SWIFTUI_EXAMPLES.md | Replaces tiling examples with restart-interval parallelism guidance for large images. |
| docs/SERVER_SIDE_EXAMPLES.md | Updates large-file guidance from tiling to restart intervals; refreshes server-side examples accordingly. |
| docs/RELEASE_NOTES_TEMPLATE.md | Updates template example to use restart interval configuration. |
| docs/README.md | Updates docs index to reflect removal of acceleration docs and addition of optimization plan. |
| docs/MILESTONES.md | Notes removal/supersession of tile processor by restart intervals. |
| docs/METAL_GPU_ACCELERATION.md | Removes Metal acceleration documentation (feature removed). |
| docs/GETTING_STARTED.md | Updates “large image” pattern from tiling to restart intervals; removes old accelerator/buffer-pool patterns. |
| docs/DICOMKIT_INTEGRATION.md | Updates performance guidance from tiling to restart intervals; removes cache-buffer example. |
| CHANGELOG.md | Adds 0.9.0 release notes describing restart intervals, hot-path rewrite, fixes, and removals. |
| .gitignore | Adds .build-unchecked/ ignore entry. |
Comment on lines
+416
to
+424
| } else if inputData.count >= 2, | ||
| inputData[inputData.startIndex] == UInt8(ascii: "P"), | ||
| inputData[inputData.startIndex + 1] == UInt8(ascii: "5") | ||
| || inputData[inputData.startIndex + 1] == UInt8(ascii: "6") { | ||
| let pnm = try PNMSupport.parse(inputData) | ||
| componentPixels = pnm.componentPixels | ||
| var bits = 1 | ||
| while (1 << bits) - 1 < pnm.maxVal { bits += 1 } | ||
| bitsPerSample = max(2, bits) |
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
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.
Summary
Implements the full optimization plan from docs/OPTIMIZATION_PLAN.md (multi-agent profiled analysis of the v0.8.0 codec), in 15 commits, each independently gated on bit-exact output.
Performance (Apple Silicon)
--restart-interval 256(wall)Encoded bitstreams are byte-identical to v0.8.0 for all existing configurations (verified by a 50-artifact golden-hash gate after every commit), and all 107 corpus frames still round-trip losslessly.
What changed
Phase 1–2 — hot path (representational only, byte-identical output):
[UInt8]+ 64-bit accumulator (was per-byte FoundationDataappends)leadingZeroBitCountunary decode (was per-bitDatasubscript reads)UInt16planes via unsafe buffers; per-pixelDictionarylookups hoisted; gradients quantised once per pixel via init-time tables on both sidesPhase 3 — features and cleanup:
Configuration.restartInterval/jpegls encode --restart-interval N; intervals encode and decode in parallel; also fixes a bug where conformant restart streams failed to decode (scan walk truncated at the first RST marker)jpegls batch encode/decodeimplemented (previously "not yet implemented" stubs); byte-identical to serial encodesPlatform/acceleration layer (Metal/Vulkan/Accelerate/ARM64/x86-64, ~9,400 lines incl. tests) plusJPEGLSBufferPool/JPEGLSCacheFriendlyBuffer/JPEGLSTileProcessor; docs rewritten to describe only what shipsReview hardening (final commit): a 6-lens adversarial review with independent verification of every finding (28 checked, 18 refuted) surfaced 3 major + 8 minor issues —
Data-slice mis-slicing indecode(), an out-of-bounds read on sub-sampled planes, a dropped component-count check, per-scan DRI semantics (T.81 B.2.4.4), and several hostile-stream traps — all fixed, with 12 regression tests.Breaking changes (0.x minor)
MetalAccelerator,VulkanAccelerator,AccelerateFrameworkAccelerator,ARM64Accelerator,X86_64Accelerator,PlatformAcceleratorprotocol,JPEGLSBufferPool,JPEGLSCacheFriendlyBuffer,JPEGLSTileProcessorand related optimizer types — none were invoked by the codecjpegls batchno longer accepts the never-functional raw-input options (--width,--height,--bits-per-sample,--components)Test plan
swift test); 23 new tests (restart intervals, robustness regressions)jpegls bench-dicomover the radiology corpus: 107/107 frames lossless, ratios unchanged (3.01:1)🤖 Generated with Claude Code