Remove chunk size from each chunk in ChunkedBitSet.

[cjgillot]

Almost all chunks in a ChunkedBitSet have the same constant CHUNK_SIZE, except the last in a bitset which takes the remainder domain_size % CHUNK_SIZE.

r? @ghost for perf

[cjgillot]

@bors try @rust-timer queue

[rust-bors]

☀️ Try build successful (CI)
Build commit: acb05a6 (acb05a6e3a3ea9007fbe164aa0c5e056e7463ba6, parent: cd7cbe818e4a66d46fe2df993d1b8518eba8a5cd)

[lqd]

We have an open PR on dense bitsets that removes the domain size as well (and one passes it in in the few cases where it’s needed; it’s usually easy to know where the bitsets are constructed and used) with good effects IIRC, it’s also something we could try on chunked bitsets.

[rust-timer]

Finished benchmarking commit (acb05a6): comparison URL.

Overall result: ❌✅ regressions and improvements - please read the text below

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please do so in sufficient writing along with @rustbot label: +perf-regression-triaged. If not, please fix the regressions and do another perf run. If its results are neutral or positive, the label will be automatically removed.

@bors rollup=never
@rustbot label: -S-waiting-on-perf +perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	1.8%	[0.3%, 3.3%]	2
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.4%	[-0.4%, -0.4%]	1
Improvements ✅ (secondary)	-0.5%	[-1.1%, -0.2%]	4
All ❌✅ (primary)	1.1%	[-0.4%, 3.3%]	3

Max RSS (memory usage)

Results (primary -0.8%, secondary -3.5%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.8%	[-0.8%, -0.8%]	1
Improvements ✅ (secondary)	-3.5%	[-3.5%, -3.5%]	1
All ❌✅ (primary)	-0.8%	[-0.8%, -0.8%]	1

Cycles

Results (primary 2.8%, secondary 4.2%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	2.8%	[2.8%, 2.8%]	1
Regressions ❌ (secondary)	4.2%	[4.2%, 4.2%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	2.8%	[2.8%, 2.8%]	1

Binary size

Results (primary 0.7%, secondary -1.7%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	0.7%	[0.7%, 0.7%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-1.7%	[-1.7%, -1.7%]	1
All ❌✅ (primary)	0.7%	[0.7%, 0.7%]	1

Bootstrap: 469.562s -> 468.956s (-0.13%)
Artifact size: 377.55 MiB -> 377.45 MiB (-0.02%)

[cjgillot]

@lqd do you have a link to the PR for dense bitsets?

[lqd]

Yes, it’s #141325 — it reduces the bitset size in general, but AFAICT is not yet in an easily reviewable state per se. The author @tage64 has told me they want to wrap that work up though.

[cjgillot]

r? compiler

[Mark-Simulacrum]

r=me with nits fixed (mostly the documentation, open either way on the last_chunk_size comment)

r? @Mark-Simulacrum

View changes since this review

[Mark-Simulacrum]

Should this use self.chunk_domain_size(chunk_index)? I guess that maybe doesn't cache the more complex computation (modulus, etc.) but that should only happen once per iteration, so maybe that's better (less register pressure etc)?

[cjgillot]

We are mutably borrowing self.chunks, so borrowck won't let us. And re-implementing is simple enough.

[rustbot]

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[cjgillot]

@bors r=Mark-Simulacrum

[bors]

📌 Commit 9c9a89a has been approved by Mark-Simulacrum

It is now in the queue for this repository.