Store chunk_domain_size explicitly in Chunk.#147802
Store chunk_domain_size explicitly in Chunk.#147802nnethercote wants to merge 1 commit intorust-lang:mainfrom
chunk_domain_size explicitly in Chunk.#147802Conversation
be864b6 to
3f23715
Compare
|
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
Store `chunk_domain_size` explicitly in `Chunk`.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (f2dee36): comparison URL. Overall result: ✅ improvements - no action neededBenchmarking 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. @bors rollup=never Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (secondary 3.9%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (secondary 2.8%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 474.3s -> 474.199s (-0.02%) |
|
Haha I removed it 2 months ago... #145480 |
3f23715 to
25a2693
Compare
|
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Store `chunk_domain_size` explicitly in `Chunk`.
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (3292998): comparison URL. Overall result: ✅ improvements - no action neededBenchmarking 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. @bors rollup=never Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary 1.3%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary 3.1%, secondary 2.4%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 478.517s -> 480.21s (0.35%) |
|
@cjgillot: going back is a small perf win on cranelift, and the code is bit more concise too. What do you think? |
This comment has been minimized.
This comment has been minimized.
|
r? rust-lang/compiler |
|
Since I reviewed #145480, probably might as well look at this. r? Mark-Simulacrum |
|
FWIW, the context here is that I wrote the original code, and then it was modified (without my knowledge), and then I was looking at it much later and thought "hmm, it seems like it would be better like this" and I managed to perfectly undo the modification and revert it to the original form without realizing. So, points for consistency if nothing else? |
|
r=me with a rebase (and optionally a clean perf run), since perf is slightly positive. But I don't have a strong opinion over whether to store this or not from a code quality perspective. Maybe worth adding a comment to the field noting that we've chosen to keep it for now, linking the ~3 involved PRs? |
25a2693 to
0288e8d
Compare
|
This PR was rebased onto a different main 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. |
|
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Store `chunk_domain_size` explicitly in `Chunk`.
This comment has been minimized.
This comment has been minimized.
Currently we compute it on demand, but it's a little simpler and slightly faster to store it.
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (bd78cbb): comparison URL. Overall result: ✅ improvements - no action neededBenchmarking 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. @bors rollup=never Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (secondary -5.1%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesThis benchmark run did not return any relevant results for this metric. Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 486.041s -> 487.115s (0.22%) |
0288e8d to
72b8bc7
Compare
View all comments
Currently we compute it on demand, but it's a little simpler and slightly faster to store it.
r? @ghost