Skip to content

Stream StudyCachables in QueryHelper - #7896

Merged
labkey-adam merged 3 commits into
developfrom
fb_stream_study_cachables
Jul 30, 2026
Merged

Stream StudyCachables in QueryHelper#7896
labkey-adam merged 3 commits into
developfrom
fb_stream_study_cachables

Conversation

@labkey-adam

@labkey-adam labkey-adam commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Rationale

QueryHelper() retrieved objects from the database via a cached stream (backed by a list), copied those objects into another list, and then enumerated the second list to create the map that it actually caches. This meant it (temporarily) held three in-memory copies of the collection at initialization time. Large studies (e.g., many study visits) also produced a warning at retrieval time. This switches to retrieving the objects via an uncached stream, and passing that stream through to the map-creation code path, eliminating the warning and two collection copies. It's still caching a (potentially large) map, but that's an acceptable trade-off for commonly used objects.

@labkey-adam labkey-adam self-assigned this Jul 30, 2026

@labkey-jeckels labkey-jeckels left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Very minor suggestions

private final List<DatasetDefinition> _nullCohortDatasets;

private DatasetCollections(Collection<DatasetDefinition> collection)
private DatasetCollections(Map<Integer, DatasetDefinition> stream)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
private DatasetCollections(Map<Integer, DatasetDefinition> stream)
private DatasetCollections(Map<Integer, DatasetDefinition> map)

private DatasetCollections(Map<Integer, DatasetDefinition> stream)
{
super(collection);
super(stream);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
super(stream);
super(map);

Comment on lines +59 to +65
try (Stream<T> stream = getTableSelector(key).uncachedStream(_objectClass))
{
return createCollections(Collections.unmodifiableMap(stream
.peek(StudyCachable::lock)
.toList()
));
.collect(LabKeyCollectors.toLinkedMap(StudyCachable::getPrimaryKey, v -> v))
));
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Marginal benefit from closing the stream/ResultSet prior to creating the collections.

Suggested change
try (Stream<T> stream = getTableSelector(key).uncachedStream(_objectClass))
{
return createCollections(Collections.unmodifiableMap(stream
.peek(StudyCachable::lock)
.toList()
));
.collect(LabKeyCollectors.toLinkedMap(StudyCachable::getPrimaryKey, v -> v))
));
}
Map<K, T> map;
try (Stream<T> stream = getTableSelector(key).uncachedStream(_objectClass))
{
map = stream.peek(StudyCachable::lock)
.collect(LabKeyCollectors.toLinkedMap(StudyCachable::getPrimaryKey, v -> v))
));
}
return createCollections(Collections.unmodifiableMap(map));

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, that's exactly what I intended, but not what my fingers typed. Should be fixed up now.

@labkey-adam
labkey-adam merged commit 24ca557 into develop Jul 30, 2026
8 of 9 checks passed
@labkey-adam
labkey-adam deleted the fb_stream_study_cachables branch July 30, 2026 19:48
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.

2 participants