grabbing image IDs instead of objects to generate group view

[erickmartins]

We (myself, @mellertd and @perlman) have noticed that, in our public server, the front Gallery page would take VERY long to load. So we decided to investigate. It seems that the code that generates that view grabs a list of every Image object belonging to a group, which is VERY slow. So I've put together a quick fix that grabs a list of every Image ID instead, and then only grabs the object for the Image that will be used as a preview.

I've run both bits of code (lines 67 to 96 of current code and equivalent bit on new code) against our public server.

Current code: Time elapsed: 14.50310492515564 groups object at the end: {'id': 53, 'name': 'Public', 'description': 'Public data', 'projectCount': 12, 'datasetCount': 913, 'imageCount': 25764, 'image': <_ImageWrapper id=163301>}

New code: Time elapsed: 0.163618803024292 groups object at the end: {'id': 53, 'name': 'Public', 'description': 'Public data', 'projectCount': 12, 'datasetCount': 913, 'imageCount': 25764, 'image': <_ImageWrapper id=163301>}

Same result, slightly faster 😄

[will-moore]

Looks great, thanks - added to daily build for testing...

[joshmoore]

The params that is passed to the projection has a limit of 1. It sounds like there's a bug somewhere if that leads to the loading of all images for a group.

[erickmartins]

I did not peek behind the curtain to see what's causing it; but it DOES take forever to load for our public server with a massive Public group, and we've tested it down to the line: it's that one object retrieval that takes ~15s to return, which does not happen when I explicitly retrieve a single object. I agree that it shouldn't happen, but the powers to see what's happening in that call are beyond me, so I made the fix that I could. 😃

[joshmoore]

@erickmartins the powers to see what's happening in that call are beyond me, so I made the fix that I could

Noted. @will-moore: can you reproduce the slow loading?

[will-moore]

Yes - Josh I can reproduce and found the bug in conn.getObjects() where the params limit was being ignored. Created an issue at https://github.com/ome/omero-py/issues/320 I'll open a fix for that...

[will-moore]

@erickmartins Just released omero-py 5.11.1 with the fix for this issue.

[sbesson]

@erickmartins Just released omero-py 5.11.1 with the fix for this issue.

What's the implication on this PR? Is a fix at the app level still necessary or is the upstream OMERO.py fix sufficient to address the performance issue?

[will-moore]

The omero-py bug that this revealed should mean that this PR isn't needed. Hopefully @erickmartins can confirm that's the case?

[erickmartins]

Yep - if the slow loading behavior is fixed by the omero-py release, this PR is not necessary any longer. Thanks everyone!

[perlman]

I rebuilt our omero-web + gallery container with the new omero-py and it appears to behave as expected.

Thanks for the fix!

[sbesson]

:+1: thanks all for the testing. The only thing I could thing as a replacement would be to have a patch release of omero-gallery with https://github.com/ome/omero-gallery/blob/master/setup.py#L68 updated to require omero-py>=5.11.1. This would guarantee upgraded deployment would also pull a recent OMERO.py version with the underlying performance fix. But the requirement is not really required in terms of API so happy to hear opposing arguments

[will-moore]

You'd need to tell all omero-gallery users to update omero-gallery, so that they get the latest omero-py. You could just tell them to update omero-py instead? But I guess a requirement of omero-py>=5.11.1 would also benefit new gallery users and gallery is kinda broken without this fix, so I wouldn't be against it.