perf(common): code size reduction of ngFor directive#44315
perf(common): code size reduction of ngFor directive#44315JoostK wants to merge 1 commit intoangular:masterfrom
ngFor directive#44315Conversation
|
Note: the total size savings for the todo example app is 395 bytes. The removal of the insertion tuples buffer also reduces runtime memory pressure. |
AndrewKushnir
left a comment
There was a problem hiding this comment.
This PR is in the draft mode (possibly in the "work in progress" state), but I wanted to leave a quick comment with an additional idea anyways :)
There was a problem hiding this comment.
The changes look great, thanks @JoostK 👍
One more idea that we can explore is to extract the this._viewContainer into a local const, so it's better minified:
const viewContainer = this._viewContainer;
// ...
viewContainer.move(view, currentIndex);
would be minified to something like this:
var v = this._viewContainer;
// ...
v.move(view, currentIndex);
and given the fact that we have 7 instances of this._viewContainer inside the function - we'd get more savings.
Also, that may be a tiny perf runtime improvement as well, since we'd avoid extra property reads.
There was a problem hiding this comment.
Thanks Andrew, that's an excellent idea!
|
Cool, we're now at 497 fewer bytes! I'll see if I can get the tests green (mostly golden updates) before marking this for review. |
AndrewKushnir
left a comment
There was a problem hiding this comment.
@JoostK thanks for the perf improvement 👍
ce68a0d to
5f697f6
Compare
AndrewKushnir
left a comment
There was a problem hiding this comment.
Reviewed-for: size-tracking
|
Merge assistance: presubmits for this change are successful. |
jessicajaniuk
left a comment
There was a problem hiding this comment.
LGTM 🍪
reviewed-for: size-tracking
This commit makes several changes to the implementation of `NgForOf` to reduce its code size in production builds: 1. The tailor-made message for an unsupported differ is fully tree-shaken in production builds, in favor of the exception from the differ factory itself. 2. The private `_perViewChange` method was changed into a free-standing function, to allow its name to be minimized. 3. The need for an intermediate `RecordViewTuple` was avoided by applying the operation in-place, instead of collecting all insertions into a buffer first. This is safe as the `_perViewChange` operation that used to be done on each `RecordViewTuple` is entirely local to the tuple itself. Hence, it is invariant to execution ordering which means that the `_perViewChange` can be executed directly during the `forEachOperation` loop.
5f697f6 to
a724d4b
Compare
|
FYI, there was a problem merging this PR, so I did a rebase. Will try to merge it again once CI is completed. |
|
This PR was merged into the repository by commit 1336297. |
This commit makes several changes to the implementation of `NgForOf` to reduce its code size in production builds: 1. The tailor-made message for an unsupported differ is fully tree-shaken in production builds, in favor of the exception from the differ factory itself. 2. The private `_perViewChange` method was changed into a free-standing function, to allow its name to be minimized. 3. The need for an intermediate `RecordViewTuple` was avoided by applying the operation in-place, instead of collecting all insertions into a buffer first. This is safe as the `_perViewChange` operation that used to be done on each `RecordViewTuple` is entirely local to the tuple itself. Hence, it is invariant to execution ordering which means that the `_perViewChange` can be executed directly during the `forEachOperation` loop. PR Close #44315
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
This commit makes several changes to the implementation of
NgForOftoreduce its code size in production builds:
tree-shaken in production builds, in favor of the exception from the
differ factory itself.
_perViewChangemethod was changed into a free-standingfunction, to allow its name to be minimized.
RecordViewTuplewas avoided byapplying the operation in-place, instead of collection all insertions
into a buffer first. This is safe as the
_perViewChangeoperationthat used to be done on each
RecordViewTupleis entirely local tothe tuple itself. Hence, it is invariant to execution ordering which
means that the
_perViewChangecan be executed directly during theforEachOperationloop.