Avoid copies in live update attachments#184
Conversation
Merging this PR will improve performance by 13.26%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ⚡ | test_density_view_exact_pan |
12.6 ms | 11.2 ms | +13.26% |
Tip
Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.
Comparing agent/zero-copy-update-buffers (6527b06) with main (2a7d898)
Footnotes
-
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. ↩
Greptile SummaryThis PR removes redundant copies from live-update binary attachments. The main changes are:
Confidence Score: 5/5This looks safe to merge. No blocking issues found in the changed code.
What T-Rex did
Important Files Changed
Reviews (1): Last reviewed commit: "Avoid copies in live update attachments" | Re-trigger Greptile |
Root cause
Live view-update assembly encoded arrays into the required contiguous f32/u8/u32 representation and then immediately copied every array again with
ndarray.tobytes(). A 200k-point drill-in can attach roughly 4 MB across geometry, channels, and density values, so the redundant ownership copy sat directly on the zoom/selection latency path.The anywidget comm and scatter/gather
XYBFframing already accept contiguous buffer-protocol objects. Socket.IO is the one adapter that requires owned bytes and already has an explicit conversion boundary.Changes
WireBuffer = bytes | memoryviewcontract and owner-retaining ndarray byte-view helpermemoryviewattachments in:BufferWriter.add_f32/add_u8encode_frame_partsbytes()conversion at the python-socketio boundaryon_selectexists, so user mutation cannot rewrite an already assembled attachment; callback-free replies remain zero-copyFrame bytes, metadata, selection masks, and protocol shape are unchanged.
Measured impact
A direct 1,000,000-element contiguous f32 attachment (4,000,000 bytes), measured after source allocation:
bytesmemoryviewAt 200k scale, isolated avoidable attachment assembly saved approximately:
End-to-end selection remains dominated by its earlier
np.uniquescan; the deterministic win here is removal of the second payload-sized ownership copy. Alternating same-process full-path timing showed forced-copy overhead of about 0.0015 ms (decimation), 0.0111 ms (density), 0.1236 ms (selection), and 0.0530 ms (drill).Verification
encode_frame_partsowner retentionty: passedgit diff --check: passedThe remaining broad-suite failure is the current-main minified-bundle marker mismatch, reproduced unchanged at
b0f8780and already fixed by #177. The sandbox-only loopback and Chromium checks both passed outside the restricted sandbox.Closes #173