perf($compile): only iterate over elements with link functions

[jbedard]

I've been thinking about this a while and @IgorMinar added a TODO for it in 3e0a2e1.

In a quick test I found this lowered the link time 5-10% for a table row with only ~2/15 cells requiring linking. For DOM trees with a higher percentage of annotated nodes this shouldn't have any noticeable effect. However I think having 50%+ non-annotated nodes is very common (a newline/whitespace text node per element...).

[IgorMinar]

This is interesting! I'll review properly tomorrow

On Fri, Aug 22, 2014, 9:28 PM Jason Bedard [email protected] wrote:

I've been thinking about this a while and @IgorMinar
https://github.com/IgorMinar added a TODO for it in 3e0a2e1
3e0a2e1
.

In a quick test I found this lowered the link time 5-10% for a table row
with only ~2/15 cells requiring linking. For DOM trees with a higher
percentage of annotated nodes this shouldn't have any noticeable effect.
However I think having 50%+ non-annotated nodes is very common (a

newline/whitespace text node per element...).

You can merge this Pull Request by running

git pull https://github.com/jbedard/angular.js compile-linkfns

Or view, comment on, or merge it at:

#8741
Commit Summary

• perf($compile): only iterate over elements with link functions

File Changes

• M src/ng/compile.js
  https://github.com/angular/angular.js/pull/8741/files#diff-0 (27)

Patch Links:

• https://github.com/angular/angular.js/pull/8741.patch
• https://github.com/angular/angular.js/pull/8741.diff

—
Reply to this email directly or view it on GitHub
#8741.

[IgorMinar]

you are creating sparse arrays here. that will disable all major array optimizations in the VM. we need to avoid that.

[jbedard]

Could just revert this block then. Or could the sparse array lookups be worth the shorter array and loop?

Or make stableNodeList non-sparse by just pushing the nodes onto it. Then we'd need another array mapping stableNodeList index => linkFn index? Sounds ugly...

[IgorMinar]

I thought about this more and getting rid of the sparse array is not that easy as it makes code in the later loop more complicated. we should at least prealocate the array as we used to.

[IgorMinar]

I went ahead and made the changes myself since I had your branch checked out already. I measured the impact and it's surprisingly small in the large table macro-benchmark. I think it will have bigger impact in the real world where the dom is more sparsely annotated though.

thanks for the PR!

[caitp]

✨

[jbedard]

That was fast. Thanks! Could this also be done merged into 1.2?

[IgorMinar]

We need to cherry-pick a whole bunch of commits to 1.2 but I won't have
time this week so unless someone else syncs master and 1.2 branches it will
have to wait a bit

On Mon, Aug 25, 2014, 11:17 AM Jason Bedard [email protected]
wrote:

That was fast. Thanks! Could this also be done merged into 1.2?

—
Reply to this email directly or view it on GitHub
#8741 (comment).