Skip to content

Skip redundant linear visibility scans#179

Draft
Alek99 wants to merge 1 commit into
mainfrom
agent/skip-redundant-visibility-scans
Draft

Skip redundant linear visibility scans#179
Alek99 wants to merge 1 commit into
mainfrom
agent/skip-redundant-visibility-scans

Conversation

@Alek99

@Alek99 Alek99 commented Jul 22, 2026

Copy link
Copy Markdown
Member

Closes #169.

What changed

  • Skip the second finite/log visibility mask for direct line and scatter payloads when both axes are linear. Direct rows have already passed the zone-map-gated finite filter, while M4 output is finite by contract.
  • Let linear area/error-band payloads trust all-finite zone maps for x, y, and the baseline; base-column nulls still force filtering.
  • Preserve positive-domain filtering on log axes and all existing NaN/inf filtering.
  • Avoid constructing a full identity selection for all-visible area traces, which also avoids an unnecessary keyed-transition gather.
  • Update the Rust-engine placement audit and changelog.

Root cause

_finite_sel already used zone-map null_count to avoid scanning all-finite direct x/y columns, but _emit_line and _emit_scatter immediately called _log_visible_mask anyway. That rebuilt isfinite(x) & isfinite(y) and reduced it with np.all even on ordinary linear axes. Area always built the mask and a full flatnonzero selection; its safe fast path also has to account for baseline nulls.

Impact

A forced-direct, all-finite 2,000,000-point scatter payload build improved from 2.141 ms median to 1.569 ms median across 9 warmed runs on this checkout (about 27% faster). Large lines are already screen-bounded by M4; the main gain is large forced-direct scatter and the common no-filter work avoided on every direct xy build.

Verification

  • New regression test proves finite linear line/scatter/area builds never call the visibility-mask path, while log axes and a non-finite area baseline still do.
  • Broad non-browser suite: 2,086 passed, 94 skipped, 1 deselected.
    • Excluded the Reflex adapter directory because main currently contains the minified-marker assertion already fixed by open PR Type the render client: no implicit any, no implicit this #177.
    • Excluded the loopback transport test and Chromium PNG test because this execution sandbox denies loopback binds/browser process operation.
  • ruff check . — passed.
  • ruff format --check . — 341 files already formatted.
  • pre-commit run --all-files — all hooks passed (Ruff check, Ruff format, docs codespell).
  • ty check python/xy/_payload.py — passed.
  • git diff --check — passed.
  • Baseline native evidence from the same clean main: Rust 102 passed, ABI smoke 121 checks passed, committed JS bundles fresh.

Safety notes

The fast path is deliberately narrow:

  • log x/y axes still run positive-domain filtering;
  • direct line/scatter rows with NaN or infinity are filtered by _finite_sel;
  • decimated line/area rows rely on M4's finite-output contract;
  • area/error-band baseline null counts are included in the guard, avoiding the unsafe x/y-only shortcut.

@codspeed-hq

codspeed-hq Bot commented Jul 22, 2026

Copy link
Copy Markdown

Merging this PR will improve performance by 13.04%

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 1 improved benchmark
✅ 96 untouched benchmarks
⏩ 1 skipped benchmark1

Performance Changes

Benchmark BASE HEAD Efficiency
test_animation_plain_payload_100k 3.3 ms 3 ms +13.04%

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/skip-redundant-visibility-scans (a6523cd) 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.

@greptile-apps

greptile-apps Bot commented Jul 22, 2026

Copy link
Copy Markdown

Greptile Summary

This PR avoids redundant visibility scans for finite linear payloads. The main changes are:

  • Skips repeated finite checks for linear line and scatter traces.
  • Adds a guarded fast path for finite area and error-band traces.
  • Preserves log-domain and non-finite baseline filtering.
  • Adds focused tests and updates the changelog and engine audit.

Confidence Score: 5/5

This looks safe to merge.

No blocking issues found in the changed code. Log axes still apply positive-domain filtering. Direct rows and area baselines retain non-finite filtering where required. A missing area selection remains equivalent to selecting every transition key.

T-Rex T-Rex Logs

What T-Rex did

  • Focused pytest exited with code 0, confirming the committed regression coverage passes.
  • The after-harness ran side-by-side comparisons of the parent and PR implementations at equal scope using target/release/libxy_core.so.
  • The after result preserves required filtering while eliminating redundant all-finite linear scans.

View all artifacts

T-Rex Ran code and verified through T-Rex

Important Files Changed

Filename Overview
python/xy/_payload.py Adds narrowly guarded fast paths while preserving required filtering for log axes and columns with non-finite values.
tests/test_figure.py Tests scan avoidance for finite linear traces and retained filtering for log axes and non-finite area baselines.
CHANGELOG.md Documents the visibility-scan optimization and its filtering boundaries.
spec/design/rust-engine.md Updates the engine audit to describe zone-map-based scan avoidance.

Reviews (1): Last reviewed commit: "Skip redundant linear visibility scans" | Re-trigger Greptile

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.

Skip finite/log visibility scans when zone maps already prove all-finite on linear axes

1 participant