Skip to content

Direct issues to templates instead of GitHub Discussions#8685

Merged
connortsui20 merged 1 commit into
developfrom
ct/remove-discussions
Jul 8, 2026
Merged

Direct issues to templates instead of GitHub Discussions#8685
connortsui20 merged 1 commit into
developfrom
ct/remove-discussions

Conversation

@connortsui20

Copy link
Copy Markdown
Member

Rationale for this change

We want to move away from discussions in general because they are not great to work with and everyone is more familiar with issues.

What changes are included in this PR?

Remove the Discussions redirect from the issue chooser and add Feature Request and Question issue forms in its place. Update CONTRIBUTING.md and the docs to point at issues (or Slack) rather than Discussions, and fix bug_report.yml to apply the existing "bug" label instead of the nonexistent "needs-triage".

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

N/A

Remove the Discussions redirect from the issue chooser and add Feature
Request and Question issue forms in its place. Update CONTRIBUTING.md
and the docs to point at issues (or Slack) rather than Discussions, and
fix bug_report.yml to apply the existing "bug" label instead of the
nonexistent "needs-triage".

Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
@connortsui20 connortsui20 added the changelog/chore A trivial change label Jul 8, 2026
@connortsui20 connortsui20 enabled auto-merge (squash) July 8, 2026 13:45
@codspeed-hq

codspeed-hq Bot commented Jul 8, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

⚡ 5 improved benchmarks
❌ 2 regressed benchmarks
✅ 1600 untouched benchmarks
⏩ 42 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation slice_empty_vortex 310.3 ns 368.6 ns -15.83%
Simulation chunked_varbinview_opt_canonical_into[(100, 100)] 305.2 µs 340.2 µs -10.28%
Simulation chunked_varbinview_into_canonical[(1000, 10)] 206.4 µs 170.3 µs +21.23%
Simulation encode_varbin[(1000, 4)] 161.5 µs 142.2 µs +13.64%
Simulation encode_varbin[(1000, 2)] 159.4 µs 141.7 µs +12.52%
Simulation bitwise_not_vortex_buffer_mut[128] 273.6 ns 244.4 ns +11.93%
Simulation encode_varbin[(1000, 32)] 164.8 µs 148.2 µs +11.22%

Tip

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


Comparing ct/remove-discussions (243ad11) with develop (b9779d6)

Open in CodSpeed

Footnotes

  1. 42 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.

@connortsui20 connortsui20 merged commit 3e32163 into develop Jul 8, 2026
75 of 78 checks passed
@connortsui20 connortsui20 deleted the ct/remove-discussions branch July 8, 2026 13:52
robert3005 pushed a commit that referenced this pull request Jul 14, 2026
A small pool of microbenchmarks flips ±10-35% between two fixed values
on unrelated PRs (including docs-only and lockfile-only changes),
spamming every CodSpeed report. This PR fixes them (and removes only 1
benchmark). No `#[cfg(codspeed)]` gating; CI and local runs stay
identical. One commit per benchmark.

## Changes

- **mimalloc as global allocator** in the 5 flaky `vortex-array` bench
files: `chunk_array_builder`, `dict_compress`, `varbinview_compact`,
`compare`, `binary_ops`
- **`bitwise_not_vortex_buffer_mut`**: drop the 128/1024/2048 sizes
(measured only harness overhead)
- **`slice_empty_vortex`**: rewrite as a 1024-iteration tight loop,
renamed `slice_empty_tight_loop_vortex`
- **`rebuild_naive` (vortex-zstd)**: the one benchmark removed instead
of fixed — its cost is dominated by zstd-internal copies (glibc
`ifunc`-resolved `memcpy`) that no bench-level change can stabilize, its
fixture is degenerate (a 4-string dictionary with all-zero offsets), and
ListView rebuild is already benchmarked across element types and list
shapes in `vortex-array/benches/listview_rebuild.rs`. The crate's
now-unused `divan` dev-dependency goes with it.

<details>
<summary>Which benchmarks were flaky, and the evidence</summary>

Identified by reading the CodSpeed comments on the ~47 PRs merged since
June 25 (post-#8490). The tell: the same benchmark flipping between the
same two values, in both directions, on PRs that can't have affected it
— including deny.toml-only (#8712, #8716), uv.lock-only (#8732),
docs-only (#8737, #8728, #8685), and CI-YAML-only (#8660, #8683)
changes.

| Benchmark | Evidence |
|---|---|
| `bitwise_not_vortex_buffer_mut[128/1024/2048]` | ~half of all PRs —
worst offender |
| `chunked_varbinview_*` ×4 | ~20 PRs, both directions |
| `chunked_bool_canonical_into[(1000,10)]` | ~2× flips (16µs ↔ 35µs) on
4 PRs |
| `encode_varbin`, `encode_varbinview` | ~12 PRs;
`encode_varbinview[(10000,4)]` also flipped on an earlier revision of
this PR |
| `compact`, `compact_sliced` (90%-utilization args) | ~8 PRs |
| `compare_int_constant` | ±11.1% verbatim on ≥9 PRs |
| `eq_i64_constant` | same ±11% signature incl. docs-only PR |
| `slice_empty_vortex` | -14.66% verbatim on ~13 PRs |
| `rebuild_naive` (vortex-zstd) | ~10 PRs, both directions |

Watch list (left alone, below the ≥3-independent-sightings bar):
`copy_nullable`/`copy_non_nullable[65536]` in `cast_decimal.rs`,
`true_count_vortex_buffer[128]`.
</details>

<details>
<summary>Root causes and why each fix matches</summary>

- **Allocation in the timed region** → glibc malloc's code differs
across CodSpeed runner images, so alloc-heavy benchmarks trace
differently for byte-identical Vortex code. Vendored mimalloc removes
glibc malloc from the trace. Empirical support: the only three bench
files already using mimalloc (`single_encoding_throughput`,
`common_encoding_tree_throughput`, `row_encode`) are the most
alloc-heavy suites in the repo and were never flagged once in the 47-PR
window.
- **Sub-microsecond work** → the measurement is fixed harness overhead
plus binary code layout, which shifts with any unrelated change. Fix by
making the operation dominate: the in-place NOT (no alloc, no copy)
keeps only sizes where the loop dominates; the empty slice runs 1024×
per iteration, mirroring the neighboring `slice_tight_loop_vortex`.
- **Environment-bound and low-signal** → `rebuild_naive`, per the
justification above: unfixable at the bench level and redundant with the
vortex-array ListView rebuild suite, so removed.
</details>

<details>
<summary>Validation: A/A reruns and a stacked canary PR</summary>

- **Expected one-time step changes on this PR**: swapping the allocator
changes the trace of every benchmark in the touched files, so this PR's
report shows a few ±10-20% level shifts (including on never-flaky
`encode_primitives`, which just shares a file). These need a one-time
acknowledgment on the CodSpeed dashboard; after merge every PR compares
mimalloc-vs-mimalloc.
- **A/A reruns** (same bench binaries measured three times, on separate
runners and commits): every value reproduced exactly — 211.5µs, 137.1µs,
14.6µs, 26.3µs — with zero new flags across 1656 benchmarks.
- **Canary #8743** (a #8681-style cold-string change stacked on this
branch — the class of change that used to collect five false flags):
**Performance Gate Passed**, `✅ 1660 untouched`, zero changes reported.
</details>

No public API changes; benchmark-only.

https://claude.ai/code/session_01T6PPcdrqcNeUkfi1EGd4oC

---------

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.

2 participants