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

Turn on more aggressive compiler optimizations. #3223

Merged
merged 1 commit into from Jun 6, 2016

Conversation

cramforce
Copy link
Member

@cramforce cramforce commented May 15, 2016

  • allows devirtualization of private methods.
  • allows renaming of private properties and methods.
  • Aliases .prototype. pattern where useful.
  • other less important optimizations.

Implements a coding convention for AMP that marks all properties not ending in _ as exported (so they can travel between co
mpilation units).

This change by itself has little size impact, but should make our code quite a bit tighter to execute, because many virtual p
rivate method calls because function calls. Primarily this is a preparation to turn on private property renaming when the ups
tream change lands in closure compiler.

@cramforce
Copy link
Member Author

cramforce commented May 15, 2016

25KB pre-gzip win on v0.js :)

@erwinmombay

@erwinmombay
Copy link
Member

woot, good stuff.

@erwinmombay
Copy link
Member

@cramforce guessing this would also have an improvement on eval time?

@cramforce cramforce force-pushed the nocollapse branch 2 times, most recently from bcd762e to 6d61eb1 Compare May 16, 2016 03:38
@cramforce
Copy link
Member Author

Got another 4KB improvement. amp-iframe is currently broken, because firstAttachedCallback is optimized away from main binary. Need to either explicitly extern it or turn off the responsible DCE.

@cramforce cramforce changed the title [Not yet landable] Turn on much more aggressive compiler optimizations. Turn on more aggressive compiler optimizations. Jun 2, 2016
@cramforce
Copy link
Member Author

@erwinmombay Please take a look. I removed the property renaming from this change. Little size impact, but having the externs in place and such will make the next change easier.

@cramforce
Copy link
Member Author

I guess 10K pre-gzip on main binary is still kinda worth it anyway :)

// AMP's globals
window.AMP_TEST;
window.AMP_TAG;
window.AMP = {};
Copy link
Member

Choose a reason for hiding this comment

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

do we need to add window.AMP_CONFIG

Copy link
Member Author

Choose a reason for hiding this comment

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

Added

@cramforce cramforce force-pushed the nocollapse branch 2 times, most recently from c4c0982 to 079f063 Compare June 3, 2016 01:38
@erwinmombay
Copy link
Member

@cramforce LGTM

@cramforce cramforce force-pushed the nocollapse branch 3 times, most recently from 812a753 to 6e0fd8d Compare June 6, 2016 04:45
- allows devirtualization of private methods.
- allows renaming of private properties and methods.
- Aliases `.prototype.` pattern where useful.
- other less important optimizations.

Implements a coding convention for AMP that marks all properties not ending in `_` as exported (so they can travel between compilation units).

This change by itself has little size impact, but should make our code quite a bit tighter to execute, because many virtual private method calls because function calls. Primarily this is a preparation to turn on private property renaming when the upstream change lands in closure compiler.
@cramforce cramforce merged commit 5c2710d into ampproject:master Jun 6, 2016
@cramforce cramforce deleted the nocollapse branch June 6, 2016 14:15
blickly pushed a commit to google/closure-compiler that referenced this pull request Jun 6, 2016
…coding convention.

This is a slight change in behavior for projects based on the GoogleCodingConvention that considers properties starting with "_" to be exported, which should be very rare under this convention.

Compiler change prototyped in ampproject/amphtml#3223
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=124020030
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants