Commit 19f771f
perf: allow cross-crate inlining of BitBuffer accessors (#9285)
Related: #9259
## Rationale for this change
`Buffer<T>` and `BufferMut<T>` are generic, so their MIR travels in the
rlib and a downstream crate inlines them without LTO. `BitBuffer`,
`BitBufferMut`, `BitBufferView` and `BitBufferMutView` are concrete, so
a method without `#[inline]` reaches a downstream crate as a declaration
only. The thin ones cost a real call per invocation, and the caller
loses the offset and length constants it needs to fold the surrounding
code.
Note that `#[inline]` is not what enables inlining across codegen units
inside a crate. MIR inlining runs before partitioning, and
`profile.bench` sets `lto = false`, which is thin-local LTO rather than
no LTO. Both already inline small functions across a codegen unit
boundary within `vortex-buffer`. The attribute only matters across the
crate boundary, which is where every measurement below was taken.
## What changes are included in this PR?
`#[inline]` on the thin wrappers of those four types: constructors,
iterator factories, and the methods that forward a slice, an offset and
a length. Methods with a body worth outlining keep their current
behavior, including `append_buffer`, whose bitvec fallback path is too
large to justify inlining for the 1.6% it measured.
Marginal instructions per iteration, measured with callgrind against a
probe crate that calls `vortex-buffer` across a crate boundary, built at
the `profile.bench` settings:
| kernel | before | after | fat LTO |
| --- | --- | --- | --- |
| `BitBuffer::slice` | 8972 | 6415 | 5390 |
| `BitBufferView::slice` | 3017 | 2186 | 1994 |
`slice_vortex_buffer` measures 1.904us to 1.829us of wall time, and
stops intermittently landing on a slower 2.12us mode. The wall-clock
gain is much smaller than the instruction-count gain because these paths
are bound by refcount atomics rather than by instruction issue. CodSpeed
measures instruction counts, so expect its numbers to sit closer to the
table than to the wall time.
<details>
<summary>Measurement method, and the parts of the LTO win this does not
reach</summary>
The probe is a separate crate that calls `vortex-buffer` over a real
crate boundary, so the cross-crate path is the one under test. Each
kernel runs at two iteration counts under `valgrind --tool=callgrind`
and the totals are differenced, which cancels process startup, CPU
feature warmup and setup allocations. This matches what CodSpeed's
Simulation mode reports.
`BitBuffer::set_indices` goes 77702 to 69503 at `codegen-units = 1`. At
`codegen-units = 16` the baseline lands on the faster value about half
the time depending on how thin-local LTO's import decisions fall, so the
change makes a previously partition-dependent win reliable rather than
producing a new one.
The remaining fat LTO gap on these benchmarks is not cross-crate
inlining, and `#[inline]` cannot reach it:
- `set_slices` is 1.68x, and it is arrow's `BitSliceIterator`. Ten
`#[inline]` attributes on the `BitSliceIterator` and `UnalignedBitChunk`
chain recover 13729 to 10026 with no LTO. That belongs upstream in
arrow-rs.
- `from_iter` and `bitand_owned` are 1.9x. Nightly
`-Zcross-crate-inline-threshold=always` does not move either one, so no
amount of MIR availability explains them. Fat LTO is partially rescuing
a per-bit read-modify-write loop by unrolling it. The real fix is that
`BitBuffer::from_iter` costs 5.19 instructions per bit while
`BitBufferMut::from(&[bool])` does the same job at 0.19 through the
word-packing kernels in `pack.rs`. Follow-up.
- `value_vortex_buffer` and `value_arrow_buffer` both reported +56.8% on
#9259. The probe measures both at exactly 147467 instructions in every
profile. That row is divan overhead.
Thin LTO was measured as an alternative and rejected: 0 to 2.5% across
these kernels for 2.5x the bench build time.
</details>
Signed-off-by: "Connor Tsui" <connor@spiraldb.com>
Co-authored-by: Claude <noreply@anthropic.com>1 parent fed7038 commit 19f771f
4 files changed
Lines changed: 55 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
95 | 95 | | |
96 | 96 | | |
97 | 97 | | |
| 98 | + | |
98 | 99 | | |
99 | 100 | | |
100 | 101 | | |
| |||
115 | 116 | | |
116 | 117 | | |
117 | 118 | | |
| 119 | + | |
118 | 120 | | |
119 | 121 | | |
120 | 122 | | |
| |||
142 | 144 | | |
143 | 145 | | |
144 | 146 | | |
| 147 | + | |
145 | 148 | | |
146 | 149 | | |
147 | 150 | | |
| |||
154 | 157 | | |
155 | 158 | | |
156 | 159 | | |
| 160 | + | |
157 | 161 | | |
158 | 162 | | |
159 | 163 | | |
| |||
171 | 175 | | |
172 | 176 | | |
173 | 177 | | |
| 178 | + | |
174 | 179 | | |
175 | 180 | | |
176 | 181 | | |
177 | 182 | | |
178 | 183 | | |
| 184 | + | |
179 | 185 | | |
180 | 186 | | |
181 | 187 | | |
| |||
271 | 277 | | |
272 | 278 | | |
273 | 279 | | |
| 280 | + | |
274 | 281 | | |
275 | 282 | | |
276 | 283 | | |
| |||
345 | 352 | | |
346 | 353 | | |
347 | 354 | | |
| 355 | + | |
348 | 356 | | |
349 | 357 | | |
350 | 358 | | |
| |||
376 | 384 | | |
377 | 385 | | |
378 | 386 | | |
| 387 | + | |
379 | 388 | | |
380 | 389 | | |
381 | 390 | | |
382 | 391 | | |
383 | 392 | | |
384 | 393 | | |
385 | 394 | | |
| 395 | + | |
386 | 396 | | |
387 | 397 | | |
388 | 398 | | |
| |||
413 | 423 | | |
414 | 424 | | |
415 | 425 | | |
| 426 | + | |
416 | 427 | | |
417 | 428 | | |
418 | 429 | | |
| |||
424 | 435 | | |
425 | 436 | | |
426 | 437 | | |
| 438 | + | |
427 | 439 | | |
428 | 440 | | |
429 | 441 | | |
430 | 442 | | |
431 | 443 | | |
| 444 | + | |
432 | 445 | | |
433 | 446 | | |
434 | 447 | | |
435 | 448 | | |
436 | 449 | | |
| 450 | + | |
437 | 451 | | |
438 | 452 | | |
439 | 453 | | |
| |||
484 | 498 | | |
485 | 499 | | |
486 | 500 | | |
| 501 | + | |
487 | 502 | | |
488 | 503 | | |
489 | 504 | | |
490 | 505 | | |
491 | 506 | | |
| 507 | + | |
492 | 508 | | |
493 | 509 | | |
494 | 510 | | |
| |||
510 | 526 | | |
511 | 527 | | |
512 | 528 | | |
| 529 | + | |
513 | 530 | | |
514 | 531 | | |
515 | 532 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
102 | 102 | | |
103 | 103 | | |
104 | 104 | | |
| 105 | + | |
105 | 106 | | |
106 | 107 | | |
107 | 108 | | |
| |||
125 | 126 | | |
126 | 127 | | |
127 | 128 | | |
| 129 | + | |
128 | 130 | | |
129 | 131 | | |
130 | 132 | | |
| |||
134 | 136 | | |
135 | 137 | | |
136 | 138 | | |
| 139 | + | |
137 | 140 | | |
138 | 141 | | |
139 | 142 | | |
| |||
143 | 146 | | |
144 | 147 | | |
145 | 148 | | |
| 149 | + | |
146 | 150 | | |
147 | 151 | | |
148 | 152 | | |
| |||
158 | 162 | | |
159 | 163 | | |
160 | 164 | | |
| 165 | + | |
161 | 166 | | |
162 | 167 | | |
163 | 168 | | |
| |||
247 | 252 | | |
248 | 253 | | |
249 | 254 | | |
| 255 | + | |
250 | 256 | | |
251 | 257 | | |
252 | 258 | | |
253 | 259 | | |
254 | 260 | | |
| 261 | + | |
255 | 262 | | |
256 | 263 | | |
257 | 264 | | |
| |||
299 | 306 | | |
300 | 307 | | |
301 | 308 | | |
| 309 | + | |
302 | 310 | | |
303 | 311 | | |
304 | 312 | | |
| |||
308 | 316 | | |
309 | 317 | | |
310 | 318 | | |
| 319 | + | |
311 | 320 | | |
312 | 321 | | |
313 | 322 | | |
| |||
415 | 424 | | |
416 | 425 | | |
417 | 426 | | |
| 427 | + | |
418 | 428 | | |
419 | 429 | | |
420 | 430 | | |
| |||
609 | 619 | | |
610 | 620 | | |
611 | 621 | | |
| 622 | + | |
612 | 623 | | |
613 | 624 | | |
614 | 625 | | |
615 | 626 | | |
616 | 627 | | |
| 628 | + | |
617 | 629 | | |
618 | 630 | | |
619 | 631 | | |
620 | 632 | | |
621 | 633 | | |
| 634 | + | |
622 | 635 | | |
623 | 636 | | |
624 | 637 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
19 | 19 | | |
20 | 20 | | |
21 | 21 | | |
| 22 | + | |
22 | 23 | | |
23 | 24 | | |
24 | 25 | | |
| |||
29 | 30 | | |
30 | 31 | | |
31 | 32 | | |
| 33 | + | |
32 | 34 | | |
33 | 35 | | |
34 | 36 | | |
| |||
44 | 46 | | |
45 | 47 | | |
46 | 48 | | |
| 49 | + | |
47 | 50 | | |
48 | 51 | | |
49 | 52 | | |
| |||
0 commit comments