-
Notifications
You must be signed in to change notification settings - Fork 10.5k
switch var to let in benchmark directory #24011
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
CC: @lorentey Please review this. |
@swift-ci Please benchmark |
let and var can generate slightly different code in SILGen... let’s run benchmarks just in case |
Build failed before running benchmark. |
@slavapestov |
@slavapestov |
@swift-ci please benchmark |
Performance: -O
Code size: -O
How to read the dataThe tables contain differences in performance which are larger than 8% and differences in code size which are larger than 1%.If you see any unexpected regressions, you should consider fixing the Noise: Sometimes the performance results (not code size!) contain false Hardware Overview
|
@CodaFi thanks! |
I can’t remember if the smoke tests check SIL in the benchmarks. May as well get this over with @swift-ci please test |
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.
Looks good, thanks!
Build failed |
Somehow |
@CodaFi |
I think it's because you touched some GYB'd files by mistake. StringWalk notes that it is a generated file, so you should revert your edit there and see if that fixes things. If it does, just change the GYB template. |
OK thank you |
This reverts commit 6133471.
@Gumichocopengin8 You'll also need to manually run generate_harness.py to regenerate the |
@swift-ci test |
@swift-ci benchmark |
Build failed |
Performance: -O
Code size: -O
How to read the dataThe tables contain differences in performance which are larger than 8% and differences in code size which are larger than 1%.If you see any unexpected regressions, you should consider fixing the Noise: Sometimes the performance results (not code size!) contain false Hardware Overview
|
This comment has been minimized.
This comment has been minimized.
Does it often happen to fail either OS X or Linux? |
Unrelated failures inevitably happen from time to time if the master branch gets temporarily destabilized. They should be the exception, not the rule, though.
|
@swift-ci please test linux platform |
Build failed |
I am investigating. It is possibly helpful to note that the specific tests that are crashing haven't changed in around a week. |
@millenomi |
@swift-ci smoke test |
@lorentey @jrose-apple Could you run test please? |
@Gumichocopengin8 Can you do a rebase instead of a merge? |
We can squash to a single commit at merge time, so we can make do without a rebase here. @swift-ci please smoke test |
I use
let
insteadvar
.