Shrink mir::Statement to 40 bytes#159012
Conversation
|
@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.
shrink mir::Statement to 40 bytes
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (5aa011a): comparison URL. Overall result: ❌✅ regressions and improvements - please read:Benchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf. Next, please: If you can, justify the regressions found in this try perf run in writing along with @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.4%, secondary -2.3%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary -2.9%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeResults (primary 0.0%, secondary 0.0%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Bootstrap: 489.147s -> 487.269s (-0.38%) |
|
Good Max RSS numbers, but I'm a bit confused by the |
|
Looks like this is just overhead of all the new management code ( @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.
shrink mir::Statement to 40 bytes
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (cc54650): comparison URL. Overall result: ❌✅ regressions and improvements - please read:Benchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf. Next, please: If you can, justify the regressions found in this try perf run in writing along with @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%, secondary -2.4%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (secondary -4.3%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeResults (primary 0.0%, secondary 0.0%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Bootstrap: 487.967s -> 487.075s (-0.18%) |
|
This looks better. |
|
Actually, I want to try one more thing. Let's try to be a lot more aggressive about this inline/outline split. This reduced the overhead a bit in my local cachegrind measurements of @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.
shrink mir::Statement to 40 bytes
|
r? @JohnTitor rustbot has assigned @JohnTitor. Use Why was this reviewer chosen?The reviewer was selected based on:
|
|
Part of the icount result looks like noise (especially I squashed the commits and added a comment. |
We do this by using old-style ThinVec for storing debuginfos collection, which is almost always empty.
053c2b9 to
74dc8e7
Compare
|
Done. I actually don't know whether I have rights to do this. Let's try. @bors r=JohnTitor |
|
@panstromek: 🔑 Insufficient privileges: not in review users |
|
Yea, I can't do this. @JohnTitor can you please r+ this yourself? Sorry for the churn. |
|
Oh sorry I thought you had it... |
|
@bors rollup=iffy |
…ller, r=JohnTitor Shrink mir::Statement to 40 bytes We do this by using (old-style) ThinVec for storing debuginfo. This collection is almost always empty, so we can optimize for that case. I think we could do better by storing the debuginfo elsewhere, but this is the least invasive change for now.
…uwer Rollup of 8 pull requests Successful merges: - #159126 (Fix PR number in bootstrap's change tracker) - #156968 (diagnostics: suggest generic_const_args for const ops) - #159012 (Shrink mir::Statement to 40 bytes) - #158182 (std: use `OnceLock` for SGX argument storage) - #159114 (Add regression test for $-prefixed fragment specifier in repetition) - #159060 (Look for the cdb architecture that corresponds to the target triple) - #159089 (riscv: update c-variadic test for LLVM changes) - #159093 (pretty-print: use inline asm's actual macro name)
…uwer Rollup of 8 pull requests Successful merges: - #159126 (Fix PR number in bootstrap's change tracker) - #156968 (diagnostics: suggest generic_const_args for const ops) - #159012 (Shrink mir::Statement to 40 bytes) - #158182 (std: use `OnceLock` for SGX argument storage) - #159114 (Add regression test for $-prefixed fragment specifier in repetition) - #159060 (Look for the cdb architecture that corresponds to the target triple) - #159089 (riscv: update c-variadic test for LLVM changes) - #159093 (pretty-print: use inline asm's actual macro name)
…uwer Rollup of 14 pull requests Successful merges: - #159126 (Fix PR number in bootstrap's change tracker) - #155811 (Include AtomicU128/AtomicI128 in docs for any target) - #156968 (diagnostics: suggest generic_const_args for const ops) - #159012 (Shrink mir::Statement to 40 bytes) - #156618 (rustdoc: test ignoring rustc lints in CLI) - #158182 (std: use `OnceLock` for SGX argument storage) - #159114 (Add regression test for $-prefixed fragment specifier in repetition) - #158523 (Fix feature gate for `repr(simd)`) - #158876 (Fix multiple logic bugs in `Arc::make_mut`) - #158928 (std: fix Xous UDP send_to length mismatch and truncation) - #159060 (Look for the cdb architecture that corresponds to the target triple) - #159089 (riscv: update c-variadic test for LLVM changes) - #159093 (pretty-print: use inline asm's actual macro name) - #159122 (doc: use ptr::addr in offset_from docs) Failed merges: - #158732 (Apply MCP 1003 and move diagnostics.rs into its own module)
|
@rust-timer build ca5b0d2 |
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (ca5b0d2): comparison URL. Overall result: ❌✅ regressions and improvements - no action neededBenchmarking means the PR may be perf-sensitive. Consider adding rollup=never if this change is not fit for rolling up. @rustbot label: -S-waiting-on-perf -perf-regression 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%, secondary -2.4%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (secondary -2.7%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeResults (primary 0.0%, secondary 0.0%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Bootstrap: 489.809s -> 489.264s (-0.11%) |
View all comments
We do this by using (old-style) ThinVec for storing debuginfo.
This collection is almost always empty, so we can optimize for that case. I think we could do better by storing the debuginfo elsewhere, but this is the least invasive change for now.