Skip to content

perf(tree): keep tree-selection linear in the number of rows (#DS-5796) - #2255

Open
nazirgitwork wants to merge 2 commits into
koobiq:mainfrom
nazirgitwork:perf/DS-5796-tree-rendering
Open

nazirgitwork wants to merge 2 commits into
koobiq:mainfrom
nazirgitwork:perf/DS-5796-tree-rendering

Conversation

@nazirgitwork

Copy link
Copy Markdown

Summary

KbqTreeSelection did work proportional to the number of rows several times on each render, and some of it for each row. In a large tree that work grew with the square of the number of rows. This PR removes the quadratic paths and the per-row subscriptions. It also stops mouseenter from running change detection in trees outside a select.

List of notable changes:

  • updated the option ordering in KbqTreeSelection: options are indexed once into a by-node map and a by-value map. Before, unorderedOptions was searched for every node, which was O(n²). As before, the first option wins in each map, and a node match takes precedence over a value match.
  • updated optionFocusChanges / optionBlurChanges: each one is now a single stream that options report to through the new KbqTreeOptionParent.reportOptionFocus() / reportOptionBlur() methods. Before, every call merged the streams of all rendered options, and the tree re-subscribed after each render. That cost O(n) per render, and a subscriber never received focus events from rows rendered after it subscribed. It now does.
  • fixed the focus of a row that a data change brings in: it reaches the key manager immediately. Before, the per-option subscriptions were only rebuilt after delay(0), so a row focused in that gap was missed.
  • updated KbqTreeOption: mouseenter is no longer a host listener. It is bound outside the Angular zone and re-enters the zone only when the tree is inside a select, the only case that reacts to it. A tree outside a select no longer checks views for every row the pointer crosses.
  • added tests: no view checks on mouseenter outside a select, focus of a newly rendered row, optionFocusChanges reporting rows rendered after subscribing, and keyboard order across expand, collapse and new data when rows share values.
  • updated the API golden file and the check-api-docs baseline: two new members, and optionFocusChanges / optionBlurChanges are now documented.

What should reviewers focus on?

  • KbqTreeOptionParent gains two required methods. Code that provides its own implementation of KBQ_TREE_OPTION_PARENT_COMPONENT will stop compiling until it adds them. Is that acceptable here, or should they be optional?
  • optionFocusChanges / optionBlurChanges now also emit for rows rendered after the subscription. Before, the observable covered only the rows rendered when the getter was called.
  • The mouseenter listener runs outside the zone. Please check it in zoneless apps and in kbq-tree-select, where hovering a row still has to move focus.

…(#DS-5796)

Re-rendering the rows searched the selected values once per row, costing
rows × selected. The values now go into a Set.

The tree also completes optionFocusChanges and optionBlurChanges on destroy,
so their subscribers complete with it.
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.

1 participant