Skip to content

Initialize properties to avoid hidden class thrashing.#1752

Merged
sebmck merged 1 commit intobabel:masterfrom
loganfsmyth:hidden-class-change
Jun 15, 2015
Merged

Initialize properties to avoid hidden class thrashing.#1752
sebmck merged 1 commit intobabel:masterfrom
loganfsmyth:hidden-class-change

Conversation

@loganfsmyth
Copy link
Copy Markdown
Member

I think this is all the properties for NodePath?

On my machine, building ember goes down 10-15%ish.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

i would like to recommend undefiend over `null.

undefined: no value
null: not a value

but both shape the object correctly.

This allows for obj.key === undefined checks for fast property checks, without needing a sentinel.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I agree with Stefan, undefined is probably better choice.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@bmeurer does it have better perf than initializing as null and comparing against null later? or are you suggesting this because of the mentioned different semantics (no value / not a value)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

There's probably no performance difference, but I think it's easier to maintain long-term if you keep the invariant that undefined stands for no value as @stefanpenner pointed out above.

sebmck added a commit that referenced this pull request Jun 15, 2015
Initialize properties to avoid hidden class thrashing.
@sebmck sebmck merged commit c0e5059 into babel:master Jun 15, 2015
@sebmck
Copy link
Copy Markdown
Contributor

sebmck commented Jun 15, 2015

@stefanpenner Probably doesn't matter too much. Not sure if there's many places were that would come in handy.
@loganfsmyth Thank you! 😄

@sebmck sebmck mentioned this pull request Jun 15, 2015
13 tasks
@loganfsmyth loganfsmyth deleted the hidden-class-change branch June 15, 2015 16:41
@JLHwung JLHwung mentioned this pull request Sep 23, 2019
@lock lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Oct 5, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area: perf outdated A closed issue/PR that is archived due to age. Recommended to make a new issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants