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

Fix invalid recursive dirtying when mutating nodes (significant performance improvement). #148

Merged
merged 1 commit into from
Oct 17, 2019

Conversation

timlindvall
Copy link
Contributor

Currently, ember-template-recast is getting hung up on deeply nested
trees with lots of changes. It appears markAsDirty(), which should traverse all its parents,
is making a bunch of additional recursions that aren't necessary.

This change updates the logic so it strictly visits its ancestors only.

Currently, ember-template-recast is getting hung up on deeply nested
trees with lots of changes. It appears markAsDirty(), which should traverse all its parents,
is making a bunch of additional recursions that aren't necessary.

This change updates the logic so it strictly visits its ancestors only.
Copy link
Member

@rwjblue rwjblue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, why did I do this?!

It looks like a broken refactor or something 🤔. Like I was looping upwards marking as dirty, then refactored it to be recursive instead but forgot to remove the loop?

@timlindvall
Copy link
Contributor Author

timlindvall commented Oct 17, 2019

Afraid I'm not sure if there's any meaning or intent behind the double-loop. As far as I can tell, the additional work is superfluous, but I might be missing something. git blame isn't yielding anything useful from the commit history. =/

The good news is that updating this means scripts using ember-template-recast won't get stuck when processing deeply nested nodes, as well as other operations in general appear to run much faster. ^_^

@rwjblue rwjblue merged commit 051f700 into ember-template-lint:master Oct 17, 2019
@rwjblue rwjblue changed the title Fix looping logic in markAsDirty() Fix invalid recursive dirtying when mutating nodes (significant performance improvement). Oct 17, 2019
@rwjblue rwjblue added the bug Something isn't working label Oct 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants