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

Right panel performance fixes #6075

Closed

Conversation

will-moore
Copy link
Member

@will-moore will-moore commented Jul 3, 2019

See https://trello.com/c/vv4mqsAi/5752-omero-omeroweb-on-demo-is-slow
This PR improves the performance of the right-hand metadata panel, particularly for Images and noticeable with large numbers of users in the current group.

Calling conn.getObjects() or conn.getObject() with group context -1 is slow with large numbers of users in the group. We could use the current group of the webclient (session['active_group']) to set the group context, but it could result in 404 if user has e.g webclient open in 2 browser tabs with a different group in each etc.
Instead, this PR reduces the number of times this is called to 1, instead of 3. So it should be 3 x faster.
Also, to avoid the risk of any additional calls to getObject() being added in the future, we set the group context once we have loaded the Object and know the group.

For example, using {{ ome.user.id }} in a template was causing getObject("Experimenter", id) to be called. Setting the context improved the speed of this, although I also replaced this with {{ ome.user_id }} which removes the call entirely.

To test functionality (not broken):

  • check that the right panel still loads and displays metadata for P/D/I etc (not broken)
  • Try adding comments etc.
  • check that right panel still works for Shares, adding Comment to the Share.

To test performance (learning server with large number of users):

I guess if we want to quantify speed improvements, we could time the right panel loading with and without this PR merged?

cc @mtbc, @pwalczysko

Seems that session['user_id'] can get out of sync with current userId
The decorator sets 'user' to be conn.getUser function and {{ ome.user.id }} calls this to get Experimenter
and then to get the ID from the Experimenter. Instead we can get the user_id directly.
@rgozim
Copy link
Member

rgozim commented Jul 9, 2019

Provided the number of users is similar to what was on demo back in 2018, then this looks like the performance is better. Previously it was taking longer than 10 seconds to view content on the right hand panel, and although not instantaneous, it is substantially quicker with this fix/"quick win".

@rgozim
Copy link
Member

rgozim commented Jul 9, 2019

On a side note, clicking on the Acquisition tab in the right hand menu is a bit slow (not terribly so), and maybe could be do with being added as fix to this PR - if that is something that is also affected by the "group id -1" issue.

@mtbc
Copy link
Member

mtbc commented Jul 10, 2019

Good to know that OMERO 5.5.2 should be a further improvement for Paul then!

@joshmoore joshmoore added exclude transfer Migrate to another repository labels Aug 20, 2019
@will-moore
Copy link
Member Author

Ported to ome/omero-web#17

@will-moore will-moore closed this Sep 18, 2019
@will-moore will-moore reopened this Sep 15, 2022
@will-moore will-moore closed this Sep 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exclude transfer Migrate to another repository
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants