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.

Sign up for GitHub

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

More specific if conditions lead to ~10% faster render. #610

Merged
merged 4 commits into from
Apr 2, 2017
Merged

More specific if conditions lead to ~10% faster render. #610

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4ee46be
More specific if conditions lead to ~10% faster render.
asolove Mar 26, 2017
2098502
Merge branch 'master' into faster-explicit-if
developit Mar 29, 2017
b8e367f
Merge branch 'master' into faster-explicit-if
robertknight Mar 31, 2017
052a1e0
Merge branch 'master' into faster-explicit-if
robertknight Apr 2, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/vdom/diff.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,10 +187,10 @@ function innerDiffNode(dom, vchildren, context, mountAll, absorb) {
min = 0,
len = originalChildren.length,
childrenLen = 0,
vlen = vchildren && vchildren.length,
vlen = vchildren ? vchildren.length : 0,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the real culprit, as now vlen will always be a Number (and known to the compiler as such). You could probably go one step further and avoid the ToBoolean on vchildren as well by writing something like vlen = (vchildren !== undefined) ? vchildren.length : 0 if that matches the contract.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your insights, @bmeurer!

Would an undefined default (e.g.vlen = vchildren ? vchildren.length : undefined) be less optimal for the compiler than the default of 0? Same question for null. Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

undefined or null are less ideal than 0 here, as the compiler needs to choose a tagged representation that can represents both undefined/null as well as numbers. If you use only (small integer) numbers, then the optimizing compilers in VMs can usually pick the ideal unboxed Word/Word32 representation.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it. Thank you!

j, c, vchild, child;

if (len) {
if (len!==0) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While you're making things more explicit, don't you really mean len > 0 in both cases?

Copy link
Contributor Author

@asolove asolove Mar 29, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using > and < does valueOf coercion on non-numeric values, so I have a vague guess that doing an exact equals check may be faster than greater-than. But, I did not actually profile and am not an expert on this, so if someone else knows better please correct me.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strict equality should be faster than a comparison since it does no casting, but @bmeurer is far more knowledgeable on that than I am.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand now. You're focusing on perf over explicit code intent. 👍

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the ?: above the optimizing compiler knows that len is a number, so len > 0 is fine. Shouldn't be any speed difference for this. Even the interpreted code is mostly equivalent from perf perspective.

for (let i=0; i<len; i++) {
let child = originalChildren[i],
props = child[ATTR_KEY],
Expand All @@ -205,7 +205,7 @@ function innerDiffNode(dom, vchildren, context, mountAll, absorb) {
}
}

if (vlen) {
if (vlen!==0) {
for (let i=0; i<vlen; i++) {
vchild = vchildren[i];
child = null;
Expand Down