Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

Perf Idea: Template Compilation Re-use #13915

Open
dcherman opened this issue Feb 1, 2016 · 6 comments
Open

Perf Idea: Template Compilation Re-use #13915

dcherman opened this issue Feb 1, 2016 · 6 comments

Comments

@dcherman
Copy link
Contributor

dcherman commented Feb 1, 2016

Filing this for future consideration since this will be too big of a change to even think about considering it for 1.5.

One optimization that I've used with a lot of success in perf sensitive components is the re-use of the compiled template rather than letting the compile step re-compile the same template over and over again.

The basic idea is that once a template is compiled, you store that template under a cache that is keyed with the template string itself. As a result, you get to skip quite a lot of the work previously done in compile especially in directives that are composed together.

Generic Bag of Concerns:

  1. Unbounded memory growth potential? What happens when a template is a function that changes often enough such that we have many stale cache entries. Is this reasonable?
  2. Similar to the above, are there concerns with people replacing the content of an already loaded templateUrl?
  3. This would completely fall apart if a directive uses compile. This is the biggest consideration I've come up with so far; the compile function would modify the generated function for all future linking usage which is almost definitely not what you would want, however we can feature detect this by checking for the presence of a compile function and bailing out.
@lgalfaso
Copy link
Contributor

lgalfaso commented Feb 1, 2016

Quite a few alternatives were discussed regarding performance improvements to the compiler. On top of the one @dcherman stated above, these (among many) were discussed:

  • If a directive defines a templateUrl and the template is in the $templateCache, then handle the template synchronously
  • If a directive defines a template, then do not inline the template directly but compile and cache it first (just like what ngView does)
  • Generate a DOM-like or virtual DOM and, for a given template, collect the directives in a pre-processing step. This information should then be used so the template DOM is not traversed looking for directives

In all cases, the biggest problem is that any change like this would be too big of a breaking change and would cause friction for people to upgrade to Angular's latest version.

This does not mean that we cannot revisit some of these opportunities and put some effort into them, but that any work on this behalf does need some careful consideration.

@dcherman
Copy link
Contributor Author

dcherman commented Feb 3, 2016

So I thought about my original idea more. The biggest downfall is the compile function. Even if the user didn't provide one on the directive with a template, an attribute directive with a lower priority could have been tinkering with the DOM in it's own compile function, so we cannot know the final state that should be linked unless you at least collect the directives and examine them for compile functions.

That means that we must always at least run the collectDirectives portion of compile, but may be able to save the applyDirectivesToNode portion, but only if we can guarantee no directives tinkered with anything. Since many of Angular's own directives use the compile function, that also means that we are unlikely to be able do this optimization since we cannot know whether or not that function will modify the DOM or not without actually running it.

End users can still implement this optimization themselves where they have increased knowledge of how a directive is used and what should be allowed, but this is looking unlikely to be a library level optimization 😢

@dcherman
Copy link
Contributor Author

dcherman commented Feb 4, 2016

Out of curiosity, what are the concerns with making the linking process synchronous when templateUrl is used with a warm cache?

I think I can make a strong argument that it may actually be preferable to do so in 1.5 as it may actually help address #13884. One of my co-workers should be posting a reproducer (kind of) with a writeup tomorrow from some debugging that we did, but I will add something to that issue if he doesn't since it's pretty late here now.

@lgalfaso
Copy link
Contributor

lgalfaso commented Feb 4, 2016

The main issue is that directives authors that use templateUrl may have some expectation that their directives will be compiled asynchronously always; that their code is not able to handle the synchronous and asynchronous case, and that testing all variations (say with nested directives) can be difficult.

About #13884, other options are in the table (none fully baked for 1.5). The main issue is getting a solution that has a consistent and predictable execution, and does not break the existing code.

@dcherman
Copy link
Contributor Author

dcherman commented Feb 4, 2016

So if you look at #13884 (comment), you can actually see that even in existing versions, users are exposed to both sync and async behaviors due to how async directives are replaced with derived sync ones during the compilation process.

As a result, you'll get different linking behavior right now if you have a simple parent -> child structure versus that same structure but with transclusion involved via something like ngIf, ngRepeat, etc.. on the child directive.

Since I continue to assert that the common case (and best practice) is for people to pre-warm the template cache via a build step, making the template compilation synchronous would result in behaviors more similar to previous versions that didn't have lazy compilation. In either case, it looks like people cannot currently rely on linking order in any version. I generally recommend using controllers and require in order to inform the parent when a child arrives versus the parent assuming a child is present due to linking order FWIW.

@lgalfaso
Copy link
Contributor

lgalfaso commented Feb 5, 2016

I know about the current discrepancies, but the end goal for the compilation/linking to not have these discrepancies and simplify things for developers. I find that adding this synchronous templateUrl mode goes agains this end goal.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants