HoriSun / closure-compiler

Automatically exported from code.google.com/p/closure-compiler
0 stars 0 forks source link

Optimize totally struct when possible #1186

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
I get this code after try some code bigger to show this issue. 
But I noted that this small code is better to show the optimization issue.

    /** @constructor */
    var emptyClass = function() { };

    /** @type {emptyClass} */
    var emptyInstance = new emptyClass;
    /** @type {function()} */
    var callback;

    /** @type {function()} */
    emptyInstance.unknowVariable = function() {
        alert("Hello World!");
    };

    callback = emptyInstance.unknowVariable;
    callback();

What do you expect with this? 
Basically it just call the alert.

Some notes:

 * On first optimization the `unknowVariable` was created needless here, but let us just ignore it for now.

 1. The `emptyClass` constructor now was exposed, so it name is optimizable, fine?
 2. So we create the `emptyInstance` of `emptyClass` here, no sense, right?
 3. The `callback` will store some callback to call, last.
 4. Now the optimizer create this `unknowVariable` to set my real callback function.
 5. Now it set the `callback` with last value. But why? Why it just don't set directly to this variable?
 6. Finally it call.

The major problem is that optimizer not checked if `emptyInstance` and 
`emptyClass` really need to exists, and create it for nothing! Nothing was set 
as `@expose` on code.

What version of the product are you using? On what operating system?
- v20131118

Original issue reported on code.google.com by david.pr...@gmail.com on 22 Dec 2013 at 3:12

GoogleCodeExporter commented 9 years ago
1. The `emptyClass` constructor **not** was exposed, so it name is optimizable, 
fine?

Original comment by david.pr...@gmail.com on 22 Dec 2013 at 3:22

GoogleCodeExporter commented 9 years ago
in general, I don't think that it's worth optimizing the case where people add 
methods to one-off instances. It would require an expensive analysis that would 
rarely be helpful. See:
https://code.google.com/p/closure-compiler/wiki/FAQ#How_do_I_submit_a_feature_re
quest_for_a_new_type_of_optimization

Original comment by Nicholas.J.Santos on 27 Dec 2013 at 3:34

GoogleCodeExporter commented 9 years ago
Well, on this specific case, this code was generated by a previous compilation, 
so it is very strange. For a small code, it can be very superfluous, really. 
But if you consider make it on jQuery (if this one day is able to build on 
advanced mode), it can make a big difference.

But on this case, we can see a visible needless code, that performs about 80% 
of compression rate (105 bytes gzipped vs 22 bytes gzipped, excluding the 
header), plus additional memory usage and processing for dead code.

I also do not think it will be hard to optimize. I also do not know exactly how 
the code works inside [of CC], but a very superficial analysis of what could be 
done in this case:

 - `emptyClass` is a function/constructor initialized an UNIQUE time, so we can set it directly via `new function()`;
 - `emptyInstance` is the UNIQUE instance of `emptyClass` above, this complements what I ended up talking. Shortcut:
   var emptyInstance = new function() { };
 - `emptyInstance.unknowVariable` is a variable setted ONLY and UNIQUE time with the real function...;
 - and it is setted DIRECTLY to never used and local scopped variable `callback`. Shortcut:
   var callback = function() { alert("Hello World") };
 - On this case, no make more sense to exists `emptyInstance`, so, we can remove it from code;
 - Finally the `callback` is executed directly, without `.apply` or `.call` or additional arguments. Shortcut:
   (function() { alert("Hello World"); })();
 - So, what we can do it on this last case? UNIQUE execution, UNIQUE code:
   alert("Hello World");

Original comment by david.pr...@gmail.com on 27 Dec 2013 at 4:09