Generate code that is friendlier to rustc's item_bodies_checking #34
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sujayakar
reviewed
Jan 7, 2019
| @@ -315,14 +315,14 @@ def gen_decomposition_tables(canon_decomp, compat_decomp, out): | |||
| for table, name in tables: | |||
| out.write("#[inline]\n") | |||
| out.write("pub fn %s_fully_decomposed(c: char) -> Option<&'static [char]> {\n" % name) | |||
| out.write(" match c {\n") | |||
| out.write(" Some(match c {\n") | |||
There was a problem hiding this comment.
@dotdash, can you add a comment here explaining the motivation for wrapping the match in Some? otherwise, it may look non-idiomatic to future readers of this codegen script.
|
r=me with @sujayakar's comment addressed |
Having the Some() constructor in each match arm body puts extras stress on rustc's item_bodies_checking pass. We can work around that by moving the Some() constructor around the match, and directly returning None from the default arm, which is the only one that generates a None value. On my box, this almost cuts the time spent in item_bodies_checking in half, going from about 8.7s to about 4.6s, and reduces the complete compile time from about 13s to about 9s, so about a third less. There are no changes in performance in `cargo bench`. Note that doing the same for the composition_table() function does not yield any compile time wins, but would cause a performance regression. cc #29
|
Added the requested comment, and adjusted the commit message to say that there is no change in performance, rather than in the generated assembly code, as |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Having the Some() constructor in each match arm body puts extras stress
on rustc's item_bodies_checking pass. We can work around that by moving
the Some() constructor around the match, and directly returning None
from the default arm, which is the only one that generates a None value.
On my box, this almost cuts the time spent in item_bodies_checking in
half, going from about 8.7s to about 4.6s, and reduces the complete
compile time from about 13s to about 9s, so about a third less.
There are no changes in performance in
cargo bench.Note that doing the same for the composition_table() function does not
yield any compile time wins, but would cause a performance regression.
cc #29