Repository navigation
Strange performance issue in 2.0 when rendering Angular components for the first time #179
Description
Hi all,
The work on theme 2.0 is superb, but I have an interesting problem with performance.
Please take a look at these two sample projects:
Using theme 1.0:
https://www.dropbox.com/s/pglosneddnf2t08/Ns61Old.zip?dl=1
Using theme 2.0:
https://www.dropbox.com/s/m0i970t3ihk2vxl/Ns61New.zip?dl=1
I am running this on Android, using NS 6.1 and all the latest packages available.
I understand the demo is not exactly real-world, but it gets the point across.
home.component.html looks as follows:
<Button text="Toggle" (tap)="toggle()"></Button>
<Switch [(ngModel)]="isExpanded"></Switch>
<StackLayout *ngIf="isExpanded">
<ns-test [time]="time"></ns-test>
</StackLayout>
<ng-container *ngFor="let i of arr">
<ns-my-component [index]="i"></ns-my-component>
</ng-container>
The issue is that when I am loading a component for the first time, depending on how much content is on the screen, it takes a seemingly logarithmic amount of time to render. In the demo provided above, on my Android tablet, it is taking ~3s to render a tiny component the first time I try by clicking Toggle. With theme 1.0 it takes ~250ms.
In the demo I am spinning up 700 instances of ns-my-component. I understand this is not what would happen in the real world, but in my more complicated real-world app I am having the same issue with components taking ~5s to render the first time sometimes, except with significantly less amount of components being rendered, but more complex styling and more nesting, perhaps.
Additionally, even though 700 instances of a component is not a real-world example, it is demonstrable that whilst rendering a component for the first time is still slower in 1.0, between 1.0 and 2.0 first-time rendering becomes 10x slower.
But only the first time the component renders, which is another point. Even though there are a lot of components on the screen, why should this affect rendering a tiny component later on? And only the first time that component is ever used?
Interestingly, in all cases, AOT and other production build options don't seem to make any meaningful difference to this. I have not tested snapshots or iOS.
The screen I have where I am getting ~5s delay is really not a very busy screen, neither does it have anything like 700 components on it.
Help!
Activity
joshcomley commentedon Sep 15, 2019
I'll just add: I can hack around this issue for my users, by rendering a blank version of whatever component is slow to load early on.
So, for example, if you change
test.component.tsngOnInit()to look like this:And modify
home.component.html:You'll find the first toggle of the component by hitting the
Togglebutton is ~10ms, even though there are 700 items rendered elsewhere on the page.This smells like a bug because it can be worked around with a hack quite easily.
bundyo commentedon Sep 15, 2019
From what I read, there might be a bug in {N} itself, than in the Theme. I'll check it out tomorrow though. Thanks for reporting!
joshcomley commentedon Sep 15, 2019
Yes I was going to comment along those lines actually, having inspected the source for theme there's no magic going on in terms of code running, so it must be how NS itself is handling things and it's merely exemplified by perhaps more complexity in the latest theme 2.0. Let me know if you want me to post this bug over there!
bundyo commentedon Sep 16, 2019
This happens because of the way the selectors map is created for each new view. After the first creation it is cached, so it will be reused for consequent creations. I'll dig a little there if there is something that can be done to speed it up. Meanwhile, you can use compat styling, which doesn't try to style all your elements in one go.
joshcomley commentedon Sep 16, 2019
Hey thanks for looking into it. Is it possible to pre-cache the selectors map somehow? I sort of follow what you're saying, but why would the amount of content on the rest of the screen at the time of instantiation affect things? In other words, why would 100 instances of X slow down the (first time) loading of one instance of Y?
I'll take a look at compat styling - although I thought that was just for backwards compatibility?
bundyo commentedon Sep 16, 2019
It is, though at the same time you can still change light/dark with it. :)
bundyo commentedon Sep 16, 2019
As for the other question - it shouldn't slow down with the number of items, yes. At least it seems that it shouldn't. I'll check it out.
joshcomley commentedon Sep 17, 2019
Thanks again for the responses. I think I'll have to do the compat for now - I have one screen with a toggle that takes (just measured it) 45 seconds to render the first time, and ~2s every subsequent time. The 2s is my own code, but that 45 seconds is crazy! If I limit what else is on the screen then it speeds up significantly. I think the 45s is a combination of a moderate amount of vertical components, but a reasonable amount of nesting, too.
If you'd like I wouldn't mind having a peek at the source of the problem myself - would you be able to point me to where in NS source it is doing the selector map? Maybe I can contribute!
MartoYankov commentedon Oct 4, 2019
@joshcomley Can you try removing the empty
.scssfiles from the example projects you provided? It seems there is a bug there. All in all if you have style files with repeating styles, that is when you use the same nested components a lot, it's better to extract those styles in theapp.cssfile.bundyo commentedon Oct 4, 2019
Yes, seems loading all those component CSS files triggers a bug in {N}. Use Marto's workaround until it gets fixed.
joshcomley commentedon Oct 4, 2019
@MartoYankov Thanks for this. Out of curiosity, would it help to put a dummy rule in these files, or does the bug still persist to files that only contain rules that are never used? I have a huge project with 100s of generated components, would be easier to inject a dummy rule than delete the file and update all the component
tsfiles!MartoYankov commentedon Oct 4, 2019
@joshcomley No, unfortunately the bug is reproduced even if you just reference the style file in your component.
joshcomley commentedon Oct 4, 2019
Hi all, I tried what you said on the sample project but it had no effect. If I put the style in the global CSS files, it works a treat and toggling the toggle is lightning fast. But removing blank SCSS files has no effect:
https://www.dropbox.com/s/cxs44mnzmzqsgxp/Ns61New.zip?dl=1
(I wrote a script to scan and remove all blank SCSS files and comment out the respective line in the component).
However, if I don't use component level SCSS files at all, then it's lightning fast again. For example, if you remove the non-blank SCSS file from
test.component.tsbut left all the other blank SCSS files in and referenced, loading times go from ~2500ms down to ~10ms.joshcomley commentedon Oct 4, 2019
Unless I totally misunderstood this - the bit about them being empty was just highlighting the uselessness of them in the sample project..? I read it as empty SCSS files cause the bug, but I think you meant that any component level SCSS files trigger it, is that right?
joshcomley commentedon Oct 4, 2019
I hate asking this, but is there any estimate on the fix for this, because it would be a mammoth task to re-organise my project structure to separate component level SCSS. I especially make use of simple css class names that are component scoped to keep things clean. Moving all of this to a global context would require major re-engineering to avoid conflicts :(
bundyo commentedon Oct 4, 2019
If the component is reused hundreds of times - you should remove its S/CSS file. If it is used only a couple of times - you can leave its file.
bundyo commentedon Oct 4, 2019
Unfortunately we can't give fix estimate, as currently we only know what triggers the bug and we don't know where the bug is and what it will take to fix it. Might be simple, or it might require refactoring of several components and therefore not simple.
Support for adding scoped CSS for bug fix: NativeScript/theme#179
Scoped styles should not trigger a global refresh (for bug: NativeScr…
joshcomley commentedon Oct 5, 2019
Hi all,
I spent a little time today investigating this issue and isolated the pain points (see the PRs referenced immediately above).
Essentially the issue is that encapsulated, scoped CSS was still being treated like a global CSS modification, and thus triggering a styling refresh of the entire view tree. This is unnecessary for scoped CSS, and the simple fix above eliminates entirely the performance issue (except in the case of using
encapsulation: ViewEncapsulation.None).bundyo commentedon Oct 10, 2019
Closing this as the problem turned out to not be in the Theme.
Support for adding scoped CSS without triggering global refresh (#1999)