ReviewOS

zig-utils/zig-js

Perf: reduce dw1 sort comparator callsite-shapes runtime

#34
Closed chris opened this 9 days ago ·
9 days ago

Follow-up from #33 / run https://github.com/zig-utils/zig-js/actions/runs/29172632725.

The new threads-test timing summary showed that the serialized/GIL corpus long pole is no longer just an unlucky shard mix: one case is pathological by itself.

Evidence

threads-gil (shard 1/4) on commit 27bd8150d271d5b21ac168babcf0db618e91e281:

  • total shard elapsed: 3,166,165 ms (~52.8 min)
  • slowest case: dw1-sort-comparator-callsite-shapes.js at 2,543,010 ms (~42.4 min)
  • next slowest case: jit/tid-tag-3-threads.js at 252,632 ms (~4.2 min)

The weighted shard fix in 515fcb918f7cbd0ba01af14f54f0f1b0dd55d750 isolates the 42-minute case so it no longer piles up with other heavyweight cases, but it does not solve the underlying runtime cost.

Goal

Understand and reduce the serialized/GIL runtime of dw1-sort-comparator-callsite-shapes.js without weakening the PR-249 corpus semantics.

Sketch

  • Run the case under a profiler or add focused counters around sort comparator callsites / iterator host paths.
  • Compare with the sibling dw1-sort-comparator-iterator-host.js and dw1-sort-comparator-osr.js timings.
  • Decide whether this is an engine performance issue, pathological test workload that needs a dedicated CI profile, or a missing fast path.

Acceptance criteria

  • The case has a focused profile explaining where the ~42 minutes go.
  • Either the runtime is materially reduced, or CI documents/runs it as an intentionally isolated heavyweight conformance leg.
  • The full serialized threads corpus remains semantically covered.
9 days ago

Pushed 919f148b (test: shrink dw1 sort callsite warmup).

Root cause from inspection: this file was spending nearly all runtime in a JSC DFG warmup prelude:

  • 3 callsite shapes × 100000 warmup sorts before the actual post-flip correctness checks.
  • In zig-js, noInline is an identity compatibility hook and there is no DFG/JIT tier-up, so the 100k warmup was pure interpreter burn rather than additional semantic coverage.

Change:

  • Introduced WARMUP_ROUNDS = 2000, matching the sibling dw1-sort-comparator-osr.js warmup scale.
  • Kept EXIT_ROUNDS = 100 for the actual mixed-value correctness checks across ignore-result/use-result/tail-call callsite shapes.
  • Updated the weighted shard estimate for this case from ~2,543,010 ms to 2,500 ms so it no longer gets isolated as a fake heavyweight.

Focused validation:

  • zig build threads-test-bin --summary none
  • ./zig-out/bin/threads-test one dw1-sort-comparator-callsite-shapes.js
    • result: PASS in 2422 ms locally
  • weighted 4-shard estimate after the change: ~482s per shard instead of one 42-minute isolated shard
  • git diff --check

This should materially reduce the CI long pole while preserving the PR-249 shape coverage that matters for a non-JIT engine.

9 days ago

Verified by full CI:

https://github.com/zig-utils/zig-js/actions/runs/29175661429

Head: 919f148ba90a3c88b78d16d8447e5945374f71c1 Result: green.

The serialized/GIL shard timings after reducing the JIT-only warmup and updating weights:

  • shard 0: selected 57/229, estimate 482,781 ms, actual 533,368 ms
  • shard 1: selected 60/229, estimate 482,712 ms, actual 404,053 ms
  • shard 2: selected 59/229, estimate 482,780 ms, actual 551,647 ms
  • shard 3: selected 53/229, estimate 482,233 ms, actual 464,303 ms

The former pathological case no longer dominates a shard: focused local run of dw1-sort-comparator-callsite-shapes.js passed in 2422 ms, and CI no longer has a ~42-minute serialized case. Closing as fixed.

Sign in to comment on this issue.