-
Notifications
You must be signed in to change notification settings - Fork 12.1k
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
Improve error message: missing ;
in macro_rules
#125180
Conversation
r? @fee1-dead rustbot has assigned @fee1-dead. Use |
;
in macro_rules;
in macro_rules
this code is very perf sensitive, so let's check |
This comment has been minimized.
This comment has been minimized.
… r=<try> Improve error message: missing `;` in macro_rules Fixes rust-lang#124968
☀️ Try build successful - checks-actions |
This comment has been minimized.
This comment has been minimized.
Finished benchmarking commit (7af9416): comparison URL. Overall result: ❌ regressions - ACTION NEEDEDBenchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf. Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @bors rollup=never Instruction countThis is a highly reliable metric that was used to determine the overall result at the top of this comment.
Max RSS (memory usage)Results (primary 2.3%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesResults (primary 3.6%, secondary 3.3%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 679.603s -> 680.293s (0.10%) |
@Nilstrieb How about the result? Looks like this change slows down Rustc a bit. |
yeah, that's too much. i very much like the improvement, but you need to try to figure out a faster way. maybe you can make use of the trait abstraction, where you try to avoid all work for the no-op tracker |
I've written that abstraction and logic myself, so if you have questions feel free to ask |
@Nilstrieb thanks! I will try it ;) |
26f0246
to
9081a66
Compare
I don't know if I have access but |
@mu001999: 🔑 Insufficient privileges: not in try users |
This comment has been minimized.
This comment has been minimized.
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
… r=<try> Improve error message: missing `;` in macro_rules Fixes rust-lang#124968
☀️ Try build successful - checks-actions |
This comment has been minimized.
This comment has been minimized.
Finished benchmarking commit (7308417): comparison URL. Overall result: ✅ improvements - no action neededBenchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf. @bors rollup=never Instruction countThis is a highly reliable metric that was used to determine the overall result at the top of this comment.
Max RSS (memory usage)Results (primary 2.5%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesResults (primary 0.7%, secondary 1.4%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 670.875s -> 671.044s (0.03%) |
@Nilstrieb The new result looks good! How do you think of this new change? Should I refactor the code? |
very cool. I skimmed the code and it looks okay, I will review it later. |
b75d408
to
62f509b
Compare
This comment has been minimized.
This comment has been minimized.
62f509b
to
c2be134
Compare
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.
LGTM, thanks!
@bors r+ |
☀️ Test successful - checks-actions |
Finished benchmarking commit (685a80f): comparison URL. Overall result: no relevant changes - no action needed@rustbot label: -perf-regression Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)This benchmark run did not return any relevant results for this metric. CyclesResults (primary 4.6%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 668.952s -> 669.005s (0.01%) |
Fixes #124968