Skip to content

Add a per-pass profiler for the Arrow row diff - #98

Open
ksco92 wants to merge 4 commits into
mainfrom
perf/90-spool-reread
Open

Add a per-pass profiler for the Arrow row diff#98
ksco92 wants to merge 4 commits into
mainfrom
perf/90-spool-reread

Conversation

@ksco92

@ksco92 ksco92 commented Sep 6, 2026

Copy link
Copy Markdown
Owner

Summary

  • Add a profile feature to onix-arrow that compiles per-pass wall-time and peak-RSS boundaries into diff_rows and the streaming cell pass. The boundaries expand to nothing without the feature (off by default, never enabled by the release wheel), so the instrumentation is absent from the shipped library — verified with strings on libdeepdiff_rs.dylib (0 matches for the pass labels and the sampler thread name; present in the feature-built example). Recording is gated on a begin/finish window; RSS is sampled through /bin/ps in a background thread that finish joins. No new dependency, no unsafe.
  • Add the committed row_diff_profile example (built only with the feature) with two modes: file (reads each side from an uncompressed Arrow IPC file, --key/--threads) for the real fixtures, and generated (deterministic proxy shapes) for a dependency-free check. It prints an uninstrumented wall, an instrumented wall, and the per-pass table (wall + peak RSS).
  • Instrument the re-read decode (spool decode (reader.next)) and the spill routing (cell: spill route) so the per-pass decomposition closes (top-level passes sum to within 1% of the total) and shows decode, not re-hashing, as the dominant re-read term.
  • Share the streaming generator between row_diff_rss and row_diff_profile (examples/shared/gen_shapes.rs); row_diff_rss's generated data and counts are unchanged.
  • perf docs: a Profiling section (file + generated commands, pyarrow conversion) and a RESULTS.md per-pass section with the real narrow/wide fixtures at 1M and full plus the proxy shapes, all on deepdiff-rs 0.11.2.
  • Add make test-all-features (cargo test --workspace --all-features) to check so the feature-on instrumentation and the profiler's own tests run in the gate.
  • Version 0.11.1 → 0.11.2 (internal change; no public surface change).

Issues

Baseline per-pass profile (real fixtures, file mode, 18 threads)

Medians (11 at 1M, 5 at full), no contending process. Each cell is wall s (peak RSS MB).

Pass narrow 1M wide 1M narrow full wide full
hash and classify 0.044 0.153 1.292 1.941
materialize 0.035 0.078 1.721 1.523
cell: spill (route + write + re-read) 0.031 0.408 1.550 8.728
cell: render sort keys 0.000 0.020 0.015 0.373
cell: read-back and render 0.010 0.300 0.149 6.005
cell: sort and interleave 0.003 0.172 0.101 3.535
decode (across re-read passes) 0.072 0.244 2.691 4.588
total wall 0.132 1.194 4.870 22.041

Peak RSS (median of per-pass peaks): narrow full ~4.0 GB, wide full ~19.7 GB. Full RESULTS.md has the RSS-per-pass and the route/write/read-back/render sub-rows.

Where the time goes (decode dominates the re-read)

The re-read is decode-bound. The decode (reader.next) accum is 60–66% of the combined hash + materialize + spill-re-read wall at every size, and materialize (re-reads with an empty select) is essentially all decode — scanning every key value adds only a few ms. So the re-read optimization wins by decoding each side once instead of three times, not by avoiding the key re-hash. On narrow full the three re-read rounds are 91% of the total (4.45 of 4.87 s); on wide full they are 32% (the rest is the cell pass's own route/render/reorder).

Stop-and-report triage (issue #90's decode clause)

#90 says to stop if the parquet→IPC decode itself dominates. It does dominate the re-read rounds — but the design still wins, because it removes two of the three decodes: the materialize and cell passes read only their key-hash partition, written once during the first read, so the input is decoded once, not three times. Deliverable 2 proceeds on that corrected premise. Expected saving from the corrected split: removing the materialize re-read and the cell-spill re-read (both decode-dominated) is the bulk of the re-read term — 91% of narrow full, and the 32% re-read share of wide full.

Test Results

  • Lint: PASS (cargo fmt --check; cargo clippy --all-targets --all-features -- -D warnings)
  • Tests: PASS (cargo test --workspace and cargo test --workspace --all-features)
  • Coverage: PASS (cargo llvm-cov --workspace --fail-under-lines 95: 97.19% total, row_diff.rs 97.61%). profile.rs compiles only under the feature, so it is outside the default-feature coverage build; measured under --all-features it is 99.44% lines (and the --all-features total is 97.21%).
  • Mutants: cargo mutants --in-diff over the changed row_diff.rs lines (default build): 11 tested, 6 caught, 5 unviable, 0 missed — including the columns: ctx.right_columns mutant in materialize_row_members, now caught by the different-column-order test. (profile.rs is feature-gated and not in the default mutants build; it is exercised by its own tests under --all-features, 99.44% line coverage.)
  • Docs / supply chain / dep hygiene: PASS (cargo doc -D warnings, cargo deny check, cargo machete)
  • Python: onix-py source unchanged; the wheel builds as 0.11.2 and diff_tables returns correct counts through pyarrow on 3.14.

Notes

  • The per-pass walls exclude the boundary ps cost (the timer starts after enter's RSS read, stops before the guard's), so they sum to about the uninstrumented wall; the instrumented wall is reported separately.
  • The generated proxy shapes are labelled "proxy" in RESULTS.md and differ materially from the real fixtures (two int64 columns vs the narrow fixture's five typed columns; 34 identical Utf8 columns vs the wide fixture's 34 distinct scalar types), so the real-fixture rows are the baseline and sit above them.

The row diff re-reads each spooled input once per pass (hash, materialize,
and the cell pass's two spills), and the serial re-read layer is the largest
remaining term in the Arrow row diff's wall time. Measuring where the time
goes needs a committed, reproducible harness rather than scratch
instrumentation.

Add a `profile` feature to onix-arrow that compiles per-pass wall-time and
peak-RSS boundaries into diff_rows and the streaming cell pass. The boundaries
expand to nothing without the feature, which is off by default and never
enabled by the release wheel, so the instrumentation is absent from the
shipped library (verified with strings). Peak RSS is sampled from ps in a
background thread, so the feature adds no dependency and no unsafe.

The row_diff_profile example (built only with the feature) spools two
generated sides to anonymous Arrow IPC files and prints the per-pass table,
reproducing the re-read layer the Python bindings' input spool incurs.

Extract materialize_row_members so the added pass boundaries keep diff_rows
within the line budget; behaviour is unchanged.
Add a Profiling section to the perf README naming the row_diff_profile
example, its exact command, and the rule that every row-diff performance
change posts its before/after per-pass table from it; the example's module
docstring is the single home for the method. Add a Per-pass profile section
to RESULTS.md with the narrow- and wide-shape tables at 1M rows and the
re-read breakdown the numbers show.
- Factor the shared streaming generator (Shape, Generated, GenReader, its
  TableInput/RecordBatchReader impls) into examples/shared/gen_shapes.rs,
  included by both row_diff_rss and row_diff_profile; row_diff_rss's generated
  data is unchanged (same code, same counts).
- profile: keep the sampler JoinHandle in state and join it in finish(), so a
  back-to-back begin()/finish() can never leave a second sampler running; add a
  test over 20 cycles asserting no sampler outlives finish(). Sample RSS through
  the absolute /bin/ps rather than resolving ps via PATH, and say the sampler is
  ps-based (macOS/Linux) in the module doc.
- perf docs: state the per-pass table is deepdiff-rs 0.11.2, medians of 11 runs,
  and update the environment row; drop the repeated spool-rationale restatements
  and the follow-up design mechanism, keeping the removable-re-read observation
  with a pointer to #90.
- Instrument the spool decode (spool reader next()) and the spill routing
  (take + cast) so the per-pass decomposition closes and shows decode, not the
  key re-hash, as the dominant re-read term.
- Gate recording on a begin()/finish() window so a diff built with the feature
  but not being profiled records nothing (and cannot grow the global state), and
  the profiler's own tests are deterministic under concurrent diffs.
- Give the example a file mode that reads each side from an uncompressed Arrow
  IPC file with --key/--threads (the real fixtures, converted once from parquet
  with pyarrow), alongside the generated proxy mode; time an uninstrumented run
  next to the instrumented one so the reported wall excludes the ps sampling
  cost.
- Add a mutant-catching test: a right side whose key and value columns sit at
  different positions than the left pins that each side materializes through its
  own SideColumns.
- profile.rs tests (finish joins the sampler, a second begin spawns none,
  label/accum ordering, ps of a missing pid is None); add make test-all-features
  to check so the feature-on instrumentation and these tests run in the gate.
- perf docs: real narrow/wide fixtures at 1M and full in the RESULTS per-pass
  section (decode-dominated, decomposition closing), proxies labelled as proxies;
  document the file mode and the pyarrow conversion in the README.
@ksco92
ksco92 force-pushed the perf/90-spool-reread branch from 86bbb7f to 4257241 Compare September 6, 2026 17:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant