-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
enable the SLP Vectorizer optimization pass by default #26594
Conversation
@nanosoldier |
Meant to run this: @nanosoldier |
Those test runs don't seem to show noticeable performance improvements. Is this working properly? |
The test run is supposed to show compilation time. |
Oh that makes sense, sorry. |
@ararslan any idea about Nanosoldier? |
Ugh, yeah. It's hitting JuliaWeb/HTTP.jl#220 again. I'll restart the server. |
@nanosoldier |
Any ideas about nanosoldier @ararslan? |
Same error, but it looks like other requests have gotten through, so I'll just try again. GitHub.jl needs to be updated to work with the changes in HTTP.jl, which is why the error keeps happening. @nanosoldier |
Perhaps needs a restart @ararslan? |
I restarted the server, dunno if it will help but worth a try. @nanosoldier |
I ran the benchmarks locally and got https://gist.github.com/KristofferC/c220cb87cda1f77654ed3f89edd5ec60 (filtering out the scalar results because they seemed like noise). My computer was not completely idle when running though so not sure how reliable those are. What I was mostly interested in looking at was the tuple linear algebra ones which all seems to have gotten a significant boost. Would be nice to get a real nanosoldier run though. |
Other Nanosoldier runs have been getting through but for whatever reason this particular PR seems to hit the error from HTTP every time. |
d4bebfd
to
6190bf2
Compare
I definitely think we should do this, though I've been confused over time as to what happens when this is off (e.g. when I don't use |
I modified the logging level in the server, so if this fails, hopefully we'll have a better understanding of why. Sorry for the noise here, Kristoffer, and thanks for your patience. @nanosoldier |
I had to modify the server's local clone of HTTP to fix an @nanosoldier |
This PR hit the same error again, and unfortunately Nanosoldier is now down until further notice for on-site work. |
@nanosoldier |
Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @ararslan |
Glad a Nanosoldier run was able to go through finally. I wouldn't put too much weight on those results though, since the benchmarks hadn't been retuned before running. |
Benchmarks have been retuned. @nanosoldier |
Maybe I should rebase this? I don't think it is running on the merge commit. |
It should always be running on the merge commit unless there are branch conflicts. |
Okay but I m quite sure it doesn't because e.g.the memory regressions from #26435 (comment) also shows up here which mean that this commit includes that PR but not the one that we compared against. |
Oh sorry you're right, I don't think it checks out the merge commit with master if it's comparing against another specific commit. Then yes, it would be good to rebase this. |
6190bf2
to
3de3834
Compare
@nanosoldier let's try this |
Nanosoldier seems to be consistently hitting the |
JuliaWeb/GitHub.jl#108 maybe :/ |
Trying something out, testing in production. #yolo. @nanosoldier |
Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @ararslan |
He lives! |
3de3834
to
8244230
Compare
@nanosoldier |
Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @ararslan |
rebase onto master, and then LGTM |
@nanosoldier |
Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @ararslan |
CI failures are all network failures. |
The justification for this is that it seems to pretty much negligble impact compilation time but has serious performance improvements for linear algebra and other operations with static arrays ( #26398 (comment))
SLP Disabled:
Sysimg build time: User time (seconds): 323.47
Running tests for StaticArrays
SLP Enabled
Sysimg build time: User time (seconds): 329.58
Running tests for StaticArrays