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

Optimize performance of maps #10270

Merged
merged 4 commits into from
Aug 13, 2016
Merged

Optimize performance of maps #10270

merged 4 commits into from
Aug 13, 2016

Conversation

ahejlsberg
Copy link
Member

This PR optimizes the compiler's use of objects as maps as follows:

  • Require the host to be ECMAScript 5 compliant and create maps using Object.create(null). This ensures maps have no prototype (and thus no inherited members).
  • Apply the delete operator to every newly created map object. This causes V8 to put the object in "dictionary mode" and disables creation of hidden classes which are very expensive for objects that are constantly changing shape.
  • Skip hasOwnProperty calls on property lookups. They're no longer necessary when objects are known to have no prototype.

The PR introduces a new MapLike<T> type that represents a regular object used as a lookup (e.g. an object literal). A MapLike<T> requires use of hasOwnProperty to determine whether a particular property was inherited from the prototype.

The Map<T> type now has a brand and Map<T> instances can only be created by calling createMap<T>() (which in turn calls Object.create(null) and puts the object into dictionary mode).

Performance impact on the Monaco real world code base is as follows:

  • 15% overall speed increase (6.7s vs. 7.9s).
  • 19% less memory consumed (211Mb vs. 261Mb).

Definitely a worthwhile optimization, provided we're ok with dropping support for ECMAScript 3 hosts.

@RyanCavanaugh
Copy link
Member

👍, logged #10278

@rbuckton
Copy link
Member

@ahejlsberg #10298 is an alternative to this and is a port to master of what I was working on for the transforms branch. It has the same improvements in speed and memory usage as this PR but also keeps support for ES3.

@rbuckton
Copy link
Member

👍 following the discussion in today's design meeting. I've closed #10298 in favor of this. I'll follow up with a separate PR for the isolated cases that should also be switched from MapLike to Map.

@@ -1084,7 +1084,7 @@ namespace ts {

function internIdentifier(text: string): string {
text = escapeIdentifier(text);
return hasProperty(identifiers, text) ? identifiers[text] : (identifiers[text] = text);
return identifiers[text] || (identifiers[text] = text);
Copy link
Member

Choose a reason for hiding this comment

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

Could do === undefined to avoid re-interning ""

@ahejlsberg ahejlsberg merged commit 5bdde3b into master Aug 13, 2016
@WanderWang
Copy link

WanderWang commented Aug 22, 2016

from this PR , my project got lots of improvement on compile time and memory used .

before :

Files: 606
Lines: 201741
Nodes: 697043
Identifiers: 205454
Symbols: 100360
Types: 17258
Memory used: 407817K
I/O read: 0.00s
I/O write: 0.05s
Parse time: 0.03s
Bind time: 0.00s
Check time: 3.35s
Emit time: 0.86s
Total time: 4.24s

after:

Files: 606
Lines: 201744
Nodes: 697061
Identifiers: 205460
Symbols: 100366
Types: 17266
Memory used: 333287K
I/O read: 0.00s
I/O write: 0.05s
Parse time: 0.03s
Bind time: 0.00s
Check time: 2.76s
Emit time: 0.86s
Total time: 3.64s

Great jobs !!!

@RyanCavanaugh RyanCavanaugh deleted the optimizeMaps branch August 22, 2016 16:20
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants