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

Property binding perf improvements #32212

Conversation

@pkozlowski-opensource
Copy link
Member

commented Aug 20, 2019

This is a set of small perf changes related to property bindings. Changes from those 2 commits save us ~6% of the processing time on the property_binding micro-benchmark.

More (and more substantial) changes to follow.

@googlebot googlebot added the cla: yes label Aug 20, 2019

@pkozlowski-opensource pkozlowski-opensource force-pushed the pkozlowski-opensource:prop_binding_check_perf branch from 7e137e8 to 33e5a50 Aug 20, 2019

@ngbot ngbot bot modified the milestone: needsTriage Aug 20, 2019

@pkozlowski-opensource pkozlowski-opensource marked this pull request as ready for review Aug 20, 2019

@pkozlowski-opensource pkozlowski-opensource requested a review from angular/fw-core as a code owner Aug 20, 2019

@mhevery
Copy link
Member

left a comment

Just some minor suggestions.

packages/core/src/render3/util/misc_utils.ts Outdated Show resolved Hide resolved
@kara
kara approved these changes Aug 20, 2019
Copy link
Contributor

left a comment

LGTM

packages/core/src/render3/util/misc_utils.ts Outdated Show resolved Hide resolved

@pkozlowski-opensource pkozlowski-opensource force-pushed the pkozlowski-opensource:prop_binding_check_perf branch from 33e5a50 to b7517a1 Aug 21, 2019

// NaN is the only value that is not equal to itself so the first
// test checks if both a and b are not NaN
return !(a !== a && b !== b) && a !== b;
return a !== b && !(typeof a === 'number' && isNaN(a) && typeof b === 'number' && isNaN(b));

This comment has been minimized.

Copy link
@alfaproject

alfaproject Aug 21, 2019

Contributor

If the typeof check is faster than isNaN you might want to do: (typeof a === 'number' && typeof b === 'number' && isNaN(a) && isNaN(b))

perf(ivy): improve NaN checks in change detection
This commit drops our custom, change-detection specific, equality comparison util
in favour of the standard Object.is which has desired semantics.

There are multiple advantages of this approach:
- less code to maintain on our end;
- avoid NaN checks if both values are equal;
- re-write NaN checks so we don't trigger V8 deoptimizations.

@pkozlowski-opensource pkozlowski-opensource force-pushed the pkozlowski-opensource:prop_binding_check_perf branch from b7517a1 to 1314622 Aug 21, 2019

@kara
kara approved these changes Aug 21, 2019
Copy link
Contributor

left a comment

LGTM

@kara

This comment has been minimized.

Copy link
Contributor

commented Aug 21, 2019

AndrewKushnir added a commit that referenced this pull request Aug 21, 2019
perf(ivy): improve NaN checks in change detection (#32212)
This commit drops our custom, change-detection specific, equality comparison util
in favour of the standard Object.is which has desired semantics.

There are multiple advantages of this approach:
- less code to maintain on our end;
- avoid NaN checks if both values are equal;
- re-write NaN checks so we don't trigger V8 deoptimizations.

PR Close #32212
ngdevelop-tech added a commit to ngdevelop-tech/angular that referenced this pull request Aug 27, 2019
ngdevelop-tech added a commit to ngdevelop-tech/angular that referenced this pull request Aug 27, 2019
perf(ivy): improve NaN checks in change detection (angular#32212)
This commit drops our custom, change-detection specific, equality comparison util
in favour of the standard Object.is which has desired semantics.

There are multiple advantages of this approach:
- less code to maintain on our end;
- avoid NaN checks if both values are equal;
- re-write NaN checks so we don't trigger V8 deoptimizations.

PR Close angular#32212
sabeersulaiman added a commit to sabeersulaiman/angular that referenced this pull request Sep 6, 2019
sabeersulaiman added a commit to sabeersulaiman/angular that referenced this pull request Sep 6, 2019
perf(ivy): improve NaN checks in change detection (angular#32212)
This commit drops our custom, change-detection specific, equality comparison util
in favour of the standard Object.is which has desired semantics.

There are multiple advantages of this approach:
- less code to maintain on our end;
- avoid NaN checks if both values are equal;
- re-write NaN checks so we don't trigger V8 deoptimizations.

PR Close angular#32212
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.