assert more in release in rustc_ast_lowering
#142267
Conversation
workingjubilee
added
the
S-experimental
rustbot
added
the
T-compiler
|
@bors2 try |
This comment has been minimized.
This comment has been minimized.
…ing, r=<try> [EXPERIMENTAL] assert in release builds in `rustc_ast_lowering` My understanding of the compiler's architecture is that in the `ast_lowering` crate, we are constructing the HIR as a one-time thing per crate. This is after tokenizing, parsing, resolution, expansion, possible reparsing, reresolution, reexpansion, and so on. In other words, there are many reasons that perf-focused PRs spend a lot of time touching `rustc_parse`, `rustc_expand`, `rustc_ast`, and then `rustc_hir` and "onwards", but `ast_lowering` is a little bit of an odd duck. In this crate, we have a number of debug assertions. Some are clearly expensive checks that seem like they are prohibitive to run in actual optimized compiler builds, but then there are simple integer equalities. I am curious if the latter introduce a serious problem if we simply *do* them. r? `@ghost`
rustbot
added
the
S-waiting-on-perf
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (d4e2ead): comparison URL. Overall result: ❌✅ regressions and improvements - please read the text belowBenchmarking 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 @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)This benchmark run did not return any relevant results for this metric. 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: 754.885s -> 754.261s (-0.08%) |
rustbot
added
perf-regression
|
hm. I suspect the index functions might be hot enough I need to back their diff out. |
workingjubilee
force-pushed
the
debug-assert-less-in-ast-lowering
branch
from
c62171a to
dd78c95
Compare
|
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
rustbot
added
the
S-waiting-on-perf
…ing, r=<try> [EXPERIMENTAL] assert in release builds in `rustc_ast_lowering` My understanding of the compiler's architecture is that in the `ast_lowering` crate, we are constructing the HIR as a one-time thing per crate. This is after tokenizing, parsing, resolution, expansion, possible reparsing, reresolution, reexpansion, and so on. In other words, there are many reasons that perf-focused PRs spend a lot of time touching `rustc_parse`, `rustc_expand`, `rustc_ast`, and then `rustc_hir` and "onwards", but `ast_lowering` is a little bit of an odd duck. In this crate, we have a number of debug assertions. Some are clearly expensive checks that seem like they are prohibitive to run in actual optimized compiler builds, but then there are simple integer equalities. I am curious if the latter introduce a serious problem if we simply *do* them. r? `@ghost`
|
☀️ Try build successful - checks-actions |
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (a729c24): comparison URL. Overall result: no relevant changes - 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 countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)Results (primary -3.5%, secondary -1.9%)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: 754.321s -> 755.264s (0.13%) |
rustbot
removed
S-waiting-on-perf
workingjubilee
changed the title
rustc_ast_loweringrustc_ast_lowering
|
rustbot has assigned @compiler-errors. Use |
rustbot
added
the
S-waiting-on-review
|
@rustbot label: -S-experimental |
rustbot
removed
the
S-experimental
bors
added
S-waiting-on-bors
|
@bors rollup |
…n-ast-lowering, r=oli-obk assert more in release in `rustc_ast_lowering` My understanding of the compiler's architecture is that in the `ast_lowering` crate, we are constructing the HIR as a one-time thing per crate. This is after tokenizing, parsing, resolution, expansion, possible reparsing, reresolution, reexpansion, and so on. In other words, there are many reasons that perf-focused PRs spend a lot of time touching `rustc_parse`, `rustc_expand`, `rustc_ast`, and then `rustc_hir` and "onwards", but `ast_lowering` is a little bit of an odd duck. In this crate, we have a number of debug assertions. Some are clearly expensive checks that seem like they are prohibitive to run in actual optimized compiler builds, but then there are a number that are simple asserts on integer equalities, `is_empty`, or the like. I believe we should do some of them even in release builds, because the correctness gain is worth the performance cost: almost zero.
…n-ast-lowering, r=oli-obk assert more in release in `rustc_ast_lowering` My understanding of the compiler's architecture is that in the `ast_lowering` crate, we are constructing the HIR as a one-time thing per crate. This is after tokenizing, parsing, resolution, expansion, possible reparsing, reresolution, reexpansion, and so on. In other words, there are many reasons that perf-focused PRs spend a lot of time touching `rustc_parse`, `rustc_expand`, `rustc_ast`, and then `rustc_hir` and "onwards", but `ast_lowering` is a little bit of an odd duck. In this crate, we have a number of debug assertions. Some are clearly expensive checks that seem like they are prohibitive to run in actual optimized compiler builds, but then there are a number that are simple asserts on integer equalities, `is_empty`, or the like. I believe we should do some of them even in release builds, because the correctness gain is worth the performance cost: almost zero.
My understanding of the compiler's architecture is that in the
ast_loweringcrate, we are constructing the HIR as a one-time thing per crate. This is after tokenizing, parsing, resolution, expansion, possible reparsing, reresolution, reexpansion, and so on. In other words, there are many reasons that perf-focused PRs spend a lot of time touchingrustc_parse,rustc_expand,rustc_ast, and thenrustc_hirand "onwards", butast_loweringis a little bit of an odd duck.In this crate, we have a number of debug assertions. Some are clearly expensive checks that seem like they are prohibitive to run in actual optimized compiler builds, but then there are a number that are simple asserts on integer equalities,
is_empty, or the like. I believe we should do some of them even in release builds, because the correctness gain is worth the performance cost: almost zero.