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

Entities with many subclasses load super slowly #912

Closed
kevinschaper opened this issue Nov 25, 2024 · 7 comments · Fixed by #944
Closed

Entities with many subclasses load super slowly #912

kevinschaper opened this issue Nov 25, 2024 · 7 comments · Fixed by #944
Assignees

Comments

@kevinschaper
Copy link
Member

example: http://dev.monarchinitiative.org/PATO:0000001

I'm not sure if this is a backend problem (something about the hierarchy code is silly, like making one request per subclass entity), or if it's a rendering problem (something in the AppNodeBadge is expensive?).

If there's lots of associations, but few subclasses, everything seems fine, so it does seem to be specific to the hierarchy API request or UI components.

@ptgolden
Copy link
Member

It's a rendering issue. I'll check it out.

@ptgolden ptgolden self-assigned this Nov 27, 2024
@ptgolden
Copy link
Member

The biggest issue is (obviously) rendering all of the subclasses.

@ptgolden
Copy link
Member

ptgolden commented Dec 2, 2024

The bottleneck is happening in AppLink inside AppNodeBadge, in this component:

<router-link
v-else
:to="{
...routeTo,
state: mapValues(state, (value) => stringify(value)),
}"
:replace="replace"
>

@ptgolden
Copy link
Member

ptgolden commented Dec 2, 2024

The issue is that for every single AppLink, the breadcrumbs part of the state includes a reference to the (very large) entity with tons of subclasses, and it's getting stringified N times (where N is the number of rendered subclasses)

@ptgolden
Copy link
Member

ptgolden commented Dec 2, 2024

A further issue here is that the entire node is stored in the browser's history state. That can end up being quite a bit of data for nodes like this one.

The easiest solution here would be to only store a minimal amount of a node's information in the breadcrumb state. That is, the smallest representation of an entity that could be rendered by SectionBreadcrumbs. That component only renders the node as an AppNodeBadge, so that means having a representation that only includes the following elements:

  • id
  • category
  • name
  • label
  • in_taxon_label
  • info

@ptgolden
Copy link
Member

ptgolden commented Dec 2, 2024

Note that this is already done in the AssociationsSummary component:

export function getBreadcrumbs(
node: Node,
association: DirectionalAssociation,
side: "subject" | "object",
) {
const subject = {
id: association.subject,
name: association.subject_label,
category: association.subject_category,
in_taxon_label: association.subject_taxon_label,
};
const object = {
id: association.object,
name: association.object_label,
category: association.object_category,
in_taxon_label: association.object_taxon_label,
};

@ptgolden ptgolden added this to the 2024-12 Release milestone Dec 3, 2024
@sagehrke
Copy link
Member

sagehrke commented Jan 8, 2025

@ptgolden I am moving this to the January 2025 milestone. Please update it to a later date if you need more time.

ptgolden pushed a commit that referenced this issue Jan 21, 2025
Previously, if an AppLink was rendered from a node, the entire node was
serialized. Since a node is a non-trivially large JSON structure, this
resulted in a lot of wasted serialization, as only a few pieces of
information are necessary to generate a link. As a result, pages that
rendered a non-trival amount of links (in this case, SectionHierarchy)
would be very laggy as the browser struggled to do all that
serialization.

This commit filters only those properties necessary to render a link:
`id`, `category`, `name`, and `in_taxon_label`.

(Note that this stretegy is already used in AssociationSummary.vue).

Fixes #912
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 a pull request may close this issue.

3 participants