Skip to content

Updatethrottle singleton#3792

Open
rjwills28 wants to merge 3 commits intoControlSystemStudio:masterfrom
rjwills28:updatethrottle_singleton
Open

Updatethrottle singleton#3792
rjwills28 wants to merge 3 commits intoControlSystemStudio:masterfrom
rjwills28:updatethrottle_singleton

Conversation

@rjwills28
Copy link
Copy Markdown
Contributor

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 RepresentationUpdateThrottle instance 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 10 RepresentationUpdateThrottle all scheduling jobs on the single UI thread.

Instead we found that it was slightly more efficient to turn the RepresentationUpdateThrottle class 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:

    • The feature has automated tests
    • Tests were run
    • If not, explain how you tested your changes
  • Documentation:

    • The feature is documented
    • The documentation is up to date
    • Release notes:
      • Added an entry if the change is breaking or significant
      • Added an entry when adding a new feature

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.
@sonarqubecloud
Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant