Skip to content

No outlining? / counterproductive inlining? #3203

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

Open
juj opened this issue Jan 19, 2019 · 5 comments
Open

No outlining? / counterproductive inlining? #3203

juj opened this issue Jan 19, 2019 · 5 comments
Labels
internal-issue-created An internal Google issue has been created to track this GitHub issue triage-done Has been reviewed by someone on triage rotation.

Comments

@juj
Copy link

juj commented Jan 19, 2019

Consider

A:

function foo(timeStamp) {
  console.log(timeStamp);
}

function emscripten_request_animation_frame_loop(cb) {
  function tick(timeStamp) {
    cb(timeStamp);
    requestAnimationFrame(tick);
  }
  requestAnimationFrame(tick);
}

emscripten_request_animation_frame_loop(foo);

this minifies down to

1:

(function(a){function b(c){a(c);requestAnimationFrame(b)}requestAnimationFrame(b)})(function(a){console.log(a)});

which is 113 bytes. Beautified to be more readable:

(function(a) {
    function b(c) {
        a(c);
        requestAnimationFrame(b)
    }
    requestAnimationFrame(b)
})(function(a) {
    console.log(a)
});

the extern browser API function requestAnimationFrame is sufficiently long, that it would be beneficial to outline access to that function. That is, one can hand-write the following equivalent minified code:

2:

function d(a){requestAnimationFrame(a)}(function(a){function b(c){a(c);d(b)}d(b)})(function(a){console.log(a)});

which is 112 bytes, winning by one character. Beautified for better readability:

function d(a) {
    requestAnimationFrame(a)
}(function(a) {
    function b(c) {
        a(c);
        d(b)
    }
    d(b)
})(function(a) {
    console.log(a)
});

I.e. if access to requestAnimationFrame() is outlined to a function of its own that is minified, then it can be accessed via a minified name to net a size saving. Here the saving is only one byte, but if across the whole program there were more accesses to requestAnimationFrame(), the saving would be more considerable.

Of course there is an extra function call indirection, which might mean lower performance, so not sure if this is something that would always be better, but perhaps a tradeoff call. However, if I manually outline, i.e. had hand-written the following JS code to start with:

B:

function raf(f) {
	return requestAnimationFrame(f);
}

function foo(timeStamp) {
  console.log(timeStamp);
}

function emscripten_request_animation_frame_loop(cb) {
  function tick(timeStamp) {
    cb(timeStamp);
    raf(tick);
  }
  raf(tick);
}

emscripten_request_animation_frame_loop(foo);

then it seems like an incorrect call for Closure to inline function raf, because the resulting program will be bigger by one character than the version where raf was not inlined. So in this case, one might expect to get version 2 instead of 1 to retain the small size? However the above code does generate 1, instead of 2.

The above raises two questions to mind:

  • Does Closure ever outline functions if that would produce a net win in code size? If not, I'd like to manually identify places where I should outline, like the requestAnimationFrame() case above.
  • I wonder if the fact that it inlined version B was just a fluke of this example because of the really close call in sizes between the two versions (112 vs 113 bytes)? Now Closure is undoing my own manual outlining for a counterproductive effect. Is there a way I could tell Closure to not inline my function?
@rishipal
Copy link
Contributor

For your second question, I believe there was a @noinline annotation introduced for preventing inlining. In your specific example, perhaps it can be used as an annotation with function raf(f).
Discussion here - (#2751)

@lauraharker
Copy link
Contributor

Created Google internal issue http://b/123244691

@lauraharker lauraharker added triage-done Has been reviewed by someone on triage rotation. internal-issue-created An internal Google issue has been created to track this GitHub issue labels Jan 22, 2019
@lauraharker
Copy link
Contributor

re 1: No, Closure doesn't currently 'outline' functions. In this case I'm not sure that would be something we want to do: Closure prefers to optimize for post-gzip size, even when that means pre-gzip size is larger. And gzip (and other compression tools) are generally good at handling long repeated strings. The gzipped size of the Closure example is less than the handwritten example (95 bytes vs. 106 bytes)

re 2: As with your original question, I'd expect inlining raf helps post-gzip size. If you don't care about gzip then you can use the linked @noinline annotation that @rishipal linked.

If you're interested, there's another rarely-used Closure option to alias long strings in the code to short variables. This is not on by default inside Google, as most of the time gzip does a better job with the strings, but a few projects have found it helps them:
https://github.com/google/closure-compiler/blob/master/src/com/google/javascript/jscomp/AliasStrings.java

@juj
Copy link
Author

juj commented Dec 6, 2019

Coming back to this after a while.. How do I enable the AliasStrings pass? I tried looking at the documentation at https://github.com/google/closure-compiler/wiki/Flags-and-Options , searching for "pass" or "alias", but was unsure how to enable it.

I agree that outlining automatically is probably not a smart thing to do. In my case, using a @noinline annotation is difficult, since I cannot statically know if a function should be noinline: if there is only one use of the function, then it should be inlineable, but if there happen to be multiple uses, then it should not be inlined.

I do not place much value to estimating post-gzip sizes, since that is dangerous to do especially on these kind of tiny examples. Even if a really small example compresses slightly better, there is no guarantee that in a large page it would behave the same. (the only cases I care about post-gzip sizes are when there is an "entropy proof" that can be applied to the reasoning, e.g. "always use single-quotes and not a mix of single and double quotes", and so on)

@juj
Copy link
Author

juj commented Mar 26, 2021

Optimizing Emscripten WebGL support library in emscripten-core/emscripten#13732 today, and still affected by this issue. In today's scenario, I have

function getLeftBracePos(name) {
      return name.slice(-1) == ']' && name.lastIndexOf('[');
    }
...
var leftBrace = getLeftBracePos(nm);
...
leftBrace = getLeftBracePos(name);

where Closure wants to inline getLeftBracePos, leading to larger code.

Explicitly passing /** @noinline */ causes Closure to not do the counterproductive inlining. That is great, but in general it is a somewhat lucky scenario when one happens to notice when Closure did this - auditing all code can be infeasible and most of these occurrences probably go unnoticed :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal-issue-created An internal Google issue has been created to track this GitHub issue triage-done Has been reviewed by someone on triage rotation.
Projects
None yet
Development

No branches or pull requests

3 participants