aj/OpenNest
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

perf(fill): remove discarded extents pitch geometry

Browse Source
This commit is contained in:
aj
2026-09-25 19:52:19 -04:00
parent cec7396da6
commit 6efa6b1117
8 changed files with 961 additions and 20 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/performance/fill-performance.md
+133
View File
@@ -230,3 +230,136 @@ dotnet test OpenNest.Tests/OpenNest.Tests.csproj -c Debug \
```
To reproduce before/after independently, use separate disposable worktrees at the baseline revision and the delivered revision, copy the identical measured harness/tests into both, and verify their hashes before rebuilding and running. Do not substitute a test-only approximation for the before production path or mix measurements from changed harnesses.
## Task 2 — eliminate discarded extents pitch geometry — 2026-09-25
### Scope, proof, and compatibility
`FillExtents.BuildColumn` now uses `pair.Bbox.Width + partSpacing` directly for finite pair height and finite, nonnegative spacing. The source comment records the equivalence: the old helper returns that pitch for negative/no-hit slide distances; otherwise `minSlide >= 0`, so `Max(pairHeight - minSlide, pairHeight + partSpacing)` selects the same pitch. Boundary creation, two temporary clones, and vertical slide queries were discarded work in that domain.
The caller audit found that `NestJobValidator` rejects negative/nonfinite spacing, but the public constructor, `Plate.PartSpacing`, `PlateFillService`, and `ExtentsFillStrategy` do not establish that invariant for all interactive/public callers. The legacy calculation and its helpers therefore remain behind a negative/nonfinite-spacing or nonfinite-height guard. No validation or exception policy was added. Pair construction still prepares both boundaries. Tiling, adjustment, overlap fallbacks, progress, cancellation, bounds recomputation, and horizontal repetition are untouched. Task 3 and ML work are not included.
### Source and measurement provenance
- Resumed base: `cec7396da6d62386ef4817a6eb0e1b573ddd58de` on `master`, with unfinished Task 2 edits. The final implementation is delivered with this report; resolve its commit with `git log -1 --format=%H -- OpenNest.Engine/Fill/FillExtents.cs`.
- Original `FillExtents.cs` SHA-256: `e4110022f142ec4f954a00b8569dea5d451dbd5216756b0b6bf6a9595f2a83d0`.
- Optimized `FillExtents.cs` SHA-256: `efaacfca83a6d94bb7fdc65ba4da4be09c97c14c9c4e161491d48d64a69d1be3`.
- Identical final before/after harness `FillPerformanceTests.cs` SHA-256: `bb2adf31fa3348d1b84c239b01cdffa50fffc92193af3d1a4122461a93181cda`.
- Identical behavior/work tests `FillExtentsTests.cs` SHA-256: `b43508e30ab9683a123c4e00a0478e92cc1467fcf1afc5c6778f8fb2d548f02b`.
- Frozen test reference `LegacyFillExtents.cs` SHA-256: `b50b3b64014446d9688facc0b711b59cf126b64037fdf667603ce3d85859a324`. Reversing only the type/constructor rename and public-to-internal visibility change reproduced the baseline file byte-for-byte. The full reference is retained because the tests compare intermediate progress and private column-adjustment/fallback behavior as well as final layouts.
- Before tests/measurements temporarily restored the actual original production file, with restoration of the optimized file in `finally`. Source/project manifests matched across measurements except for `FillExtents.cs`; diagnostics and all tests were identical. No reference implementation or reflection is inside timing.
- Host: `hermes`, Ubuntu 24.04.5 LTS x64, Linux 6.8.0-142-generic, KVM guest with four vCPUs presented as AMD Ryzen 9 5900X. SDK 10.0.112. All measured tests emitted .NET 8.0.31, Release, and a 1,000,000,000 Hz stopwatch. Shared VM, no CPU pinning or host isolation.
- The resumed exploratory logs were not used for final numerical estimates: the full measurement pair was rerun after test formatting, then repeated in reverse process order. All final raw rows are preserved below; no best-run selection or mixing of harness versions.
### Behavior and deterministic work evidence
35 new Release behavior cases cover a 16-case shape/spacing/rotation matrix (rectangle, right triangle, concave L, native quarter-arc profile; spacing 0 and 0.5), nonzero work-area origins, no-fit parts/pairs, pre-cancellation, adjusted columns, overlap fallback at a 30-degree triangle angle, negative spacing, nonfinite no-fit calls, and isolated unsupported negative-spacing column calculations. Exact ordered comparisons include drawing reference identity, location, rotation, bounds, program geometry, and progress messages/layouts. Caller program/code identities and values, cached area, work-area values, and clone program-sharing/rotation ownership are checked. Main supported fixtures are finite, positive-area, in-bounds, and nonoverlapping; exact legacy equivalence preserves their spacing behavior rather than introducing a new spacing algorithm.
Two dedicated differential cases explicitly retain a pre-existing limitation: at zero spacing in a `(3,5,40,27)` area, rectangle/native-arc fills overrun the right edge by approximately `1e-5`. Those cases are not counted as valid-layout evidence and are not silently repaired by this optimization. The normal validity matrix and benchmark instead use `(3,5,45,27)`. Nonfinite spacing is tested only through bounded no-fit or isolated negative-infinity column paths, not unrestricted legacy tiling that could fail to terminate.
`PerfCounters.PartBoundaryPreparations` follows the existing Debug-only conditional increment pattern. Four new work assertions run in the existing nonparallel `FillCacheCollection`, resetting counters in `finally`:
| Case (each at spacing 0 and 0.5) | Required | Original production | Optimized production |
| --- | ---: | ---: | ---: |
| Four repeated `BuildColumn` calls | 0 preparations | 8 (genuine RED) | 0 |
| Full triangle fill, initial plus rebuilt column | 2 preparations | 6 (genuine RED) | 2 |
The two remaining full-fill preparations are the necessary `BuildPair` work, also asserted independently. Every full-fill work case returns the same 24-part layout. Six column-adjustment cases establish both accepted adjustment and fallback: four retain nonoverlapping adjusted columns, and two reject overlapping adjusted columns in favor of the unchanged initial column. The selected column and final layout remain in bounds. Release counter calls compile away; zero Release counters are not evidence of skipping work.
### Workload and results
The opt-in benchmark uses a closed right triangle `(0,0)-(10,0)-(0,8)-(0,0)`, work area `(3,5,45,27)`, rotation 0, and spacing 0 or 0.5. Both modes return 24 parts and rebuild their column once. Each spacing gets two 50-call warmups and seven 200-call measured batches per process. Spacing order alternates per batch. The first pair runs original then optimized; the repeat runs optimized then original. These are separate rebuilt test processes, not an interleaved in-process implementation comparison.
Timing includes real synchronous production fill, geometry preparation, tiling, adjustment, overlap checking/fallback, GC, delegates/loops, and count consumption. Setup, reference fills, assertions, and output are outside timing. Every batch consumes 4,800 parts and checks its last layout against the frozen reference. Source drawing data/JIT paths are warm; no forced collection, cache reset, file fixture, or network dependency is involved.
Microseconds per fill, minimum / median / maximum over seven batches:
| Process pair | Spacing | Before µs/fill | After µs/fill | Median change |
| --- | ---: | --- | --- | ---: |
| Forward | 0 | 2205.240 / 2281.382 / 2332.728 | 2158.270 / 2186.747 / 2207.422 | -4.15% |
| Forward | 0.5 | 2244.825 / 2310.086 / 2330.661 | 2155.409 / 2190.618 / 2215.091 | -5.17% |
| Reverse | 0 | 2165.221 / 2229.736 / 2403.664 | 2165.604 / 2210.005 / 2316.437 | -0.88% |
| Reverse | 0.5 | 2224.011 / 2302.593 / 2330.944 | 2187.911 / 2203.634 / 2266.720 | -4.30% |
The measured medians decrease in both process orders, but the ranges overlap (especially zero spacing) and the gain varies with run order. These shared-VM samples support a modest local improvement, not a stable percentage guarantee or whole-job speedup. No reproducible material regression was observed and there is no elapsed-time test gate.
Synchronous current-thread allocations are identical in every measured batch for a given implementation/spacing:
| Spacing | Before bytes/batch | After bytes/batch | Before bytes/fill | After bytes/fill | Saved bytes/fill | Reduction |
| ---: | ---: | ---: | ---: | ---: | ---: | ---: |
| 0 | 174939200 | 167924800 | 874696 | 839624 | 35072 | 4.01% |
| 0.5 | 226099200 | 205708800 | 1130496 | 1028544 | 101952 | 9.02% |
These are `GC.GetAllocatedBytesForCurrentThread` managed-allocation differences around synchronous fills, not retained memory, RSS, or parallel-job totals. The deterministic result is removal of discarded boundary preparation; the allocation savings reproduce in both process orders.
### Tests and serial reviews
| Run | Passed | Skipped | Failed |
| --- | ---: | ---: | ---: |
| Original targeted Release | 43 | 0 | 0 |
| Original targeted Debug (intentional RED) | 43 | 0 | 4 |
| Optimized targeted Release | 43 | 0 | 0 |
| Optimized targeted Debug | 47 | 0 | 0 |
| Each of four final opt-in Release measurement runs | 1 | 0 | 0 |
| Extents opt-in gate unset | 0 | 1 | 0 |
| Extents opt-in gate `0` | 0 | 1 | 0 |
| Full main Release | 1206 | 15 | 0 |
| Full main Debug | 1219 | 15 | 0 |
| Full engine Release | 300 | 0 | 0 |
The targeted filter is `FillExtentsTests|StrategyOverlapTests` with `FullyQualifiedName~` on each term. Full-suite TRX outcomes were parsed rather than inferred from console totals: main Release has 1,221 total results, Debug 1,234, engine 300. All skips are the 12 optional CHR-font fixtures and three opt-in performance tests. The adapter confirms .NET 8.0.31 in all full runs. Rebuilding the engine emits the existing `CirclePacking/Item.cs` CS0108/CS0114 warnings; rebuilding the test project also emits its existing nullable/xUnit warnings. The frozen `LegacyFillExtents.cs:30` adds one benign CS8625 warning because the unchanged legacy `reportProgress = null` default is copied into the nullable-enabled test project. It is retained verbatim for reference fidelity, not presented as a pre-existing test warning. No new production warnings or unexpected test failures were found.
Per the user's no-subagents instruction, spec review and subsequent quality/integration review were performed serially by the same agent, not independently. Spec review checked the algebraic guard, caller audit, exact frozen reference, behavior/ownership/cancellation/overlap coverage, genuine red/green work assertions, and Task 2-only scope. Quality review checked the production/test diff, Debug counter isolation and Release compilation convention, absence of new secrets or unsafe I/O, runtime/harness provenance, and all final raw TRX rows against console output. No blocking findings remained after the test-only whitespace correction. Changed-file `dotnet format whitespace --verify-no-changes` and `git diff --check` passed.
README documents the extents benchmark and work-test commands. The previously denied `CLAUDE.md` workflow sync remains blocked pending explicit approval; it was not retried. No representative corpus, Windows UI runtime test, or actual ONNX inference was run. This slice does not complete Tasks 3–5 or authorize gated follow-ups.
### Raw measured batches
Milliseconds per 200 actual production fills. Each row contains both implementations from one process pair; every batch produced 4,800 parts. Allocation totals for every row are the constant spacing-specific values above.
| Process pair | Spacing | Batch | Before ms | After ms |
| --- | ---: | ---: | ---: | ---: |
| forward | 0 | 1 | 463.229998 | 439.473112 |
| forward | 0 | 2 | 466.545605 | 437.349468 |
| forward | 0 | 3 | 450.826218 | 441.484339 |
| forward | 0 | 4 | 456.276359 | 438.751402 |
| forward | 0 | 5 | 462.975196 | 434.451956 |
| forward | 0 | 6 | 445.596554 | 433.705799 |
| forward | 0 | 7 | 441.048033 | 431.653954 |
| forward | 0.5 | 1 | 465.387515 | 443.018255 |
| forward | 0.5 | 2 | 462.017170 | 441.358437 |
| forward | 0.5 | 3 | 466.132129 | 438.198245 |
| forward | 0.5 | 4 | 450.824107 | 431.081841 |
| forward | 0.5 | 5 | 462.570224 | 438.123662 |
| forward | 0.5 | 6 | 454.096721 | 433.895019 |
| forward | 0.5 | 7 | 448.964925 | 437.877448 |
| reverse | 0 | 1 | 480.732828 | 463.287364 |
| reverse | 0 | 2 | 445.947233 | 434.966923 |
| reverse | 0 | 3 | 450.506707 | 442.001094 |
| reverse | 0 | 4 | 433.764890 | 446.086663 |
| reverse | 0 | 5 | 437.597684 | 448.988001 |
| reverse | 0 | 6 | 433.044245 | 433.120823 |
| reverse | 0 | 7 | 448.325975 | 439.151450 |
| reverse | 0.5 | 1 | 460.518547 | 447.369861 |
| reverse | 0.5 | 2 | 461.616765 | 437.593637 |
| reverse | 0.5 | 3 | 466.188871 | 453.049694 |
| reverse | 0.5 | 4 | 463.178063 | 453.343928 |
| reverse | 0.5 | 5 | 453.526082 | 440.726846 |
| reverse | 0.5 | 6 | 444.802234 | 440.614301 |
| reverse | 0.5 | 7 | 448.043300 | 437.582121 |
### Reproduction and evidence retention
```bash
OPENNEST_RUN_FILL_PERF=1 dotnet test OpenNest.Tests/OpenNest.Tests.csproj -c Release \
--filter 'FullyQualifiedName~Extents_ReportsRepeatedColumnRebuilds' \
--logger 'console;verbosity=detailed'
dotnet test OpenNest.Tests/OpenNest.Tests.csproj -c Release \
--filter 'FullyQualifiedName~FillExtentsTests|FullyQualifiedName~StrategyOverlapTests'
dotnet test OpenNest.Tests/OpenNest.Tests.csproj -c Debug \
--filter 'FullyQualifiedName~FillExtentsTests|FullyQualifiedName~StrategyOverlapTests'
```
For an independent before/after comparison, keep the delivered tests/diagnostics identical in two disposable trees and use the baseline `FillExtents.cs` only in the before tree. Rebuild each configuration and verify the source/harness hashes. Do not change production spacing policy to reproduce invalid-domain cases or substitute the frozen test type for the measured production method.
Final logs, TRX files, source restoration snapshot, manifests, and parsed rows were held in `/home/aj/.hermes/cache/scratch/opennest-task2-resume-20260925/` through review. The report preserves all final timing rows, allocation totals, source hashes, runtime and test summaries; temporary evidence and the superseded resumed exploration directory are removed before commit. Next planned hardening is the separately reviewable redundant-bounds slice with threshold-fit and canonical-frame safety, not a geometry repair bundled into this change.