Add a rate limiter on the slicer's index #24
Merged
Conversation
This file contains 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
almarklein
force-pushed
the
ratelimit
branch
from
bdd6a75
to
e670173
Compare
|
It looks like there is a problem with the CI. |
almarklein
force-pushed
the
ratelimit
branch
from
a3b46b2
to
2a4af1c
Compare
|
Ready for review! |
14 tasks
emmanuelle
reviewed
Dec 1, 2020
Co-authored-by: Emmanuelle Gouillart <[email protected]>
This was referenced Dec 1, 2020
Closed
emmanuelle
reviewed
Dec 1, 2020
| ) | ||
| def upload_requested_slice(slice_index): | ||
| slice = img_array_to_uri(self._slice(slice_index)) | ||
| return {"index": slice_index, "slice": slice} | ||
|
|
||
| def _create_client_callbacks(self): | ||
| """Create the callbacks that run client-side.""" | ||
|
|
||
| # setpos (external) |
There was a problem hiding this comment.
This is some serious Ascii art :-)
There was a problem hiding this comment.
I'm eager for better ways to use diagrams to document code. In the mean time, ASCII art it is :)
There was a problem hiding this comment.
http://blockdiag.com/en/ maybe... (usable as a sphinx extension from a "sphinx documentation diagram" Google search :-), did not investigate more).
|
This looks great, feel free to merge when you want (I read the code quicky but I can read it in more details when it's merged as well, and I agree that this rate limitation mechanism is a good thing for performance). |
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.
Following up on #23, this PR improves the user experience by trying to keep the effect of a fast-changing index local to the current slicer. While dragging the slider, the current slicer is updated, but the other slicers (for the same scene_id) are not notified, nor will there be a request for new data to the server. When you stop dragging, the final value is "published", so the indicators update and the server provides the full-res data. This means all CPU cycles can be used to keep the current slicer responsive.
This is implemented using an Interval object that is turned on when the slider value is changed, and turned off when the value is published. The interval can be set to just 100ms so there is no perceived waiting time. Note that the behavior is pretty similar to the proposed approach in plotly/dash-core-components#884.
In this PR:
slider.valueis rate-limited (with a dcc.Interval) into a store called "index".reverse_y=False, and don't use it in our examples (for now).