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

enhance collapse_vars #4864

Merged
merged 1 commit into from
Apr 24, 2021
Merged

enhance collapse_vars #4864

merged 1 commit into from
Apr 24, 2021

Conversation

alexlamsl
Copy link
Collaborator

No description provided.

@alexlamsl
Copy link
Collaborator Author

@kzc this should round up #4858 (comment)

$ cat test.js
!function() {
  var e, t, r = {},
    n = {};
  t = n = function(e, t) {
    return t.get ? t.get.call(e) : t.value
  }, n.default = t, n.__esModule = true;
  var u, a = n,
    o = {};
  u = o = function(e, t, r) {
    if (!t.has(e)) throw new TypeError("attempted to " + r + " private field on non-instance");
    return t.get(e)
  }, o.default = u, o.__esModule = true;
  var l = o;
  e = r = function(e, t) {
    var r = l(e, t, "get");
    return a(e, r)
  }, r.default = e, r.__esModule = true
}();
$ uglify-js test.js -bc
!function() {
    var r;
    o = function(e, t) {
        return t.get ? t.get.call(e) : t.value;
    };
    (o.default = o).__esModule = !0;
    var o, a = o;
    o = function(e, t, r) {
        if (!t.has(e)) throw new TypeError("attempted to " + r + " private field on non-instance");
        return t.get(e);
    };
    (o.default = o).__esModule = !0;
    var l = o;
    r = function(e, r) {
        r = l(e, r, "get");
        return a(e, r);
    };
    (r.default = r).__esModule = !0;
}();
$ uglify-js test.js -bc passes=2
 

(also works with pure_getters=true)

@alexlamsl alexlamsl merged commit 10dd9d4 into mishoo:master Apr 24, 2021
@alexlamsl alexlamsl deleted the collapse_vars branch April 24, 2021 04:45
@kzc
Copy link
Contributor

kzc commented Apr 24, 2021

Wow, that was quick. Well done.

So the gist of it is that you continue to scan past side effects for other possible aliases?

@kzc
Copy link
Contributor

kzc commented Apr 24, 2021

So merge_vars doesn't interfere with this optimization? Or this optimization works even in spite of the repurposing of variables?

@alexlamsl
Copy link
Collaborator Author

So the gist of it is that you continue to scan past side effects for other possible aliases?

We already have value_def mode in collapse_vars − nothing inherently unsafe as:

  • a = b can propagate past more expression types than a = ... in general
  • should be full substitution when possible for best optimisation

This PR extends it to recognise a = b = ...

So merge_vars doesn't interfere with this optimization?

collapse_vars by its nature is very narrow-sighted, so doesn't really interact with merge_vars much unlike say reduce_vars − I just assumed you turned it off for easier visualisation & deduction, like how I normally go for --rename -cb braces 👻

@kzc
Copy link
Contributor

kzc commented Apr 24, 2021

collapse_vars by its nature is very narrow-sighted, so doesn't really interact with merge_vars much unlike say reduce_vars − I just assumed you turned it off for easier visualisation & deduction

The reason why I mentioned merge_vars is illustrated in the following code produced by uglify-js@v3.13.4 just prior to your recent optimizations:

$ cat merge_vars.js 
!function() {
    var e = n = function(e, n) {
        return n.get ? n.get.call(e) : n.value;
    };
    n.default = e, n.__esModule = !0;
    var t = n, n = e = function(e, n, t) {
        if (!n.has(e)) {
            throw new TypeError("attempted to " + t + " private field on non-instance");
        }
        return n.get(e);
    };
    e.default = n, e.__esModule = !0;
    var a = e, e = n = function(e, n) {
        return n = a(e, n, "get"), t(e, n);
    };
    n.default = e, n.__esModule = !0, new WeakMap(), new WeakSet(), async function() {
        console.log("hi");
    }();
}();

Notice that e and n were reused throughout the code by merge_vars.

If I take that output and run it through latest master it produces:

$ cat merge_vars.js | uglify-js --rename -b braces -c passes=9,unsafe
!function() {
    var a = c = function(e, f) {
        return f.get ? f.get.call(e) : f.value;
    };
    c.default = a, c.__esModule = !0;
    var b = c, c = a = function(g, h, i) {
        if (!h.has(g)) {
            throw new TypeError("attempted to " + i + " private field on non-instance");
        }
        return h.get(g);
    };
    a.default = c, a.__esModule = !0;
    var d = a, a = c = function(j, k) {
        return k = d(j, k, "get"), b(j, k);
    };
    c.default = a, c.__esModule = !0, async function() {
        console.log("hi");
    }();
}();

The repurposing of variables by merge_vars precludes the recent optimizations in collapse_vars.

So I was a bit surprised that enhancing collapse_vars alone without changing merge_vars would effectively reduce the original code. Perhaps the order of the compress optimizations was significant in that case.

I don't think the last output above can be optimized away very easily without using SSA in rename.

@alexlamsl
Copy link
Collaborator Author

Oh I see what you mean − indeed merge_vars is run after optimize() & tighten_body() within a given AST_Scope, much the same way unused is done.

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