-
Notifications
You must be signed in to change notification settings - Fork 5.9k
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
[release/microbenchmarks] Please take a look at the results (and possibly sign off) #15247
Comments
Note: For these benchmarks the cluster was stopped as it would otherwise complain about passing a |
@AmeerHajAli is the client stable enough for these benchmarks to be meaningful? @krfricke do you have the output of |
I'll run this shortly |
It seems like the single client put call also has regression actually. |
So, it looks like put calls have 20%~ ish regression in general. |
Is it possible to bisect this? |
@krfricke Ignore my comment above. Is this error message appearing in every test?
|
New fresh run:
Comparison with 1.2.0:
|
The error message did not appear this time (and previously only once) |
Sang can you look into why that message might be appearing? We should be
always trying to use devshm when possible, the error message makes it seem
like there is some rounding error where we detected 50GB but it only has
49GB or something.
…On Mon, Apr 12, 2021, 1:05 PM Kai Fricke ***@***.***> wrote:
The error message did not appear this time
—
You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub
<#15247 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAADUSRXFWUITXSGQYUXFBDTINHCJANCNFSM42ZEIYEA>
.
|
cc @wuisawesome ^ |
@krfricke it looks like most of these tests have performance regressions. Can we bisect to figure out when the task/actor submission performance dropped? And also when the |
Did some bisecting. After
After
Conclusion: #14192 probably caused the multi client put regression. |
Thanks Alex for bisecting this. I'll try to find the client remote put calls regression. |
I wonder why |
Found the regression. 4b4941435d42e5b7329388de137a590f376d18bb:
3a230fa1a439a7c6b56099d450faf5702ac5b4ae
Relevant PR: #13919 cc @richardliaw |
Hey @richardliaw Do you take a look at this issue now? |
I am taking a look right now, yes |
@richardliaw assigning you for the ray client regressions @rkooo567 assigning you since you own #15287 (the multi client put regression). |
I tried this again on the most up to date release branch and am getting these results
@rkooo567 looks like merging #15287 did not do the trick. multi client put still has ~12% performance regression. |
Actually @wuisawesome I'm not able to replicate your results. I tried and am still getting
|
Also shouldn't |
Hmm @wuisawesome @amogkam can you guys try bisecting again to isolate the commit? |
Yep I can bisect this |
Running bisect now |
Here seems to be a regression:
Relevant PR: #14345 Hm wait, it seems this PR actually improved performance... |
I think 26907b7 should be a noise. It only changes Ray Java code. |
Could this be a regression here?
It seems like the commits after 72d8709 are also around ~6500 1:1 actor calls PR: #14211 |
Here are the final results that have been signed off by core team:
|
Can you quickly post the absolute (rather than relative) performance or alternatively add them to the release log branch? |
I pushed it to the branch already, but here they are
|
Results from 1.2.0:
It seems there are a few regressions here, e.g.:
and notably
I also got this
is this the reason for the regressions?
cc @wuisawesome @rkooo567 @ericl
The text was updated successfully, but these errors were encountered: