Skip to content

Deduplicate shared payload geometry columns - #183

Closed
Alek99 wants to merge 1 commit into
mainfrom
agent/dedup-shared-payload-columns
Closed

Deduplicate shared payload geometry columns#183
Alek99 wants to merge 1 commit into
mainfrom
agent/dedup-shared-payload-columns

Conversation

@Alek99

@Alek99 Alek99 commented Jul 22, 2026

Copy link
Copy Markdown
Member

Root cause

The column store already deduplicates canonical arrays at ingest, but every call to _PayloadWriter.ship independently offset-encoded and appended its geometry. K traces sharing one time/x column therefore emitted K byte-identical f32 columns and paid both K encodes and K × 4N wire bytes.

Blind byte/value memoization would be unsafe: per-trace finite masks, log filtering, decimation, and other selections can produce different row mappings from the same source column.

Changes

  • add a writer-local memo for offset-encoded geometry
  • admit only the exact live canonical array (values is col.values)
  • key encoding semantics by live Column/array identity, store-local id, offset, safe scale, kind, and encoder identity
  • retain live references in memo entries to prevent object-id reuse collisions
  • reuse one column-table reference in both packed and split layouts
  • deliberately keep finite/log selections, decimated/derived arrays, and animation u32 identity columns trace-local
  • add packed/split CodSpeed rows for the common multi-series shared-x workload
  • document shared column-table references and the canonical-only safety boundary

The existing protocol already allows multiple trace fields to reference one column-table entry, so no client or wire-format change is needed.

Measured impact

16 direct traces × 150k points with one shared x and 16 distinct y columns; 15 warmed repetitions:

layout main median branch median main bytes/columns branch bytes/columns
packed 1.736 ms 1.231 ms 19.2 MB / 32 10.2 MB / 17
split 1.323 ms 1.088 ms 19.2 MB / 32 10.2 MB / 17

Both layouts reduce 16 distinct x references to one and save exactly 9,000,000 bytes, equal to (K - 1) * 4N. The local medians show 29.1% lower packed assembly time and 17.8% lower split assembly time.

Verification

  • focused canonical/selection/decimation/identity adversarial tests: 12 passed
  • independently rerun figure + animation suites: 195 passed, 1 environment-only Chromium deselection
  • new CodSpeed rows: 2 passed
  • broad core suite: 1565 passed, 29 skipped
  • pyplot suite: 536 passed, 65 skipped
  • Reflex adapter: 89 passed plus 10 real websocket tests
  • Chromium full-path test and two sandbox-blocked integration checks passed outside the restricted sandbox
  • full-tree Ruff lint and format checks: passed
  • changed production source ty: passed
  • Python-floor, public-API, claim-guardrail, and workflow checks: passed
  • changed-file pre-commit and git diff --check: passed

The one known adapter bundle-marker failure reproduces unchanged at b0f8780; it is the minified-marker mismatch already fixed by #177.

Closes #165

@greptile-apps

greptile-apps Bot commented Jul 22, 2026

Copy link
Copy Markdown

Greptile Summary

This PR deduplicates canonical geometry columns within each first-paint payload build. The main changes are:

  • Adds writer-local memoization for canonical offset-encoded geometry.
  • Keeps filtered, decimated, derived, and animation identity columns trace-local.
  • Covers packed and split layouts with tests and performance benchmarks.
  • Documents shared column-table references and the canonical-only boundary.

Confidence Score: 5/5

This looks safe to merge.

No blocking issues found in the changed code.

T-Rex T-Rex Logs

What T-Rex did

  • Ran a focused pytest subset and confirmed the 8 selected tests passed with exit code 0.
  • Compared the before and after layout behavior, noting the drop from 32 columns and 16 unique x references to 17 columns and 1 unique x reference, with bytes decreasing from 8192 to 4352 and the packed and split bytes remaining identical.
  • Collected and reviewed supporting artifacts to provide reviewable evidence of the test subset and the layout metrics.

View all artifacts

T-Rex Ran code and verified through T-Rex

Important Files Changed

Filename Overview
python/xy/_payload.py Adds writer-local canonical geometry memoization keyed by object identity and encoding semantics.
tests/test_figure.py Tests canonical reuse and isolation of selected or decimated geometry in both payload layouts.
tests/test_animation.py Verifies shared geometry while preserving separate animation identity columns.
benchmarks/test_codspeed_kernels.py Adds packed and split benchmarks for multiple traces sharing one canonical x column.
spec/design/wire-protocol.md Documents shared column references and the canonical-only memoization boundary.

Reviews (1): Last reviewed commit: "Deduplicate shared payload geometry colu..." | Re-trigger Greptile

@codspeed-hq

codspeed-hq Bot commented Jul 22, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 97 untouched benchmarks
🆕 2 new benchmarks
⏩ 1 skipped benchmark1

Performance Changes

Benchmark BASE HEAD Efficiency
🆕 test_first_payload_shared_x_packed N/A 29.6 ms N/A
🆕 test_first_payload_shared_x_split N/A 17.2 ms N/A

Comparing agent/dedup-shared-payload-columns (5ce5668) with main (b0f8780)

Open in CodSpeed

Footnotes

  1. 1 benchmark was skipped, so the baseline result was used instead. If it was deleted from the codebase, click here and archive it to remove it from the performance reports.

@Alek99 Alek99 closed this Jul 24, 2026
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.

No ship-time dedup: traces sharing a column encode and ship it K times

1 participant