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

improve compression of undefined, NaN & Infinitiy #1748

Merged
merged 2 commits into from
Mar 31, 2017

Conversation

alexlamsl
Copy link
Collaborator

Maintains identical behaviour, but move all the transformation logic from OutputStream to Compressor.

Next up would be to implement #1730.

@kzc if you want Infinity ➡️ 1/0 & undefined ➡️ void 0 to be gated behind a new flag (default enabled), mind coming up with a good name for it? 😅

Maintains identical behaviour, but move all the transformation logic from `OutputStream` to `Compressor`.
@kzc
Copy link
Contributor

kzc commented Mar 31, 2017

if you want Infinity ➡️ 1/0 & undefined ➡️ void 0 to be gated behind a new flag (default enabled)

So, just to confirm - NaN will always remain NaN?

I think undefined ➡️ void 0 should always take place as it presently does in the Compressor as there is no size or speed disadvantage - unless it's one of those crazy cases where it's a redefined variable name.

That just leaves Infinity vs. 1/0. How about the unambiguous option name keep_infinity?

@alexlamsl
Copy link
Collaborator Author

So, just to confirm - NaN will always remain NaN ?

NaN will always remain NaN unless when a locally defined variable named NaN exists, in which case it will indiscriminately turned into 0/0 to avoid the bug.

I think undefined ➡️ void 0 should always take place as it presently does in the Compressor as there is no size or speed disadvantage

So that Chrome performance degradation only applies to NaN and Infinity but not undefined? If so, yeah, I'll just gate with keep_inf (or keep_infinity in case anybody gets funny ideas about .INF files).

@alexlamsl
Copy link
Collaborator Author

And to clarify, I think the forced NaN ➡️ 0/0 is okay because it will be converted from things like +"foo", and so:

  • even with parenthesis the representation should be shorter
  • performance of things like Infinity * null would probably be comparable to 0/0 anyway

@alexlamsl
Copy link
Collaborator Author

Current PR passed half a mega-fuzz without incident.

Proceed with Phase 2. 😎

@kzc
Copy link
Contributor

kzc commented Mar 31, 2017

So that Chrome performance degradation only applies to NaN and Infinity but not undefined?

As far as I know. void 0 a common pattern that they would optimize because of the widespread use of minifiers.

@alexlamsl alexlamsl changed the title [WIP] improve compression of undefined, NaN & Infinitiy improve compression of undefined, NaN & Infinitiy Mar 31, 2017
- migrate transformation logic from `OutputStream` to `Compressor`
- always turn `undefined` into `void 0` (unless `unsafe`)
- always keep `NaN` except when avoiding local variable redefinition
- introduce `keep_infinity` to suppress `1/0` transform, except when avoiding local variable redefinition

supersedes mishoo#1723
fixes mishoo#1730
@alexlamsl alexlamsl merged commit 257ddc3 into mishoo:master Mar 31, 2017
@alexlamsl alexlamsl deleted the issue-1730 branch March 31, 2017 19:02
@kzc
Copy link
Contributor

kzc commented Mar 31, 2017

@alexlamsl Kudos for the elimination of PARENS([ AST_Infinity, AST_NaN ], DEFPRINT(AST_Undefined, DEFPRINT(AST_Infinity and DEFPRINT(AST_NaN. The code is a lot cleaner with this PR. Everything is in the correct place.

$ echo 'console.log(undefined, Infinity, NaN);' | bin/uglifyjs -c
console.log(void 0,1/0,NaN);
$ echo 'console.log(undefined, Infinity, NaN);' | bin/uglifyjs -c keep_infinity
console.log(void 0,Infinity,NaN);

+1

@alexlamsl
Copy link
Collaborator Author

@kzc glad you like it 😉

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.

2 participants