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

GraphNode: Improve performance of node disposal in large ref lists #84

Merged
merged 1 commit into from
May 21, 2023

Conversation

donmccurdy
Copy link
Owner

@donmccurdy donmccurdy commented May 7, 2023

More optimization could be done here, but I'm concerned that complex behavior changes (batch disposal, lazy cleanup, etc.) could have more side effects and maintenance requirements than I'm prepared to deal with.

In glTF Transform v4, I'm tempted to use Sets for RefLists, which should help here dramatically, but that is a breaking change to APIs, and doesn't justify a major release right now.

This change is simple enough, and improves performance dramatically when disposing of items in large reference lists. On quick benchmarks, this was a 5x performance improvement for a list containing 30,000 items.

Related:

@donmccurdy donmccurdy added the enhancement New feature or request label May 7, 2023
@codecov-commenter
Copy link

Codecov Report

Merging #84 (21d78d5) into main (935f4b6) will increase coverage by 0.03%.
The diff coverage is 100.00%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##             main      #84      +/-   ##
==========================================
+ Coverage   80.26%   80.30%   +0.03%     
==========================================
  Files           5        5              
  Lines         598      599       +1     
  Branches       34       32       -2     
==========================================
+ Hits          480      481       +1     
  Misses        117      117              
  Partials        1        1              
Impacted Files Coverage Δ
src/graph-node.ts 74.15% <100.00%> (+0.07%) ⬆️

@donmccurdy donmccurdy merged commit 2c7d318 into main May 21, 2023
@donmccurdy donmccurdy deleted the perf/dispose-callback branch May 21, 2023 16:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GraphNode: batch dispose mode
2 participants