feat: support running measurements in parallel#848
Draft
bassosimone wants to merge 2 commits intomasterfrom
Draft
feat: support running measurements in parallel#848bassosimone wants to merge 2 commits intomasterfrom
bassosimone wants to merge 2 commits intomasterfrom
Conversation
This experimental diff stems from a discussion I had this morning with @hellais, where this functionality seems helpful to him. The diff itself breaks a bunch of places in the codebase, hence it's not suitable for merging as is, but required more work, and in particular, adapting tests and making sure we're not breaking any of the supported use cases. As an overview, the core of this diff consists of making sure that the InputProcessor can run measurements in parallel. In turn, this requires using a service for submitting and a service for writing measurements back to disk. These two services should be chained: saving to disk happens _after_ submitting such that we write on disk a measurement with the report ID we got after we submitted the measurement to the backend. There is a queue between the measurer and the submitter with a buffer of four places. I have experimentally determined that a larger than this number would cause the code to really dishonour the maximum runtime. In general, measuring could be faster than submitting thus creating a large queue would mean a very long endgame condition where we're busy submitting the queued measurements. FWIW, if we're moving on with this approach, it would be quite interesting to submit a compressed batch of measurements. There are also some open questions about how it's best to report errors, given that now the code runs asynchronously. We'll see.
It's important to first wait for the submitted to have finished consuming it's queue before stopping the saver queue, otherwise we will miss saving the last measurement to disk.
Contributor
Author
|
We saw this for a previous version of this code (b2f8b47). I think the way in which I am using |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This experimental diff stems from a discussion I had this morning
with @hellais, where this functionality seems helpful to him.
The diff itself breaks a bunch of places in the codebase, hence
it's not suitable for merging as is, but required more work, and
in particular, adapting tests and making sure we're not breaking
any of the supported use cases.
As an overview, the core of this diff consists of making sure
that the InputProcessor can run measurements in parallel.
In turn, this requires using a service for submitting and a service
for writing measurements back to disk. These two services should
be chained: saving to disk happens after submitting such that we
write on disk a measurement with the report ID we got after we
submitted the measurement to the backend.
There is a queue between the measurer and the submitter with a buffer
of four places. I have experimentally determined that a larger than
this number would cause the code to really dishonour the maximum
runtime. In general, measuring could be faster than submitting thus
creating a large queue would mean a very long endgame condition
where we're busy submitting the queued measurements.
FWIW, if we're moving on with this approach, it would be quite
interesting to submit a compressed batch of measurements.
There are also some open questions about how it's best to report
errors, given that now the code runs asynchronously. We'll see.
Checklist
Description
Please, insert here a more detailed description.