Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reflectometry performance #1458

Merged
merged 9 commits into from
Aug 8, 2022
Merged

Reflectometry performance #1458

merged 9 commits into from
Aug 8, 2022

Conversation

Tom-Willemsen
Copy link
Contributor

@Tom-Willemsen Tom-Willemsen commented Aug 3, 2022

Description of work

Improve the performance of the reflectometry OPI in terms of memory & cpu for rules, and also in terms of not sleeping on the UI thread from OPI scripts.

Note that I don't think the original issue was actually a real memory leak at all - rather, the reflectometry OPI was taking up a very large amount of space (400MB) - INTER had two references to it (refl perspective and synoptic) while POLREF had three references to it (refl perspective, synoptic, device screen).

We have made changes elsewhere so that refl front panel can no longer be opened as a synoptic/device screen. The changes in this PR bring the REFL OPI resource consumption down to more reasonable levels. Combined, these changes should be sufficient to ensure that the excessive memory consumption seen on INTER/POLREF is fixed.

Ticket

ISISComputingGroup/IBEX#7212

Acceptance criteria

On master:

  • Open a fresh client
  • Open VisualVM and connect it to the running client
  • Wait for the client resource usage to stabilize (e.g. wait about 60s after start)
  • Switch to INTER or another reflectometer of your choice
  • Wait for the client resource usage to stabilize (e.g. wait about 60s after instrument switch)
  • Press "Perform GC" in VisualVM and note the "used" memory just after the GC
    • This is the "baseline" memory
  • Switch to reflectometry perspective, and click at least once on each tab to activate the OPIs
  • Press "Perform GC" in VisualVM and note the "used" memory just after the GC
  • Subtract the baseline memory from the memory you just measured to get a number for the memory used by the REFL perspective.

Then:

  • Perform the same steps on this branch. Verify that this branch has a significantly lower memory usage from the reflectometry perspective
  • Verify that the rules are functionally the same on master and on this branch
  • Verify that the new manual tests added in rules_test.opi pass on this branch
  • Verify that the changed functionality in populate_list.py still works

System tests

  • Manual system test for OPI rules added. OPI rules are very difficult to test in an automated way.

Documentation

wiki

Code Review

Final Steps

@@ -23,7 +23,7 @@
<repository location="http://download.eclipse.org/releases/2018-09"/>
</location>
<location includeAllPlatforms="false" includeConfigurePhase="false" includeMode="planner" includeSource="true" type="InstallableUnit">
<repository location="http://shadow.nd.rl.ac.uk/ICP_P2/css_gui_dependencies/cs-studio/p2repo/"/>
<repository location="http://shadow.nd.rl.ac.uk/ICP_P2/css_gui_dependencies_7212/p2repo/"/>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note for reviewer: once you are happy with this PR, please:

  • Go to ICP_P2
  • Move css_gui_dependencies to css_gui_dependencies_old
  • Move css_gui_dependencies_7212 to css_gui_dependencies
  • Update this line to point at gui_dependencies rather than gui_dependencies_7212

@LowriJenkins LowriJenkins merged commit 28ff0af into master Aug 8, 2022
@LowriJenkins LowriJenkins deleted the reflectometry_performance branch August 11, 2022 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants