Skip to content

Commit faab1e3

Browse files
mantheymanthey
committed
Merge branch 'master' into girder-5
2 parents 47f28e2 + 615f669 commit faab1e3

File tree

7 files changed

+173
-49
lines changed

7 files changed

+173
-49
lines changed

‎CHANGELOG.md‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,14 @@
55
### Improvements
66

77
- Speed up annotation centroid queries ([#1981](../../pull/1981))
8+
- Improve large annotation load and display speed ([#1982](../../pull/1982))
9+
- Denormalize some values in the annotation collection to support improving display speed ([#1984](../../pull/1984))
10+
- Improve indices for annotationelement queries ([#1985](../../pull/1985))
11+
- Use some raw bson handling to speed up annotationelement serialization ([#1986](../../pull/1986))
12+
13+
### Bug Fixes
14+
15+
- Add a guard to avoid a javascript except if annotations are not loaded enough ([#1983](../../pull/1983))
816

917
## 1.33.0
1018

‎girder_annotation/girder_large_image_annotation/models/annotation.py‎

Lines changed: 54 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -658,14 +658,18 @@ def initialize(self):
658658
], {}),
659659
'_version',
660660
'updated',
661+
([
662+
('_active', SortDir.ASCENDING),
663+
('_elementCount', SortDir.ASCENDING),
664+
], {}),
661665
])
662666
self.ensureTextIndex({
663667
'annotation.name': 10,
664668
'annotation.description': 1,
665669
})
666670

667671
self.exposeFields(AccessType.READ, (
668-
'annotation', '_version', '_elementQuery', '_active',
672+
'annotation', '_version', '_elementQuery', '_active', '_elementCount', '_detailsCount',
669673
) + self.baseFields)
670674
events.bind('model.item.remove', 'large_image_annotation', self._onItemRemove)
671675
events.bind('model.item.copy.prepare', 'large_image_annotation', self._prepareCopyItem)
@@ -753,6 +757,47 @@ def _migrateDatabase(self):
753757
# Check that all annotations have groups
754758
for annotation in self.collection.find({'groups': {'$exists': False}}):
755759
self.injectAnnotationGroupSet(annotation)
760+
threading.Thread(target=self._migrateDatabaseBackground, daemon=True).start()
761+
762+
def _migrateDatabaseBackground(self):
763+
needed = self.find({
764+
'_active': {'$ne': False},
765+
'$or': [
766+
{'_elementCount': {'$exists': False}},
767+
{'_detailsCount': {'$exists': False}},
768+
],
769+
})
770+
count = needed.count()
771+
if count:
772+
logger.info(f'Adding {count} count/details record(s) to existing annotations')
773+
lastlog = 0
774+
for idx, annot in enumerate(needed):
775+
try:
776+
entry = next(Annotationelement().collection.aggregate([{
777+
'$match': {'_version': annot['_version']},
778+
}, {
779+
'$group': {
780+
'_id': None,
781+
'_elementCount': {'$sum': 1},
782+
'_detailsCount': {'$sum': {'$ifNull': ['$bbox.details', 1]}},
783+
},
784+
}]))
785+
except StopIteration:
786+
entry = {'_elementCount': 0, '_detailsCount': 0}
787+
if time.time() - lastlog > 10:
788+
logger.info(
789+
f'Adding {idx}/{count} count/detail record for '
790+
f'{str(annot["_id"])}, version {annot["_version"]}, '
791+
f'count {entry["_elementCount"]}, details '
792+
f'{entry["_detailsCount"]}')
793+
lastlog = time.time()
794+
self.collection.update_one(
795+
{'_id': annot['_id']},
796+
{'$set': {
797+
'_elementCount': entry['_elementCount'],
798+
'_detailsCount': entry['_detailsCount'],
799+
}},
800+
)
756801

757802
def _migrateACL(self, annotation):
758803
"""
@@ -948,14 +993,17 @@ def save(self, annotation, *args, **kwargs): # noqa
948993
annotation['annotation']['name'] = now.strftime('Annotation %Y-%m-%d %H:%M')
949994

950995
def replaceElements(query, doc, *args, **kwargs):
951-
Annotationelement().updateElements(doc)
996+
elementCount, detailsCount = Annotationelement().updateElements(doc)
952997
elements = doc['annotation'].pop('elements', None)
953998
if self._historyEnabled:
954999
oldAnnotation = self.collection.find_one(query)
9551000
if oldAnnotation:
9561001
oldAnnotation['_annotationId'] = oldAnnotation.pop('_id')
9571002
oldAnnotation['_active'] = False
9581003
insert_one(oldAnnotation)
1004+
if elements is not None:
1005+
doc['_elementCount'] = elementCount
1006+
doc['_detailsCount'] = detailsCount
9591007
ret = replace_one(query, doc, *args, **kwargs)
9601008
if elements:
9611009
doc['annotation']['elements'] = elements
@@ -968,10 +1016,13 @@ def insertElements(doc, *args, **kwargs):
9681016
# the annotation without elements, then restore the elements.
9691017
doc.setdefault('_id', ObjectId())
9701018
if doc['annotation'].get('elements') is not None:
971-
Annotationelement().updateElements(doc)
1019+
elementCount, detailsCount = Annotationelement().updateElements(doc)
9721020
# If we are inserting, we shouldn't have any old elements, so don't
9731021
# bother removing them.
9741022
elements = doc['annotation'].pop('elements', None)
1023+
if elements is not None:
1024+
doc['_elementCount'] = elementCount
1025+
doc['_detailsCount'] = detailsCount
9751026
ret = insert_one(doc, *args, **kwargs)
9761027
if elements is not None:
9771028
doc['annotation']['elements'] = elements

‎girder_annotation/girder_large_image_annotation/models/annotationelement.py‎

Lines changed: 66 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@
2323
import threading
2424
import time
2525

26+
import bson.codec_options
27+
import bson.raw_bson
2628
import pymongo
2729
from girder_large_image.models.image_item import ImageItem
2830

@@ -85,6 +87,20 @@ def initialize(self):
8587
('_version', SortDir.DESCENDING),
8688
('bbox.size', SortDir.DESCENDING),
8789
], {}),
90+
([
91+
('annotationId', SortDir.ASCENDING),
92+
('_version', SortDir.DESCENDING),
93+
('bbox.lowx', SortDir.ASCENDING),
94+
('bbox.highx', SortDir.ASCENDING),
95+
('bbox.size', SortDir.DESCENDING),
96+
], {}),
97+
([
98+
('annotationId', SortDir.ASCENDING),
99+
('_version', SortDir.DESCENDING),
100+
('bbox.lowy', SortDir.ASCENDING),
101+
('bbox.highy', SortDir.ASCENDING),
102+
('bbox.size', SortDir.DESCENDING),
103+
], {}),
88104
])
89105

90106
self.exposeFields(AccessType.READ, (
@@ -225,12 +241,14 @@ def yieldElements(self, annotation, region=None, info=None, bbox=False): # noqa
225241
'annotationId': annotation.get('_annotationId', annotation['_id']),
226242
'_version': annotation['_version'],
227243
}
244+
includeCount = True
228245
for key in region:
229246
if key in self.bboxKeys and self.bboxKeys[key][1]:
230247
if self.bboxKeys[key][1] == '$gte' and float(region[key]) <= 0:
231248
continue
232249
query[self.bboxKeys[key][0]] = {
233250
self.bboxKeys[key][1]: float(region[key])}
251+
includeCount = False
234252
if region.get('sort') in self.bboxKeys:
235253
sortkey = self.bboxKeys[region['sort']][0]
236254
else:
@@ -242,8 +260,11 @@ def yieldElements(self, annotation, region=None, info=None, bbox=False): # noqa
242260
queryLimit = max(minElements, maxDetails) if maxDetails and (
243261
not limit or max(minElements, maxDetails) < limit) else limit
244262
offset = int(region['offset']) if region.get('offset') else 0
245-
fields = {'_id': True, 'element': True, 'bbox.details': True, 'datafile': True}
246263
centroids = str(region.get('centroids')).lower() == 'true'
264+
# Specifying a limit helps mongo choose a better index
265+
if maxDetails:
266+
queryLimit = max(maxDetails, minElements) if queryLimit is None else min(
267+
queryLimit, max(maxDetails, minElements))
247268
if centroids:
248269
fields = {
249270
'_id': True,
@@ -269,20 +290,43 @@ def yieldElements(self, annotation, region=None, info=None, bbox=False): # noqa
269290
info['centroids'] = True
270291
info['props'] = proplist
271292
info['propskeys'] = propskeys
272-
elif region.get('bbox') or bbox:
273-
fields.pop('bbox.details', None)
274-
fields['bbox'] = True
293+
else:
294+
# Note that it is faster to get all of bbox rather than just
295+
# bbox.details (this is not true for centroids)
296+
fields = {'_id': True, 'element': True, 'bbox': True, 'datafile': True}
275297
logger.debug('element query %r (%r) for %r', query, fields, region)
276-
elementCursor = self.find(
277-
query=query, sort=[(sortkey, sortdir)], limit=queryLimit,
278-
offset=offset, fields=fields)
279-
298+
if centroids:
299+
elementCursor = self.find(
300+
query=query, sort=[(sortkey, sortdir)], limit=queryLimit,
301+
offset=offset, fields=fields)
302+
else:
303+
# By using raw bson to some extent, we save some decoding time
304+
# from bson to python. It isn't clear to me why this reduces
305+
# decoding time by 25% or more, but it seems consistent. When
306+
# applied to the centoids, this was actually much slower.
307+
308+
class SemiRawDocument(bson.raw_bson.RawBSONDocument):
309+
def __getitem__(self, key):
310+
if key in {'element', 'bbox'}:
311+
if hasattr(self, key):
312+
return getattr(self, key)
313+
val = {k: v if not isinstance(v, bson.raw_bson.RawBSONDocument) else
314+
bson.decode(v.raw) for k, v in super().__getitem__(key).items()}
315+
setattr(self, key, val)
316+
return val
317+
return super().__getitem__(key)
318+
319+
elementCursor = self.collection.with_options(
320+
codec_options=bson.codec_options.CodecOptions(document_class=SemiRawDocument)).find(
321+
filter=query, sort=[(sortkey, sortdir)], limit=queryLimit,
322+
skip=offset, projection=fields)
280323
info.update({
281-
'count': elementCursor.count(),
282324
'offset': offset,
283325
'filter': query,
284326
'sort': [sortkey, sortdir],
285327
})
328+
if includeCount:
329+
info['count'] = elementCursor.count()
286330
details = count = 0
287331
if maxDetails:
288332
info['maxDetails'] = maxDetails
@@ -627,6 +671,8 @@ def updateElementChunk(self, elements, chunk, chunkSize, annotation, now, insert
627671
len(entries), time.time() - chunkStartTime, prepTime,
628672
chunk + len(entries), len(elements)))
629673
lastTime = time.time()
674+
return len(entries), sum(
675+
el.get('bbox', {}).get('details') or 1 for el in entries)
630676
except Exception:
631677
logger.exception('Failed to update element chunk')
632678
raise
@@ -641,18 +687,26 @@ def updateElements(self, annotation):
641687
startTime = time.time()
642688
elements = annotation['annotation'].get('elements', [])
643689
if not len(elements):
644-
return
690+
return 0, 0
645691
now = datetime.datetime.now(datetime.timezone.utc)
646692
threads = large_image.config.cpu_count()
647693
chunkSize = int(max(100000 // threads, 10000))
648694
insertLock = threading.Lock()
695+
count, details = 0, 0
649696
with concurrent.futures.ThreadPoolExecutor(max_workers=threads) as pool:
697+
futures = []
650698
for chunk in range(0, len(elements), chunkSize):
651-
pool.submit(self.updateElementChunk, elements, chunk,
652-
chunkSize, annotation, now, insertLock)
699+
futures.append(pool.submit(
700+
self.updateElementChunk, elements, chunk, chunkSize,
701+
annotation, now, insertLock))
702+
for future in concurrent.futures.as_completed(futures):
703+
chunkCount, chunkDetails = future.result()
704+
count += chunkCount
705+
details += chunkDetails
653706
if time.time() - startTime > 10:
654707
logger.info('inserted %d elements in %4.2fs' % (
655708
len(elements), time.time() - startTime))
709+
return count, details
656710

657711
def getElementGroupSet(self, annotation):
658712
query = {

‎girder_annotation/girder_large_image_annotation/rest/annotation.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -117,8 +117,8 @@ def find(self, params):
117117
fields = list(
118118
(
119119
'annotation.name', 'annotation.description',
120-
'annotation.attributes', 'annotation.display',
121-
'access', 'groups', '_version',
120+
'annotation.attributes', 'annotation.display', 'access',
121+
'groups', '_version', '_elementCount', '_detailsCount',
122122
) + Annotation().baseFields)
123123
return Annotation().findWithPermissions(
124124
query, sort=sort, fields=fields, user=self.getCurrentUser(),

‎girder_annotation/girder_large_image_annotation/web_client/models/AnnotationModel.js‎

Lines changed: 38 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -292,6 +292,27 @@ const AnnotationModel = AccessControlledModel.extend({
292292
});
293293
},
294294

295+
fetchCentroidsWrapper: function (opts) {
296+
this._inFetch = 'centroids';
297+
return this.fetchCentroids().then(() => {
298+
this._inFetch = true;
299+
if (opts.extraPath) {
300+
this.trigger('g:fetched.' + opts.extraPath);
301+
} else {
302+
this.trigger('g:fetched');
303+
}
304+
return null;
305+
}).always(() => {
306+
this._inFetch = false;
307+
if (this._nextFetch) {
308+
var nextFetch = this._nextFetch;
309+
this._nextFetch = null;
310+
nextFetch();
311+
}
312+
return null;
313+
});
314+
},
315+
295316
/**
296317
* Fetch a single resource from the server. Triggers g:fetched on success,
297318
* or g:error on error.
@@ -317,13 +338,28 @@ const AnnotationModel = AccessControlledModel.extend({
317338
if (opts.ignoreError) {
318339
restOpts.error = null;
319340
}
341+
if (this._pageElements === undefined && (this.get('_elementCount') || 0) > this.get('minElements') && (this.get('_detailsCount') || 0) > this.get('maxDetails')) {
342+
this._pageElements = true;
343+
return this.fetchCentroidsWrapper(opts);
344+
}
320345
this._inFetch = true;
321346
if (this._refresh) {
322347
delete this._pageElements;
323348
delete this._centroids;
324349
this._refresh = false;
325350
}
326-
return restRequest(restOpts).done((resp) => {
351+
return restRequest(restOpts).always(() => {
352+
if (this._inFetch !== 'centroids') {
353+
this._inFetch = false;
354+
if (this._nextFetch) {
355+
var nextFetch = this._nextFetch;
356+
this._nextFetch = null;
357+
if (this._pageElements !== false) {
358+
nextFetch();
359+
}
360+
}
361+
}
362+
}).done((resp) => {
327363
const annotation = resp.annotation || {};
328364
const elements = annotation.elements || [];
329365

@@ -333,24 +369,7 @@ const AnnotationModel = AccessControlledModel.extend({
333369
if (this._pageElements === undefined && resp._elementQuery) {
334370
this._pageElements = resp._elementQuery.count > resp._elementQuery.returned;
335371
if (this._pageElements) {
336-
this._inFetch = 'centroids';
337-
this.fetchCentroids().then(() => {
338-
this._inFetch = true;
339-
if (opts.extraPath) {
340-
this.trigger('g:fetched.' + opts.extraPath);
341-
} else {
342-
this.trigger('g:fetched');
343-
}
344-
return null;
345-
}).always(() => {
346-
this._inFetch = false;
347-
if (this._nextFetch) {
348-
var nextFetch = this._nextFetch;
349-
this._nextFetch = null;
350-
nextFetch();
351-
}
352-
return null;
353-
});
372+
this.fetchCentroidsWrapper(opts);
354373
} else {
355374
this._nextFetch = null;
356375
}
@@ -367,17 +386,6 @@ const AnnotationModel = AccessControlledModel.extend({
367386
this._fromFetch = null;
368387
}).fail((err) => {
369388
this.trigger('g:error', err);
370-
}).always(() => {
371-
if (this._inFetch !== 'centroids') {
372-
this._inFetch = false;
373-
if (this._nextFetch) {
374-
var nextFetch = this._nextFetch;
375-
this._nextFetch = null;
376-
if (this._pageElements !== false) {
377-
nextFetch();
378-
}
379-
}
380-
}
381389
});
382390
},
383391

‎girder_annotation/girder_large_image_annotation/web_client/views/imageViewerWidget/geojs.js‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -503,6 +503,9 @@ var GeojsImageViewerWidgetExtension = function (viewer) {
503503
if (annotation._centroids && centroidFeature) {
504504
if (centroidFeature.verticesPerFeature) {
505505
this.viewer.scheduleAnimationFrame(() => {
506+
if (!annotation._shownIds) {
507+
return;
508+
}
506509
const centroidFeature = featureList.find((f) => f._centroidFeature);
507510
const count = centroidFeature.data().length;
508511
const shown = new Float32Array(count);

‎large_image/tilesource/base.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1257,8 +1257,8 @@ def _outputTile(
12571257
else:
12581258
color = PIL.ImageColor.getcolor(self.edge, mode)
12591259
tile = tile.copy()
1260-
tile[:, contentWidth:] = color
1261-
tile[contentHeight:] = color
1260+
cast(np.ndarray, tile)[:, contentWidth:] = color
1261+
cast(np.ndarray, tile)[contentHeight:] = color
12621262
if isinstance(tile, np.ndarray) and numpyAllowed:
12631263
return tile
12641264
tile = _imageToPIL(tile)

0 commit comments

Comments
 (0)