Updatethrottle singleton#3792
Open
rjwills28 wants to merge 3 commits intoControlSystemStudio:masterfrom
Open
Conversation
so all updates across all windows come from a single thread
as we now only have a single UpdateThrottle and so do not need to name the instance based on how many there are.
|
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.



A little while we were looking at the performance of Phoebus when there are multiple windows open (see issue #3169), specifically that CPU usage was very high with multiple windows with PVs updating at 10Hz.
After some further investigations, including testing a standalone JavaFX application, we found that a lot of the bad performance stems from JavaFX itself and the way it handles repainting multiple windows (as also suggested by Kay in the above issue).
However, we continued to look for any ways that we could improve performance on the Phoebus side, which we address in this PR. Currently Phoebus creates an
RepresentationUpdateThrottleinstance for each window. This class is in charge of collecting updates and running them on the UI thread. This means that in the case of 10 windows, there are 10RepresentationUpdateThrottleall scheduling jobs on the single UI thread.Instead we found that it was slightly more efficient to turn the
RepresentationUpdateThrottleclass into a singleton and only have this one thread to collect updates across all windows and hence there is only one thread trying to run jobs on the UI thread. In testing, with this change, we saw CPU usage drop by about 10, e.g. from 95% to 85%.There is only one UI thread that can repaint the display and so I can't see any ill effects of only having one thread to collect updates and call this UI thread, but I would appreciate some further thoughts on this. Is there anything we haven't thought of that this might affect?
Checklist
Testing:
Documentation: