2.8.0 staging #1485

alexlamsl · 2017-02-11T09:15:27Z

This is a test merge of all 22 open PRs mentioned in #1383 - list of fix-ups:

improve string concatenation #1454 - update tests
clean up negate_iife #1451 - conflicts with fix sequences_limit #1457
fix duplicated test names #1461 - conflicts with clean up negate_iife #1451
improve keep_fargs & keep_fnames #1476 - conflicts with drop unused: toplevel, assign-only #1450
evaluate AST_SymbolRef as parameter #1477 - conflicts with clean up negate_iife #1451
enhance global_defs #1469 - conflicts with improve reduce_vars and fix a bug #1473
improve --beautify bracketize #1483 - conflicts with enhance global_defs #1469
Add .gitattributes to checkout lf eol style #1487 - conflicts with fix duplicated test names #1461
fix pure_funcs & improve side_effects #1494 - conflicts with Support marking a call as pure #1448 & clean up negate_iife #1451

/cc @avdg @kzc @rvanvelzen

alexlamsl · 2017-02-11T09:24:19Z

Not counting README.md and tests, these commits added 206 lines in total.

Tests increased by 2464 lines.

(Travis CI timed-out here. Not sure if anything should/can be done about it?)

avdg · 2017-02-11T09:51:41Z

Yeah, I should increase the timeout in #1366

avdg · 2017-02-11T09:54:53Z

Updated timeout to 15000 instead of 5000 in #1366

alexlamsl · 2017-02-11T09:59:20Z

@avdg I'm confused - how would increasing time-out period on test/mocha/portable.js fixes a time-out issue on test/mocha/number-literal.js?

Edit: ... or is it just that I'm not seeing the latest commit in #1366? 😅

alexlamsl · 2017-02-11T11:15:18Z

Tested the follow options on test/jetstream.js:

-mc warnings=false
-m
-b
-mc passes=2,warnings=false
-mc passes=3,warnings=false
-mc unsafe,warnings=false
-mc unsafe,passes=2,warnings=false
-mc unsafe,passes=3,warnings=false
-mc collapse_vars,warnings=false
-mc collapse_vars,passes=2,warnings=false
-mc collapse_vars,passes=3,warnings=false
-mc reduce_vars,warnings=false
-mc reduce_vars,passes=2,warnings=false
-mc reduce_vars,passes=3,warnings=false
-mc collapse_vars,reduce_vars,warnings=false
-mc collapse_vars,reduce_vars,passes=2,warnings=false
-mc collapse_vars,reduce_vars,passes=3,warnings=false
-mc unsafe,collapse_vars,reduce_vars,warnings=false
-mc unsafe,collapse_vars,reduce_vars,passes=2,warnings=false
-mc unsafe,collapse_vars,reduce_vars,passes=3,warnings=false
-mc unsafe,keep_fargs=0,collapse_vars,reduce_vars,warnings=false
-mc unsafe,keep_fargs=0,collapse_vars,reduce_vars,passes=2,warnings=false
-mc unsafe,keep_fargs=0,collapse_vars,reduce_vars,passes=3,warnings=false

2.7.5

https://code.jquery.com/jquery-3.1.1.js
Timing information (compressed 1 files):
- parse: 0.197s
- scope: 0.288s
- squeeze: 0.770s
- mangle: 0.031s
- generate: 0.100s

SHA1: 441e618ce79ed77d33e94a5ea8339f503d3cf892

https://code.angularjs.org/1.6.1/angular.js
Timing information (compressed 1 files):
- parse: 0.232s
- scope: 0.565s
- squeeze: 0.990s
- mangle: 0.065s
- generate: 0.184s

SHA1: b6d96dfb5a8c75641194c4c597b30a909862ab39

https://cdnjs.cloudflare.com/ajax/libs/mathjs/3.9.0/math.js
Timing information (compressed 1 files):
- parse: 0.510s
- scope: 1.560s
- squeeze: 17.244s
- mangle: 0.145s
- generate: 0.500s

SHA1: b36db92ef7c06e7b130beb7a3b01b02cbcdc2f95

https://maxcdn.bootstrapcdn.com/bootstrap/3.3.7/js/bootstrap.js
Timing information (compressed 1 files):
- parse: 0.062s
- scope: 0.118s
- squeeze: 0.206s
- mangle: 0.013s
- generate: 0.040s

SHA1: da4c56cce3d0a6abd94b4024261a3387af536408

https://unpkg.com/react@15.3.2/dist/react.js
Timing information (compressed 1 files):
- parse: 0.214s
- scope: 0.559s
- squeeze: 1.021s
- mangle: 0.053s
- generate: 0.173s

SHA1: aad6e700c1de39ad1d8310c8fd490d52f69b12c9

http://builds.emberjs.com/tags/v2.11.0/ember.prod.js
Timing information (compressed 1 files):
- parse: 0.501s
- scope: 1.286s
- squeeze: 2.187s
- mangle: 0.143s
- generate: 0.451s

SHA1: 6fe981090b52f6ff878120501d75d0f0fb345dc8

https://cdn.jsdelivr.net/lodash/4.17.4/lodash.js
Timing information (compressed 1 files):
- parse: 0.126s
- scope: 0.378s
- squeeze: 0.636s
- mangle: 0.071s
- generate: 0.108s

SHA1: 2910100ae329aae2b9d6465315cd1d1c4b6601f5

https://cdnjs.cloudflare.com/ajax/libs/d3/4.5.0/d3.js
Timing information (compressed 1 files):
- parse: 0.255s
- scope: 0.839s
- squeeze: 1.357s
- mangle: 0.197s
- generate: 0.273s

SHA1: 7fc8a34569ae383780fce638ef91680ef05fcde6

This PR

https://code.jquery.com/jquery-3.1.1.js
Timing information (compressed 1 files):
- parse: 0.121s
- scope: 0.294s
- squeeze: 0.834s
- mangle: 0.033s
- generate: 0.108s

SHA1: 7542a93fe3b60517a4e385c7cc9f14ab7f969904

https://code.angularjs.org/1.6.1/angular.js
Timing information (compressed 1 files):
- parse: 0.249s
- scope: 0.578s
- squeeze: 1.222s
- mangle: 0.067s
- generate: 0.187s

SHA1: 1d560a92dc854e35ad111ca855bc49ba12103cac

https://cdnjs.cloudflare.com/ajax/libs/mathjs/3.9.0/math.js
Timing information (compressed 1 files):
- parse: 0.555s
- scope: 1.518s
- squeeze: 18.170s
- mangle: 0.152s
- generate: 0.483s

SHA1: 09a901226439aa3273875d4e1b848261880ad08a

https://maxcdn.bootstrapcdn.com/bootstrap/3.3.7/js/bootstrap.js
Timing information (compressed 1 files):
- parse: 0.056s
- scope: 0.109s
- squeeze: 0.259s
- mangle: 0.012s
- generate: 0.039s

SHA1: 7490b7e2eb93461408c7d97a083a2ffd897a7809

https://unpkg.com/react@15.3.2/dist/react.js
Timing information (compressed 1 files):
- parse: 0.209s
- scope: 0.518s
- squeeze: 1.289s
- mangle: 0.051s
- generate: 0.177s

SHA1: e34ee5dd004d1db6f2582d2d17d8cee1c430a4b0

http://builds.emberjs.com/tags/v2.11.0/ember.prod.js
Timing information (compressed 1 files):
- parse: 0.497s
- scope: 1.296s
- squeeze: 2.611s
- mangle: 0.143s
- generate: 0.477s

SHA1: 36efff4cbe2072d639ed1fe36141827af7e95670

https://cdn.jsdelivr.net/lodash/4.17.4/lodash.js
Timing information (compressed 1 files):
- parse: 0.127s
- scope: 0.279s
- squeeze: 0.712s
- mangle: 0.066s
- generate: 0.112s

SHA1: 4e24e6f70e574b06afbf5e8654740a80ee97d6b9

https://cdnjs.cloudflare.com/ajax/libs/d3/4.5.0/d3.js
Timing information (compressed 1 files):
- parse: 0.249s
- scope: 0.786s
- squeeze: 1.700s
- mangle: 0.160s
- generate: 0.266s

SHA1: 06439dfb30a7ec469d4d6f250f90a06b32b65657

alexlamsl · 2017-02-11T11:54:14Z

uglifyjs -mc

File	Original	2.7.5	This PR
angular-1.6.1.js	1,232,632	174,174	174,065
boostrap.js	69,707	36,920	36,901
d3.js	451,131	214,137	214,126
ember.prod.js	1,852,178	530,719	530,437
jquery-3.1.1.js	267,194	87,192	87,062
lodash.js	539,590	71,380	71,377
math.js	1,590,107	470,543	470,355
react.js	701,412	207,922	207,781

avdg · 2017-02-11T12:29:52Z

uh, maybe I didn't do a complete check. The fix was suppose to solve 1 known problem anyway.

avdg · 2017-02-11T12:30:52Z

See https://travis-ci.org/mishoo/UglifyJS2/jobs/200607597#L772

alexlamsl · 2017-02-11T12:40:10Z

@avdg you are right and I'm just terminally confused 😅

FWIW, I did a search of Should not allow legacy octal literals in strict mode across the code base and test/mocha/number-literal.js is the only file that has it. But yes, as you stated it's also under the heading of portable which is where the this.timeout(5000); is at.

kzc · 2017-02-11T17:06:57Z

@alexlamsl AFAIK only #1451 conflicts with the other pending PRs.

If you changed this PR's title to "2.8.0 staging" then @rvanvelzen could simply merge this for the next release. Mind you, if we did that then the underlying PRs could no longer be updated as changes would not be picked up.

alexlamsl · 2017-02-11T17:30:59Z

@kzc may be due you used 3-way merge whereas I did git merge --squash which probably didn't.

I was thinking @rvanvelzen would do the merge, then compare notes with this PR so we can both catch any problems if they turn out to be different. Let's just say my git-fu is not at the level where I can confidently say I did everything right. 😅

kzc · 2017-02-11T17:51:37Z

Unrelated: I think you could add pure_getters and unsafe_comps to your list of options to test with jetstream:

-mc unsafe,unsafe_comps,pure_getters,keep_fargs=0,collapse_vars,reduce_vars,passes=3,warnings=false

alexlamsl · 2017-02-11T18:06:37Z

Also unrelated: in manually going over the files, so far I noticed when reduce_vars eventually causes a variable to be unused which is being assigned a RegExp as value, we will be left with the value as it is now considered to .has_side_effects().

alexlamsl · 2017-02-11T18:21:57Z

@kzc your suggested options (& two passes variants) have completed successfully 😉

alexlamsl · 2017-02-11T18:28:13Z

#1485 (comment) boils down to this:

echo 'function f(){var a=/b/;}' | uglifyjs -c

2.7.5 gives:

WARN: Dropping unused variable a [-:1,17]
function f(){}

This PR gives:

WARN: Side effects in initialization of unused variable a [-:1,17]
function f(){/b/}

kzc · 2017-02-11T18:34:10Z

RegExp literals are tricky. Unlike strings they are not truly constant - they are mutable and hold state. I'd rather err on the side of caution here rather than risk generating bad code.

kzc · 2017-02-11T18:37:41Z

        (function(){
            var result, rx = /ab*/g;
            while (result = rx.exec('acdabcdeabbb'))
                console.log(result[0]);
        })();

alexlamsl · 2017-02-11T18:38:25Z

Agreed, even though I was hoping to have some test failures when I remove def(AST_RegExp, return_true); from has_side_effects(). 😅

FWIW, the corner cases of size regression I've hit can be recovered with that line removed. Which means optimising AST_RegExp properly will now be on my TODO list. 👻

alexlamsl · 2017-02-11T18:46:54Z

Looking at AST_Object & AST_Function though:

def(AST_Function, return_false);

def(AST_Object, function(compressor){
    for (var i = this.properties.length; --i >= 0;)
        if (this.properties[i].has_side_effects(compressor))
            return true;
    return false;
});

Wouldn't AST_RegExp seem more restricted by comparison?

kzc · 2017-02-11T18:53:02Z

Technically a RegExp literal is shorthand for new RegExp(...).

$ echo 'function f(){var a=new RegExp("b");}' | uglifyjs -c pure_funcs='["RegExp"]'
WARN: Dropping unused variable a [-:1,17]
function f(){}

new RegExp() or a RegExp literal can be considered to be a pure function if otherwise unused. Perhaps the unused optimization can codify that specific rule.

alexlamsl · 2017-02-11T19:08:13Z

Throwing this at uglifyjs -c:

function f() {
  var a = {
    lastIndex: -1,
    flags: "g",
    source: "ab*",
    exec: function(s) {
      return s;
    }
  };
}

... yields function f(){}. I'm still a bit stuck on why /ab*/g is different from this object. 😕

kzc · 2017-02-11T19:15:12Z

I was hoping to have some test failures when I remove def(AST_RegExp, return_true); from has_side_effects().

I suggested that addition just to be on the safe side. !is_constant() takes precedence over has_side_effects() in collapse_vars, but I wasn't sure other AST_Constant optimization code would have the same safeguards for AST_RegExp.

$ uglifyjs -V
uglify-js 2.7.5

$ echo 'function f(){var a=/b/;}' | uglifyjs -c
WARN: Dropping unused variable a [-:1,17]
function f(){}

But since 2.7.5 correctly drops the unused RegExp literal, perhaps this PR is too restrictive and def(AST_RegExp, return_true); should be dropped from has_side_effects(). A test should be added to catch this regression.

kzc · 2017-02-11T19:21:29Z

I'm still a bit stuck on why /ab*/g is different from this object.

If has_side_effects() once again returns false for AST_RegExp does that solve your issue?

kzc · 2017-02-11T19:23:18Z

I'll remove the line in question from #1459

alexlamsl · 2017-02-11T19:26:53Z

If has_side_effects() once again returns false for AST_RegExp does that solve your issue?

Yes, removing that line makes it hit the AST_Constant case which returns false and recovers back 2.7.5 behaviour.

I'll remove the line in question from #1459

Thanks - I suppose we can add the tests you suggested over there as well.

kzc · 2017-02-11T19:39:32Z

@alexlamsl Unused RegExp literal regression addressed in #1459 and new test added. Thanks for catching that.

alexlamsl · 2017-02-11T19:41:45Z

Right, with two PRs updated, let me run through this merge exercise again... 😎

- `do{...}while(false)` => `{...}` - clean up `AST_While` logic closes mishoo#1452

N = 2: a; b; c; d; was: a, b; c; d; now: a, b; c, d; fixes mishoo#1455 closes mishoo#1457

shuffle associative operations to minimise parentheses and aid other uglification efforts closes mishoo#1454

- remove extra tree scanning phase for `negate_iife` - `negate_iife` now only deals with the narrowest form, i.e. IIFE sitting directly under `AST_SimpleStatement` - `booleans`, `conditionals` etc. will now take care the rest via more accurate accounting - `a(); void b();` => `a(); b();` fixes mishoo#1288 closes mishoo#1451

closes mishoo#1459

previously test cases with the same name would be skipped except for the last one `test/run-test.js` will now report duplicated names as errors closes mishoo#1461

- utilise in_use_ids instead of unreferenced() - drop_unused now up-to-date for subsequent passes closes mishoo#1476

- update modified flag between compress() passes - support IIFE arguments - fix corner case with multiple definitions closes mishoo#1473

fix invalid boolean conversion now exposed in `make_node_from_constant()` closes mishoo#1477

- support arrays, objects & AST_Node - support `"a.b":1` on both cli & API - emit warning if variable is modified - override top-level variables fixes mishoo#1416 closes mishoo#1198 closes mishoo#1469

- `test/benchmark.js` measures performance - `test/jetstream.js` verifies correctness - configurable mangle/compress/output options closes mishoo#1479

reduce whitespaces from if-else statements fixes mishoo#1482 closes mishoo#1483

closes mishoo#1487

fixes mishoo#1377 closes mishoo#1488

- `Array.prototype.slice` => `[].slice` closes mishoo#1491

- never exceed specified limit - otherwise warning is shown - enabled only for final output closes mishoo#1496

- only drops side-effect-free arguments - drop side-effect-free parts with discarded value from `AST_Seq` & `AST_SimpleStatement` closes mishoo#1494

fixes mishoo#1492 closes mishoo#1493

A function call or IIFE with an immediately preceding comment containing `@__PURE__` or `#__PURE__` is deemed to be a side-effect-free pure function call and can potentially be dropped. Depends on `side_effects` option. `[#@]__PURE__` hint will be removed from comment when pure call is dropped. fixes mishoo#1261 closes mishoo#1448

alexlamsl · 2017-02-21T06:30:02Z

@kzc #1448 re-merged alongside updated version of 01b8313

I have moved has_pure_annotation() and the associated warning around to facilitate code reuse in drop_side_effect_free().

@const

- fix corner cases in `const` optimisation - deprecate `/*@const*/` fixes mishoo#1497 closes mishoo#1498

alexlamsl · 2017-02-23T17:52:18Z

@kzc in merging #1498 I've noticed #1459 would suffer the same issue as described in #1497.

I have applied a fix which would make #1459 dependent on reduce_vars - PTAL.

kzc · 2017-02-23T20:32:34Z

@alexlamsl Here's the thing - const variable reassignment really ought to throw an error well before we get to the #1459 optimization - otherwise we need to have redundant wasteful checking all over the place. I would not be surprised if there are other optimizations we've missed. But as long as reduce_vars is on by default, I can live with it. Your suggestion in #1459 (comment) make sense.

kzc · 2017-02-24T00:06:53Z

@alexlamsl Thanks for getting all these features merged into master!
(I don't envy the person who will merge this into harmony ;-)

Now all you need is npm rights to create a new release.

/cc @mishoo @rvanvelzen

alexlamsl closed this Feb 11, 2017

alexlamsl reopened this Feb 11, 2017

alexlamsl and others added 18 commits February 21, 2017 13:29

tweak do-while loops

ae4db00

- `do{...}while(false)` => `{...}` - clean up `AST_While` logic closes mishoo#1452

-c sequences=N suboptimal at N expression cutoff

f584ca8

N = 2: a; b; c; d; was: a, b; c; d; now: a, b; c, d; fixes mishoo#1455 closes mishoo#1457

improve string concatenation

6b3c49e

shuffle associative operations to minimise parentheses and aid other uglification efforts closes mishoo#1454

smarter const replacement taking name length into account

6ffbecb

closes mishoo#1459

fix duplicated test names

c525a2b

previously test cases with the same name would be skipped except for the last one `test/run-test.js` will now report duplicated names as errors closes mishoo#1461

improve keep_fargs & keep_fnames

b8b133d

- utilise in_use_ids instead of unreferenced() - drop_unused now up-to-date for subsequent passes closes mishoo#1476

improve reduce_vars and fix a bug

a0f4fd3

- update modified flag between compress() passes - support IIFE arguments - fix corner case with multiple definitions closes mishoo#1473

evaluate AST_SymbolRef as parameter

974247c

fix invalid boolean conversion now exposed in `make_node_from_constant()` closes mishoo#1477

enhance global_defs

e275148

- support arrays, objects & AST_Node - support `"a.b":1` on both cli & API - emit warning if variable is modified - override top-level variables fixes mishoo#1416 closes mishoo#1198 closes mishoo#1469

add benchmark & JetStream tests

7e6331b

- `test/benchmark.js` measures performance - `test/jetstream.js` verifies correctness - configurable mangle/compress/output options closes mishoo#1479

improve --beautify bracketize

09f9ae2

reduce whitespaces from if-else statements fixes mishoo#1482 closes mishoo#1483

Add .gitattributes to checkout lf eol style

c06a50f

closes mishoo#1487

remove extraneous spaces between ++/+/--/-

ac0b61e

fixes mishoo#1377 closes mishoo#1488

introduce unsafe_proto

ec64acd

- `Array.prototype.slice` => `[].slice` closes mishoo#1491

clean up max_line_len

8898b8a

- never exceed specified limit - otherwise warning is shown - enabled only for final output closes mishoo#1496

fix pure_funcs & improve side_effects

26fbeec

- only drops side-effect-free arguments - drop side-effect-free parts with discarded value from `AST_Seq` & `AST_SimpleStatement` closes mishoo#1494

Fix: AST_Accessor missing start / end tokens

d48a308

fixes mishoo#1492 closes mishoo#1493

alexlamsl force-pushed the merge-2.8.0 branch from 89e94b2 to d48a308 Compare February 21, 2017 05:36

alexlamsl mentioned this pull request Feb 21, 2017

Support marking a call as pure #1448

Closed

kzc mentioned this pull request Feb 21, 2017

@const creeping #1497

Closed

enable collapse_vars & reduce_vars by default

4e49302

- fix corner cases in `const` optimisation - deprecate `/*@const*/` fixes mishoo#1497 closes mishoo#1498

alexlamsl mentioned this pull request Feb 23, 2017

extend smart const replacement to variables #1501

Closed

alexlamsl merged commit 229e42c into mishoo:master Feb 23, 2017

alexlamsl deleted the merge-2.8.0 branch February 24, 2017 00:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

2.8.0 staging #1485

2.8.0 staging #1485

alexlamsl commented Feb 11, 2017 •

edited

Loading

alexlamsl commented Feb 11, 2017 •

edited

Loading

avdg commented Feb 11, 2017

avdg commented Feb 11, 2017

alexlamsl commented Feb 11, 2017 •

edited

Loading

alexlamsl commented Feb 11, 2017 •

edited

Loading

alexlamsl commented Feb 11, 2017 •

edited

Loading

alexlamsl commented Feb 11, 2017 •

edited

Loading

avdg commented Feb 11, 2017

avdg commented Feb 11, 2017

alexlamsl commented Feb 11, 2017

kzc commented Feb 11, 2017

alexlamsl commented Feb 11, 2017

kzc commented Feb 11, 2017

alexlamsl commented Feb 11, 2017

alexlamsl commented Feb 11, 2017 •

edited

Loading

alexlamsl commented Feb 11, 2017

kzc commented Feb 11, 2017

kzc commented Feb 11, 2017

alexlamsl commented Feb 11, 2017

alexlamsl commented Feb 11, 2017

kzc commented Feb 11, 2017

alexlamsl commented Feb 11, 2017

kzc commented Feb 11, 2017

kzc commented Feb 11, 2017

kzc commented Feb 11, 2017

alexlamsl commented Feb 11, 2017

kzc commented Feb 11, 2017 •

edited

Loading

alexlamsl commented Feb 11, 2017

alexlamsl commented Feb 21, 2017

alexlamsl commented Feb 23, 2017

kzc commented Feb 23, 2017

kzc commented Feb 24, 2017

2.8.0 staging #1485

2.8.0 staging #1485

Conversation

alexlamsl commented Feb 11, 2017 • edited Loading

alexlamsl commented Feb 11, 2017 • edited Loading

avdg commented Feb 11, 2017

avdg commented Feb 11, 2017

alexlamsl commented Feb 11, 2017 • edited Loading

alexlamsl commented Feb 11, 2017 • edited Loading

alexlamsl commented Feb 11, 2017 • edited Loading

2.7.5

This PR

alexlamsl commented Feb 11, 2017 • edited Loading

avdg commented Feb 11, 2017

avdg commented Feb 11, 2017

alexlamsl commented Feb 11, 2017

kzc commented Feb 11, 2017

alexlamsl commented Feb 11, 2017

kzc commented Feb 11, 2017

alexlamsl commented Feb 11, 2017

alexlamsl commented Feb 11, 2017 • edited Loading

alexlamsl commented Feb 11, 2017

kzc commented Feb 11, 2017

kzc commented Feb 11, 2017

alexlamsl commented Feb 11, 2017

alexlamsl commented Feb 11, 2017

kzc commented Feb 11, 2017

alexlamsl commented Feb 11, 2017

kzc commented Feb 11, 2017

kzc commented Feb 11, 2017

kzc commented Feb 11, 2017

alexlamsl commented Feb 11, 2017

kzc commented Feb 11, 2017 • edited Loading

alexlamsl commented Feb 11, 2017

alexlamsl commented Feb 21, 2017

alexlamsl commented Feb 23, 2017

kzc commented Feb 23, 2017

kzc commented Feb 24, 2017

alexlamsl commented Feb 11, 2017 •

edited

Loading

alexlamsl commented Feb 11, 2017 •

edited

Loading

alexlamsl commented Feb 11, 2017 •

edited

Loading

alexlamsl commented Feb 11, 2017 •

edited

Loading

alexlamsl commented Feb 11, 2017 •

edited

Loading

alexlamsl commented Feb 11, 2017 •

edited

Loading

alexlamsl commented Feb 11, 2017 •

edited

Loading

kzc commented Feb 11, 2017 •

edited

Loading