perf(shapes): drop the per-render table join when coloring by a table column#712
Merged
Conversation
… column _render_shapes joined the element to its annotating table via _join_table_for_element, whose table[indices, :].copy() does an out-of-order sparse CSR row-gather copy of the whole AnnData (~150 ms on Visium-width tables, more on Xenium/Visium-HD). After #709 color is resolved per shape by _extract_color_column (region-masks + reindexes the table itself) and after #711 the join is a left join, so the joined element/table are no longer needed: use the original element + table. An audit confirms no downstream consumer needs the joined table - the matplotlib/datashader draws never touch it, _decorate_axs's adata is a dead parameter, and .uns custom colors are read from sdata[table_name] inside _set_color_source_vec. Output is pixel-identical to the join path (verified on partial-annotation/na_color, fully-annotated, real datashader (curio 69k) and real Visium), so the visual baselines are unchanged. Measured: visium_hne render_shapes(color=gene) 451 ms -> 297 ms (1.52x). The outline path keeps its own (rarer) join; converting it is a separate follow-up.
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.
What
_render_shapescolored by a table column joined the element to its annotating table via_join_table_for_element, whosetable[joined_indices, :].copy()does an out-of-order sparse CSR row-gather copy of the whole AnnData every render. This drops that join — the original element + table are used directly.This is the structural follow-up ("Phase 2") to #709 (the color re-join) and #711 (na_color for unannotated). After those landed, the joined element/table are no longer needed.
Why it's safe (no behavior change)
_extract_color_column(perf(color): extract one aligned column instead of copying the whole table #709), which region-masks and reindexes the table itself — it never needed the table pre-joined.how="left"(fix: render unannotated shapes with na_color instead of dropping them #711) already keeps all shapes with na_color for unannotated, in the element's original order — so dropping the join (which equally keeps all shapes in original order) renders the same thing.table/adataconsumer: the matplotlib and datashader draw paths never touch the table;_decorate_axs'sadatais a dead parameter;.uns["{col}_colors"]custom colors are read fromsdata[table_name]inside_set_color_source_vec, not the passed object. Nothing requires the joined/region-filtered table.Result
visium_hne render_shapes(color=gene): 451 ms → 297 ms (1.52×). The saving is the eliminated table copy and scales with table width (larger for Xenium / Visium-HD-width tables). datashader-dominated renders (e.g. curio) see little change because the copy is a small fraction of their cost.Scope
Fill path only — the outline path keeps its own (rarer)
_join_table_for_element(separate outline table + its own length-alignment logic); converting it is a small separate follow-up._join_table_for_elementitself is unchanged (still used by the outline path).