Skip to content

Skip building binaries in when there are no tests#8820

Merged
AdamGS merged 1 commit into
developfrom
adamg/skip-buildings-binaries-tests
Jul 17, 2026
Merged

Skip building binaries in when there are no tests#8820
AdamGS merged 1 commit into
developfrom
adamg/skip-buildings-binaries-tests

Conversation

@AdamGS

@AdamGS AdamGS commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Rationale for this change

We have many binaries in the main workspace, many of them don't contain any tests (in the final main.rs or equivalent file).

What changes are included in this PR?

Sets test = false for most binaries in the workspace, which means that running cargo test or cargo nextest won't build the final binary.

I've also restructured random-access a bit (with claude's help) so it actually has a dedicated lib.rs.

What APIs are changed? Are there any user-facing changes?

None

Signed-off-by: Adam Gutglick <adam@spiraldb.com>
@AdamGS AdamGS added the changelog/chore A trivial change label Jul 17, 2026
@joseph-isaacs

Copy link
Copy Markdown
Contributor

This will mean if we do add tests to these mod, they will be skipped?

@AdamGS

AdamGS commented Jul 17, 2026

Copy link
Copy Markdown
Contributor Author

If you put a test in main.rs or something - yes, it'll be skipped.

@github-actions

github-actions Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Polar Signals Profiling Results

Latest Run

Status Commit Job Attempt Link
🟢 Done 0548331 1 Explore Profiling Data

Powered by Polar Signals Cloud

@github-actions

Copy link
Copy Markdown
Contributor

Benchmarks: Vortex queries

Verdict: No clear signal (low confidence)
Attributed Vortex impact: -1.0%
Engines: DataFusion Likely regression (+10.4%, medium confidence) · DuckDB Likely improvement (-11.1%, high confidence)
Vortex (geomean): 0.865x ✅
Parquet (geomean): 0.869x ✅
Shifts: Parquet (control) -13.1% · Median polish -14.0%

How to read Verdict and Engines
  • Verdict: Overall PR-level signal after subtracting baseline drift estimated from Parquet control rows. It can be Likely improvement, Likely regression, or No clear signal.
  • Engines: Per-engine attribution. DataFusion is compared against DataFusion/Parquet controls; DuckDB is compared against DuckDB/Parquet controls. This answers whether each engine improved or regressed independently.
  • Confidence: Based on directional consistency, share of rows above the noise floor, and control-run noise.

datafusion / vortex-file-compressed (0.887x ✅, 2↑ 0↓)
name PR 0548331 (ns) base 3513367 (ns) ratio (PR/base)
vortex_q00/datafusion:vortex-file-compressed 🚀 9680427 10844188 0.89
vortex_q01/datafusion:vortex-file-compressed 🚀 6241528 7079877 0.88
datafusion / parquet (0.804x ✅, 2↑ 0↓)
name PR 0548331 (ns) base 3513367 (ns) ratio (PR/base)
vortex_q00/datafusion:parquet 🚀 19214706 23277875 0.83
vortex_q01/datafusion:parquet 🚀 4477573 5721674 0.78
duckdb / vortex-file-compressed (0.835x ✅, 2↑ 0↓)
name PR 0548331 (ns) base 3513367 (ns) ratio (PR/base)
vortex_q00/duckdb:vortex-file-compressed 🚀 9855367 11858063 0.83
vortex_q01/duckdb:vortex-file-compressed 🚀 5881897 7019048 0.84
duckdb / parquet (0.939x ➖, 0↑ 0↓)
name PR 0548331 (ns) base 3513367 (ns) ratio (PR/base)
vortex_q00/duckdb:parquet 23280585 25059837 0.93
vortex_q01/duckdb:parquet 9378896 9882780 0.95

No file size changes detected.

@codspeed-hq

codspeed-hq Bot commented Jul 17, 2026

Copy link
Copy Markdown

Merging this PR will improve performance by 11.17%

⚠️ 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 improved benchmark
✅ 1712 untouched benchmarks
⏩ 10 skipped benchmarks1

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation true_count_vortex_buffer[128] 580.6 ns 522.2 ns +11.17%

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 adamg/skip-buildings-binaries-tests (0548331) with develop (da76f5b)2

Open in CodSpeed

Footnotes

  1. 10 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 (3513367) during the generation of this report, so da76f5b was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@AdamGS
AdamGS merged commit 10b8173 into develop Jul 17, 2026
87 of 88 checks passed
@AdamGS
AdamGS deleted the adamg/skip-buildings-binaries-tests branch July 17, 2026 13:19
@joseph-isaacs

Copy link
Copy Markdown
Contributor

Seems like a correctness bug waiting to happen

@AdamGS

AdamGS commented Jul 17, 2026

Copy link
Copy Markdown
Contributor Author

what code do we have that depends on this sort of test and that's actually important?

connortsui20 added a commit that referenced this pull request Jul 20, 2026
## Rationale for this change

- Closes: #8807

Three benchmarks stayed flaky after #8742, flipping between the same two
values on PRs that can't affect them. Same root causes and fixes as
#8742:

| Benchmark | Seen flaky on | Why | Fix |
| --- | --- | --- | --- |
| `true_count_vortex_buffer[128]` | ±11.17% on 9 unrelated PRs (#8805,
#8811, #8812, #8820, #8843, #8803, …) | a 128-bit popcount measures
harness overhead and code layout, not the count | drop the 128 size |
| runend `compress[(100000, 4)]` | ±11.9% on #8805, #8750, #8856 |
allocates in the timed region; glibc malloc differs across runner images
| mimalloc as global allocator |
| `cast_decimal` `copy_*[65536]` | identical flags on #8838 and #8724 |
same glibc-malloc cause (512 KB alloc per iteration) | mimalloc as
global allocator |

Left alone: `compact_sliced[(4096, 90)]` (single sighting) and the CUDA
walltime benches (hosted-runner walltime noise, a runner config issue).

The allocator swap shifts every benchmark in the two touched binaries
once — see the comment below. Needs a one-time CodSpeed acknowledgment,
like #8742.

## What changes are included in this PR?

One commit per benchmark; bench files only. Ran `cargo check` + `clippy`
on the three bench targets, smoke-ran the binaries, `cargo +nightly
fmt`.

---------

Signed-off-by: Claude <noreply@anthropic.com>
Co-authored-by: Claude <noreply@anthropic.com>
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