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

Alternative production code generation for AMP based on the closure compiler #1218

Merged
merged 1 commit into from
Dec 24, 2015

Conversation

cramforce
Copy link
Member

  • Currently a 15K win on main binary and likely much faster initial JS compilation due to simpler code.
  • Includes a compiled compiler binary while we wait for a patch to land that is needed to compiler core-js.
  • Doesn't yet activate (and benefit in code size) from type checking.

@@ -434,7 +434,7 @@ export class Viewer {
* @param {boolean} awaitResponse
* @return {(!Promise<*>|undefined)}
* @package
* @export
* _export
Copy link
Contributor

Choose a reason for hiding this comment

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

We do manual exports practically for everything else. So, we could do the same here.

Copy link
Member Author

Choose a reason for hiding this comment

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

There is no magic here anyway. For a long time we won't be able to run with property obfuscation anyway.

@cramforce
Copy link
Member Author

@erwinmombay I ported this based on your change to use gulp for the compiler invocation. And bundled a compiled compiler that has my patch with the PR.

The nice thing: The generated code actually works :)

Makes compiled code 15KB smaller. 3KB after GZip.

This is with simple optimizations only.

@erwinmombay
Copy link
Member

@cramforce thats awesome to hear!

@cramforce cramforce changed the title [WIP] Working-an-in-it-compiles closure compiler based compilation for AMP Alternative production code generation for AMP based on the closure compiler Dec 24, 2015
@cramforce
Copy link
Member Author

@erwinmombay Updated version. Major win: I was able to get rid of listing the core-js files individually. Had to set a flag that was also otherwise important :) only_closure_dependencies: true, prunes included files that were not required by the base module and fixes the issue. That makes this whole thing much more robust.

I think we should submit this and then work from there to transform the rest of the code gen.

- Currently a 15K win on main binary and likely much faster initial JS compilation due to simpler code.
- Includes a compiled compiler binary while we wait for a patch to land that is needed to compiler core-js.
- Doesn't yet activate (and benefit in code size) from type checking.
@erwinmombay
Copy link
Member

@cramforce LGTM.

cramforce added a commit that referenced this pull request Dec 24, 2015
Alternative production code generation for AMP based on the closure compiler
@cramforce cramforce merged commit c8bc5b8 into ampproject:master Dec 24, 2015
@cramforce cramforce deleted the cc branch December 24, 2015 15:32
@qidonna qidonna mentioned this pull request Aug 27, 2020
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.

3 participants