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 |
Currently we compute it on demand, but it's a little simpler and slightly faster to store it.
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? |
|
☔ The latest upstream changes (presumably #153865) made this pull request unmergeable. Please resolve the merge conflicts. |
|
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? |
Currently we compute it on demand, but it's a little simpler and slightly faster to store it.
r? @ghost