-
Notifications
You must be signed in to change notification settings - Fork 158
Deduplicate some code and move some out of the inner loop #423
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: Daniel Egger <[email protected]>
r? @Disasm (rust_highfive has picked a reviewer for you, use r? to override) |
let quotedfield = String::from("`") + &f.name + "`"; | ||
let readerdoc = String::from("Reader of field ") + "edfield; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe it's better to have just one line for this. Is the performance impact significant compared to format!()
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
core::fmt::write
makes up 17.5% of all CPU cycles used in svd2rust so I'm looking for ways to reduce its usage in addition to trying to have canned code available instead of regenerating everything with every invocation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this code is not faster.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's completely in the noise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about merging into one line? It looks more readable and should be (insignificantly) faster:
let readerdoc = String::from("Reader of field `") + &f.name + "`";
If you prefer to leave things as they are, just bors r= me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have other plans with the quoted field name. ;) Moving out the readerdoc was just to get rid of the three separate definitions.
Still working on figuring out how to make proc_macro2
play ball with respect to being able to pre-generate the TokenTrees/TokenStreams and simply change the field names
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bors r=Disasm
423: Deduplicate some code and move some out of the inner loop r=Disasm a=therealprof Signed-off-by: Daniel Egger <[email protected]> Co-authored-by: Daniel Egger <[email protected]>
Build succeeded |
Signed-off-by: Daniel Egger [email protected]