chore: forbid the locking Id constructors with a clippy lint#8617
Conversation
b418f23 to
f58aa0b
Compare
Merging this PR will not alter performance
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Simulation | slice_empty_vortex |
339.4 ns | 397.8 ns | -14.66% |
| ⚡ | Simulation | bitwise_not_vortex_buffer_mut[128] |
273.6 ns | 244.4 ns | +11.93% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing miniex:perf/disallow-id-new-lint (74b8c39) with develop (4a90e13)
Footnotes
-
4 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. ↩
|
I think you need to rebase and fix |
f58aa0b to
301c6d9
Compare
`Id::new`/`Id::new_static` intern into a globally locked table on every call. A disallowed-methods lint now forbids them: static ids use a `CachedId` static, and the few dynamic-string sites keep `Id::new` behind `#[expect]`. `CachedId` interns via `Id::new_static`. Refs: vortex-data#8380 Signed-off-by: Han Damin <miniex@daminstudio.net>
301c6d9 to
74b8c39
Compare
Summary
Follow-up to #8614. A
clippydisallowed-methods lint now forbidsId::newandId::new_staticworkspace-wide, since both intern into a globally locked table (ThreadedRodeo) on every call. Static identifiers must use aCachedIdstatic instead; the sites that intern a genuinely dynamic string (proto/flatbuffer deserialization, arrow extension lookups, Python read contexts, tests) keepId::newbehind an#[expect(clippy::disallowed_methods, reason = "...")].CachedIditself now interns throughId::new_static, so it is the single sanctioned path for static ids.This is the enforcement half of #8380; #8614 did the migration. It is stacked on #8614, so it stays a draft until that merges; the shared migration commit drops out once this is rebased onto a merged #8614.
Refs: #8380
Testing
cargo clippy --all-targetsis clean across the edited crates (vortex-array,vortex-layout,vortex-file,vortex-ipc,vortex-zstd,vortex-python,vortex-duckdb,vortex-session), withdisallowed_methodsandunfulfilled_lint_expectationsboth at zero;cargo +nightly fmt --all --checkclean.vortex-cudacould not be built locally (no CUDA toolchain).I'm Korean, so sorry if any wording reads a little awkward.