Optimization regression - excess function parameters are no longer optimized out

[juj]

src.js

function foo(a, b) {
	console.log(a);
}
setTimeout(foo, 0); // pin a reference to foo so it does not get optimized out
foo("hello", "this_is_redundant");

Old Closure:

C:\emsdk\emscripten\incoming>java -jar third_party\closure-compiler_old\compiler.jar --version
Closure Compiler (http://github.com/google/closure-compiler)
Version: v20171023
Built on: 2017-10-26 19:00

C:\emsdk\emscripten\incoming>java -jar third_party\closure-compiler_old\compiler.jar --js=src.js --js_output_file=out.js --compilation_level=ADVANCED_OPTIMIZATIONS

C:\emsdk\emscripten\incoming>type out.js
function a(b){console.log(b)}setTimeout(a,0);a("hello");

Closure optimized out the unnecessary "this_is_redundant" input string.

New Closure:

C:\emsdk\emscripten\incoming>java -jar third_party\closure-compiler\compiler.jar --version
Closure Compiler (http://github.com/google/closure-compiler)
Version: v20181210
Built on: 2018-12-12 22:32

C:\emsdk\emscripten\incoming>java -jar third_party\closure-compiler\compiler.jar --js=src.js --js_output_file=out.js --compilation_level=ADVANCED_OPTIMIZATIONS

C:\code\emsdk\emscripten\incoming>type out.js
function a(b){console.log(b)}setTimeout(a,0);a("hello","this_is_redundant");

the optimization unfortunately no longer occurs, but "this_is_redundant" appears in the output.

In new Closure, the optimization does not occur in this case either:

src2.js

function foo(a) {
	console.log(a);
}
setTimeout(foo, 0); // pin a reference to foo so it does not get optimized out
foo("hello", "this_is_redundant");

although a warning is issued. In old Closure, this version was also optimized.

Curiously, new Closure does still optimize this variant of the code, where input parameters are reversed:

src3.js

function foo(a, b) {
	console.log(b);
}
setTimeout(foo, 0); // pin a reference to foo so it does not get optimized out
foo("this_is_redundant", "hello");

becomes

setTimeout(function(b,a){console.log(a)},0);console.log("hello");

so if a redundant parameter appears first, it does get optimized. But if it appears last, it no longer does.

[EatingW]

Created Google internal issue http://b/122893201

[EatingW]

I think having advanced optimizations would optimize this for you, see here.

I believe the passes you need to turn on are Inline Functions, Inline Variables, Remove Unused Variables, and Variable Renaming

[juj]

Hmm, I was running with --compilation_level=ADVANCED_OPTIMIZATIONS (see above, that part scrolls out off to the right side of the page).

On the test page I do see the optimization carried out. I wonder if I need to enable some custom command line flag in addition to --compilation_level=ADVANCED_OPTIMIZATIONS to turn on those passes? Or should ADVANCED_OPTIMIZATIONS already do those passes?

[EatingW]

ADVANCED_OPTIMIZATIONS should already have these flag. @lauraharker helped me investigate this a bit and discovered that while my example with alert() works above, your example with console.log() does not, which is very surprising.

[lauraharker]

I did a bit more digging and found that:

• the reason src3.js and Yiting's alert example are removed, but not the other examples, is because of function inlining's cost metric. The compiler believes that inlining would help code size in those cases but not the others.

• The pass OptimizeParameters is responsible for removing unused parameters and arguments, and would (normally) remove "this is redundant".
  In general, because the pass sometimes reorders parameters, it backs off optimizing any function that escapes, because the pass cannot safely reorder arguments at all the call sites. In this example foo escapes into the alert function so the pass does not optimize "this_is_redundant".
  However, I think it would still to rewrite foo in this case. Only reordering parameters could break call sites; removing trailing parameters and arguments should be safe.

I'm not sure what caused the change from v20171023. It could have been the function inlining cost metric or it could have been OptimizeCalls becoming more conservative.