Join GitHub today
GitHub is home to over 28 million developers working together to host and review code, manage projects, and build software together.
Sign up[do not merge] Test parallel queries #56765
Conversation
rust-highfive
assigned
michaelwoerister
Dec 13, 2018
rust-highfive
added
the
S-waiting-on-review
label
Dec 13, 2018
This comment has been minimized.
This comment has been minimized.
|
@bors try |
This comment has been minimized.
This comment has been minimized.
added a commit
that referenced
this pull request
Dec 13, 2018
This comment has been minimized.
This comment has been minimized.
|
I was able to do a local perf.rlo run last we week. I'll post the results here if this doesn't succeed. |
michaelwoerister
referenced this pull request
Dec 13, 2018
Closed
[do not merge] Benchmark parallel queries on perf.rlo. #56377
This comment has been minimized.
This comment has been minimized.
|
|
This comment was marked as outdated.
This comment was marked as outdated.
|
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
bors
added
S-waiting-on-author
and removed
S-waiting-on-review
labels
Dec 13, 2018
This comment has been minimized.
This comment has been minimized.
|
@bors retry |
This comment has been minimized.
This comment has been minimized.
added a commit
that referenced
this pull request
Dec 13, 2018
This comment has been minimized.
This comment has been minimized.
|
|
This comment was marked as outdated.
This comment was marked as outdated.
|
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
This comment was marked as outdated.
This comment was marked as outdated.
|
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
This comment has been minimized.
This comment has been minimized.
|
@bors try |
This comment has been minimized.
This comment has been minimized.
added a commit
that referenced
this pull request
Dec 13, 2018
This comment has been minimized.
This comment has been minimized.
|
|
This comment was marked as outdated.
This comment was marked as outdated.
|
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
Zoxc
force-pushed the
Zoxc:pq-test
branch
from
435a684
to
16d6920
Dec 13, 2018
This comment has been minimized.
This comment has been minimized.
|
@bors try |
This comment has been minimized.
This comment has been minimized.
added a commit
that referenced
this pull request
Dec 13, 2018
This comment was marked as outdated.
This comment was marked as outdated.
|
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
This comment has been minimized.
This comment has been minimized.
|
|
This comment was marked as outdated.
This comment was marked as outdated.
|
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
This comment has been minimized.
This comment has been minimized.
|
@bors try |
This comment has been minimized.
This comment has been minimized.
|
So what’s next? If we got rid of some the regressions, could we turn it on by default? |
Zoxc
and others
added some commits
Dec 13, 2018
Zoxc
force-pushed the
Zoxc:pq-test
branch
from
762e3fd
to
9ff6e14
Dec 23, 2018
This comment has been minimized.
This comment has been minimized.
|
@bors try |
This comment has been minimized.
This comment has been minimized.
added a commit
that referenced
this pull request
Dec 23, 2018
This comment has been minimized.
This comment has been minimized.
|
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
This comment has been minimized.
This comment has been minimized.
|
|
This comment has been minimized.
This comment has been minimized.
|
@rust-timer build f471219 |
This comment has been minimized.
This comment has been minimized.
rust-timer
commented
Dec 23, 2018
|
Success: Queued f471219 with parent ddab10a, comparison URL. |
This comment has been minimized.
This comment has been minimized.
rust-timer
commented
Dec 23, 2018
|
Finished benchmarking try commit f471219 |
This comment has been minimized.
This comment has been minimized.
|
@bors try |
This comment has been minimized.
This comment has been minimized.
added a commit
that referenced
this pull request
Dec 23, 2018
This comment has been minimized.
This comment has been minimized.
|
|
This comment has been minimized.
This comment has been minimized.
|
I'm confused. Those tests look like they actually improved by 5-10% Or am I looking at the wrong link? #56765 (comment) |
This comment has been minimized.
This comment has been minimized.
|
@oli-obk The default view sorts by instructions, sort by wall-time instead and the ctfe tests get much slower |
This comment has been minimized.
This comment has been minimized.
|
|
This comment has been minimized.
This comment has been minimized.
|
I would have expected incremental to be slower, as decoding large constants can happen to duplicate the work with parallel decoding and then throw away all results but the first one to finish. I don't know why the non-incremental case is slower. It's not like we're evaluating queries twice, right? Especially weird that wall time would go down. Maybe it's just because evaluating those large constants will run a lot of queries (one or two for all the temporary locals that are written to) and I presume that a single query's evaluation time will be slower than before due to extra synchronizations? Also note that these tests are know to fluctuate by 2-5% all the time. Maybe that fluctuation gets way worse with parallel queries? |
This comment has been minimized.
This comment has been minimized.
|
@rust-timer build 89874d8 |
This comment has been minimized.
This comment has been minimized.
rust-timer
commented
Dec 24, 2018
|
Success: Queued 89874d8 with parent ddab10a, comparison URL. |
This comment has been minimized.
This comment has been minimized.
rust-timer
commented
Dec 24, 2018
|
Finished benchmarking try commit 89874d8 |
Zoxc commentedDec 13, 2018
Let's see if I have more luck
r? @michaelwoerister