[24.1] Paginate tabular datasets in tables to load invocation reports quicker

[ahmedhamidawan]

Tries to improve performance of invocations reports as reported in #19439

The b-table was crashing the browser tab for tabular datasets with many rows. Using b-pagination here to fix that. We were also making a call to fetch datatypes for each dataset in the invocation report. Now we use the store.
Viewing reports is faster now.

use_datatypes_mapper_composable_in_reports_2.mp4

@guerler 's upcoming work on reports in Overhaul Workflow Reports and Pages #19452 could refactor this so that the useDatatypesMapper composable is called from within the HistoryDatasetDetails instead of using the store like I did here?

Note: Targeting 24.1 because the invocation view was introduced then and users clicking on reports can get their whole tab frozen because of this...

How to test the changes?

(Select all options that apply)

• I've included appropriate automated tests.
• This is a refactoring of components with existing test coverage.
• Instructions for manual testing are as follows:
  1. [add testing steps and prerequisites here if you didn't write automated tests covering all your changes]

License

• I agree to license these and all my past contributions to the core galaxy codebase under the MIT license.

[mvdbeek]

Fixes #19439

definitely not :). The chrome tab runs out of memory, that's never gonna happen just for datatypes.

[mvdbeek]

That's a nice change, but it doesn't fix the bug. Can you target dev ?

[ahmedhamidawan]

@mvdbeek

definitely not :). The chrome tab runs out of memory, that's never gonna happen just for datatypes.

Hmm interesting, the performance is definitely improved as the tab doesn't freeze anymore.

That's a nice change, but it doesn't fix the bug. Can you target dev ?

The reason I wanted to target 24.1 or still even 24.2 was mentioned in the PR description:

Targeting 24.1 because the invocation view was introduced then and users clicking on reports can get their whole tab frozen because of this..

This improved the UX at least, even if it doesn't fix the bug entirely

[mvdbeek]

Have you tested this with https://test.galaxyproject.org/workflows/invocations/81483e7017833431 ?
Yes, we fetch datatypes_and_mapping 5 times, but that's done very quickly. It seems to me that rendering the preview for GCA_900093555.2_GCA_900093555.augustus.gtf.gz is the culprit. I can remove this and it loads immediately. I have a hard time seeing how this could improve the situation.

[ahmedhamidawan]

This is https://test.galaxyproject.org/workflows/invocations/81483e7017833431 on local dev server on this same branch:

use_datatypes_mapper_composable_in_reports.mp4

The page doesnt crash anymore

[mvdbeek]

Well, this is weird. I tried your branch with yarn run develop pointing against test, and yes, that fixes it. However, why does replacing the gtf.gz dataset with something else fix this as well ? It just seems really weird that making something that is a fast operation 5 times faster. That goes against my entire experience of optimizing code 😆

Anyway, you were right, we should target 24.1

[ahmedhamidawan]

I think the reason removing that fixes it is because that is the HistoryDatasetDisplay component that uses the datatypesMapper, and I've moved the mapper to the parent fixing this.

[mvdbeek]

OK, so one difference is that the multiqc report renders as text instead of html, which I think is a bug. I think we're getting closer to the reason why this is so much faster .. it might be that the composable only fetches the datatypes that can be uploaded, which is a smaller subset. Then we make different decisions based on what we determine the datatype to be.

[mvdbeek]

diff --git a/client/src/components/Markdown/Elements/HistoryDatasetDisplay.vue b/client/src/components/Markdown/Elements/HistoryDatasetDisplay.vue
index e9cae99042..9f324a459f 100644
--- a/client/src/components/Markdown/Elements/HistoryDatasetDisplay.vue
+++ b/client/src/components/Markdown/Elements/HistoryDatasetDisplay.vue
@@ -60,15 +60,15 @@
                 <div v-else-if="error">{{ error }}</div>
                 <div v-else :class="contentClass">
                     <b-embed
-                        v-if="datatypesMapper?.isSubTypeOfAny(datasetType, ['pdf', 'html'])"
+                        v-if="datatypesMapper.isSubTypeOfAny(datasetType, ['pdf', 'html'])"
                         type="iframe"
                         aspect="16by9"
                         :src="displayUrl" />
                     <HistoryDatasetAsImage
-                        v-else-if="datatypesMapper?.isSubTypeOfAny(datasetType, ['galaxy.datatypes.images.Image'])"
+                        v-else-if="datatypesMapper.isSubTypeOfAny(datasetType, ['galaxy.datatypes.images.Image'])"
                         :args="args" />
                     <div v-else-if="itemContent.item_data">
-                        <div v-if="datatypesMapper?.isSubTypeOfAny(datasetType, ['tabular'])">
+                        <div v-if="datatypesMapper.isSubTypeOfAny(datasetType, ['tabular'])">
                             <UrlDataProvider
                                 v-slot="{ result: metaData, loading: metaLoading, error: metaError }"
                                 :url="metaUrl">
diff --git a/client/src/components/Markdown/MarkdownContainer.vue b/client/src/components/Markdown/MarkdownContainer.vue
index 7d9238824f..764203c8a8 100644
--- a/client/src/components/Markdown/MarkdownContainer.vue
+++ b/client/src/components/Markdown/MarkdownContainer.vue
@@ -179,7 +179,9 @@ function argToBoolean(args, name, booleanDefault) {
                 v-else-if="['history_dataset_embedded', 'history_dataset_display'].includes(name)"
                 :args="args"
                 :datasets="datasets"
-                :embedded="name == 'history_dataset_embedded'" />
+                :embedded="name == 'history_dataset_embedded'"
+                :datatypes-mapper="datatypesMapper"
+                :datatypes-loading="datatypesMapperLoading" />
             <HistoryDatasetDetails
                 v-else-if="
                     [

that'll cause this to crash as it did before.

[mvdbeek]

https://github.com/ahmedhamidawan/galaxy/blob/use_datatypes_mapper_composable_in_reports/client/src/components/Markdown/Elements/HistoryDatasetDisplay.vue#L77-L82 might not be performant enough to render large tables. It might help to remove reactivity with Object.freeze ?

[mvdbeek]

might not be performant enough to render large tables. I

I think it's actually b-table that can't deal with so many rows. slicing to 200 is an easy fix, but maybe we need a more performant table component.

[mvdbeek]

#19322 could be an option

[ahmedhamidawan]

Should we Object.freeze for this bugfix then?

And then as @guerler refactors and modernizes reports etc, we can replace the BTable with the AG Grid thing?

[ahmedhamidawan]

Actually, the datatypesMapper is never being used in this component now (it is always undefined/null).

When I use the composable directly in the component instead of passing it as a prop, we have the same bug again of freezing for some time, until we actually show the table.

You are right, the table is the culprit

[mvdbeek]

Actually, the datatypesMapper is never being used in this component now (it is always undefined/null).

See #19470 (comment) which I think you should apply

the table is the culprit

b-table has pagination (https://bootstrap-vue.org/docs/components/table#pagination), maybe that could be a fix that won't be too intrusive ?

[ahmedhamidawan]

I rebased it to just using b-pagination for the b-table, and the datatypesMapperStore in the singular HistoryDatasetDisplay component (using the composable in the setup of this options API component was creating weird problems with the datatypesMapperLoading ref)

use_datatypes_mapper_composable_in_reports_2.mp4

[mvdbeek]

Thanks @ahmedhamidawan, let's get this in.

[mvdbeek]

Deployed on usegalaxy.org, working nicely!