Skip to content

Validate FSST codes bounds only where an execution context exists - #10588

Merged
robert3005 merged 3 commits into
developfrom
rk/legacy-session-fsst
Oct 20, 2026
Merged

robert3005 merged 3 commits into
developfrom
rk/legacy-session-fsst

Conversation

@robert3005

@robert3005 robert3005 commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

VTable::validate has no context, but FSST validation executed the last codes offset to check it fits in the codes bytes, using legacy_session(). Validation is now split:

  • FSSTData::validate (called by VTable::validate) runs only the structural checks and needs no context.
  • The codes-bounds check moves to validate_codes_bounds, which takes an ExecutionCtx. It runs in FSST::try_new, FSST::try_new_with_symbol_table and both deserialize paths. Deserialize builds its context from the session.

part of #9446

FSST validation only reads the last codes offset when the offsets are
already a host-resident primitive array, so read it directly instead of
executing a scalar with the hidden global `legacy_session()`. The
constructors no longer need their `ExecutionCtx` and drop it.

Signed-off-by: Claude <noreply@anthropic.com>

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y1zHpE1K1Y9HvyU4kt24Tq
@codspeed

codspeed Bot commented Oct 18, 2026 •

Copy link
Copy Markdown

Merging this PR will degrade performance by 14.27%

⚠️ Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

⚠️ 1 benchmark measured no execution time

Nothing ran under measurement, usually because the compiler removed the code under test. This result is not comparable, so it counts as unchanged.

Preventing compiler optimizations

⚠️ 18 benchmarks spent significant time in system calls

System calls cannot be consistently instrumented, so they are not included in the measure, which understates the real cost. Please switch to the Walltime instrument to accurately measure system calls.

Measurement and system calls

ℹ️ 40 benchmarks have no flame graph

The flamegraph could not be generated because we failed to resolve the symbols in the profiling trace. This happens when we fail to harvest information about language frames.

⚠️ 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

⚡ 6 improved benchmarks
❌ 13 regressed benchmarks
✅ 2389 untouched benchmarks
⏩ 12 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
❌ WallTime filter_neon[8] 933 ns 1,835 ns -49.16%
❌ ℹ️ Simulation test_repeated_scan_scalar_at[int_str] 442.9 µs 855.8 µs -48.24%
❌ WallTime threshold_neon[8] 1.5 µs 2.6 µs -42.62%
❌ WallTime filter_neon[64] 1.8 µs 2.9 µs -38.18%
❌ WallTime threshold_neon[64] 2.6 µs 4.1 µs -36.41%
❌ WallTime filter_neon[80] 2.8 µs 3.5 µs -20.71%
❌ WallTime threshold_neon[80] 3.9 µs 4.6 µs -13.87%
❌ ℹ️ Simulation test_scan_filter_to_arrow_array[int_float] 3.5 ms 4.1 ms -13.42%
❌ Simulation or_true_constant 17.8 µs 20.5 µs -13.22%
❌ ⚠️ Simulation test_scan_filter[int] 3.2 ms 3.6 ms -12.96%
❌ WallTime bitpack_blocked_compress_avx2 6.8 µs 7.8 µs -12.49%
❌ ℹ️ Simulation test_repeated_scan_filter[int_float] 3.4 ms 3.9 ms -11.04%
❌ ℹ️ Simulation test_repeated_scan_filter[int] 2.4 ms 2.7 ms -10.51%
⚡ WallTime mul_u64_nonnull_neon 39.8 µs 28.7 µs +38.83%
⚡ WallTime multiply_shapes_neon[(32768, PerRowPerRow)] 38.5 µs 32.5 µs +18.44%
⚡ WallTime mul_i64_nonnull_neon 38.4 µs 32.5 µs +18.25%
⚡ Simulation sum_i64 225.1 µs 194.6 µs +15.71%
⚡ Simulation sum_v2_i64 224.7 µs 194.2 µs +15.68%
⚡ WallTime mul_u16_nonnull_avx2 12.2 µs 10.9 µs +11.62%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing rk/legacy-session-fsst (4c25064) with develop (31e0724)2

Open in CodSpeed

Footnotes

  1. 12 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

  2. No successful run was found on develop (f5f222a) during the generation of this report, so 31e0724 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

Restore the execution context on FSST::try_new, try_new_with_symbol_table and
deserialize, which check that the last codes offset fits in the codes bytes.
VTable::validate has no context, so it now runs only the structural checks
instead of executing with the global legacy session.

Signed-off-by: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y1zHpE1K1Y9HvyU4kt24Tq
The bounds check executes the last codes offset, so it works for compressed
offsets too. Drop the primitive-only restriction and propagate errors instead
of panicking.

Signed-off-by: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y1zHpE1K1Y9HvyU4kt24Tq
@robert3005 robert3005 changed the title Validate FSST arrays without an execution context Validate FSST codes bounds only where an execution context exists Oct 19, 2026
@robert3005 robert3005 added the changelog/chore A trivial change label Oct 19, 2026
@robert3005
robert3005 marked this pull request as ready for review October 19, 2026 12:58
@robert3005
robert3005 enabled auto-merge (squash) October 19, 2026 12:58
@robert3005
robert3005 merged commit 00aaffa into develop Oct 20, 2026
123 of 126 checks passed
@robert3005
robert3005 deleted the rk/legacy-session-fsst branch October 20, 2026 11:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/chore A trivial change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants