Share one visible-vertex computation and scope canvas icons to drawn types - #2120
Closed
kmcginnes wants to merge 1 commit into
Closed
Share one visible-vertex computation and scope canvas icons to drawn types#2120kmcginnes wants to merge 1 commit into
kmcginnes wants to merge 1 commit into
Conversation
…types `useRenderedEdges` called `useRenderedVertices()` while `GraphViewer` also called it directly, so the whole vertex pipeline — icon resolution included — ran twice per render. The filter predicate is now a derived atom both pipelines read, so the store computes it once. Canvas style and icon resolution is scoped to the vertex types actually drawn rather than every type in the schema: the stress schema carries 10,044 vertex types to draw 3, and resolving all of them each render was the dominant app-side render cost. The schema view keeps `useAllVertexStyles`, since drawing every type is its job. Measured over matched 10s windows on a live 10k-type schema, expanding a node: `useBackgroundImageMap` self time 59.7ms -> ~29ms, style work in `renderedEntities` 5.2ms -> ~0.7ms. Total busy time and INP were within run-to-run noise; the dominant expansion cost remains cytoscape rendering and the fcose layout, which this does not touch.
Collaborator
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Two related fixes to how much work a render does.
The vertex pipeline ran twice per render.
GraphViewercallsuseRenderedVertices()directly anduseRenderedEdges(), which calleduseRenderedVertices()internally to build its set of valid endpoints. Every hook has its own render-scoped memo, so the whole pipeline — icon resolution included — ran once per call site. The filter predicate is now a derived atom both pipelines read, so the store computes it once.Canvas style and icon resolution covered every type in the schema.
useBackgroundImageMap(useAllVertexStyles())iterated all of them on every render. On the stress schema that is 10,044 vertex types resolved to draw 3.canvasVertexStylesAtomscopes it to the types actually on the canvas. The schema view keepsuseAllVertexStyles, since drawing every type is its job.Measurement
Chrome DevTools against a live 10k-type Neptune schema, expanding a 244-neighbour airport with limit 75 (76 nodes / 150 edges). Matched 10s windows from the click, ~81k samples each:
useBackgroundImageMaprenderedEntitiesThe 59.7 → 29ms halving is the double-call fix, and matches the predicted 50% exactly. The drop to 0.0ms is the scoping — the function no longer appears in the profile at all.
What this does not fix. Total busy JS and INP were within run-to-run noise, and the dominant expansion cost remains cytoscape rendering plus the fcose layout (~390-470ms long task), which this does not touch. This removes schema-scale style work from the render path; it is not a fix for the reported expansion choppiness.
How to read
renderedEntities.ts—visibleVertexIdsAtomandcanvasVertexStylesAtom.displayVertex.ts— one selector exported so the atoms can read it.Note
The comment on
visibleVertexIdsAtomrecords that it still recomputes on a style change, becausedisplayVerticesInCanvasSelectorresolves display labels through the style atoms. Decoupling that is tracked in #2116.Stack
Both atoms are fused into one in
fuse-canvas-vertex-passeslater in this stack.