An annotation to either prevent or force inlining of a function

[gaearon]

We've been using GCC in the simple mode for compiling React for a few months, and are really happy with its heuristics. However, recently I've been noticing a minor pain point.

Problem

In some cases, as library authors we want to have more control over whether a function should be inlined or not. In particular, there are two scenarios:

1. Some functions are in a very hot path, and we have determined that not inlining them would hurt the runtime performance. Usually at this point we just inline them manually in the code. However, that often produces duplicate code that is hard to work with. Ideally, we'd love to have a way to tell GCC that it has to either inline a function or fail the build (if it’s impossible). This way we know that we won’t regress on this particular function.

2. There is an opposite problem as well. Sometimes we split a function into several to have tighter control over the argument types. For example, if some path is very hot, and we need to ensure a function is called with monomorphic arguments. However, GCC can break these optimizations by inlining the separated function (and thus preventing engines from optimizing that code due to deopts). For this use case, we'd like to have a way to tell GCC to not inline a function, even if the resulting code size will be larger.

Prior Art

The other GCC 🙂

• inline and always_inline
• noinline

Proposal

New @inline annotation forces inlining

/** @inline */
function someHelper() {
  // ...
}

// We want to fail the build if these couldn't be inlined.
someHelper();
someHelper();

This forces the function to be inlined even if the code size becomes bigger.
If it can’t be inlined, the build fails.

Alternative possible naming: @always-inline, @force-inline.

New @noinline annotation prevents inlining

/** @noinline */
function doSomethingWithString(stringValue) {
  // ...
}

/** @noinline */
function doSomethingWithNumber(numberValue) {
  // ...
}

function doSomething(mixedValue) {
  // It is important that these calls never get inlined
  if (typeof mixedValue === 'string') {
    doSomethingWithString(mixedValue);
  } else if (typeof mixedValue === 'number') {
    doSomethingWithNumber(mixedValue);
  }
}

This forces a function to never be considered for inlining, even if there is a size win.

Alternative possible naming: @never-inline, @prevent-inline.

Other hint formats?

I don't have a strong opinion about how to supply these hints. A comment, a JSDoc annotation, special function name, etc, would all work for me. Please let me know if this is something you would consider!

Current Workarounds

In case anyone’s curious, I’ve thought a bit about how to achieve this today.

Forcing inlining

There's no way to force inlining, but we could plausibly adopt a special naming convention (e.g. *Inline). We already have a special build where we disable the renaming pass (#2707) so we could analyze the bundle, and fail the build if any *Inline function still exists. This wouldn’t force GCC to inline it, but at least we’d know if it was inlined but then started bailing.

Preventing inlining

This file might contain some hints. For example attaching function to an object would prevent inlining that function.

Of course it would be nice to have an official solution for this, as both options are hacky and incomplete.

[ChadKillingsworth]

We very much shy away from annotations which direct optimizations. They tend to be major footguns. In general we'd like to improve the compiler heuristics around inlining.

You can easily block inlining of a function with a sink value. Closure-Library has an example.

Forcing inlining is a bit trickier. I don't have a good answer here. Perhaps @concavelenz has thoughts?

[gaearon]

Totally understand if that’s an intentional design decision 👍

Is the sink method you’re linking to all that different though? Perhaps it lifts the responsibility from GCC itself so it doesn’t feel like something you explicitly recommend. That makes sense to me. Is the exact approach it’s using more or less guaranteed to work in the future?

[ChadKillingsworth]

Is the sink method you’re linking to all that different though?

It behaves exactly the same no matter which compiler passes are enabled. That's one of the major issues.

[concavelenz]

The "noinline" annotation might be reasonable but I would like to understand a bit more how we break the heuristic by inlining. Is it not sufficient that the inlined code be inlined into monomorphic branch? Or do we end up de-duplicating the code branches and end up creating megamorphic code? We do have a knob to control how large a function inlining is allowed to create if it is overall function size that triggers the issue.

I'm curious about the "inline" heuristic. Where does this actually help? It has been my assumption that with the modern VMs that they would do a better job of inlining for performance, and that we should only be inlining for code simplification. Or is this a case of code running without a JIT (react native?)

[MatrixFrog]

From conversations among the team last week, it sounds like we're leaning toward implementing "noinline" but not implementing "force inline"

[brad4d]

It looks like we want to use @noinline ourselves in order to prevent $jscomp.polyfill from getting inlined, which makes it a lot harder to find when trying to remove unused polyfill code.
I recently added @nocollapse to the definition of $jscomp.polyfill as a temporary work around when
a performance improvement change caused the inlined version to become even harder to identify by removing its unused parameters early in the compilation.

[w8r]

@inline would be an extremely useful addition for the complex algorithms.

[brad4d]

Just FYI, I've started the work to add @noinline. We don't currently have plans to do the opposite yet.

[brad4d]

I've now done enough work for @noinline to work for $jscomp.polyfill, and that was my primary motivation to work on it.
There are a few more *Inline*.java classes I haven't touched, so there are probably cases where it
still gets ignored.
I volunteer to be the primary person to handle issues related to it being ignored and PRs to fix them,
but I don't plan to work on it further for now other than that.

[leeoniya]

hey guys, came here to open the same issue.

my long-time work-around to prevent Closure's aggressive inlining has been to expose these functions in the window namespace and strip them out of the bundle via regex after compilation [1].

is there any information as to why the JIT is negatively affected? i always assumed it's because the function hits a certain AST size threshold and the JIT simply bails on more advanced/expensive tracing. i remember back in the day when actual function character count was supposedly used as a de-opt heuristic (including whitespace and comments!) [2], [3].

[1] domvm/domvm#144
[2] https://www.nearform.com/blog/node-js-is-getting-a-new-v8-with-turbofan/
[3] https://github.com/davidmarkclements/v8-perf/blob/master/bench/function-size.js

[brad4d]

@leeoniya
Our team doesn't have any info about Chrome's JIT behavior.

However, if your goal is just to prevent inlining in your code, I suggest you try using the @noinline annotation. If it works for you, great. If not, let us know.

I know that I didn't update every possible code path that could lead to inlining, but I believe I got most of them, so @noinline should work for you.

Also, if you're trying to prevent inlining because you expect your compiled code to be used from other JavaScript code, you should add @export to the visible classes and functions. Then you won't even need the @noinline, since the form will be preserved to make sure your exported definitions are available to external code.

[leeoniya]

thanks @brad4d,

@noinline appears to have worked with closure-compiler-js, too. for some reason i figured annotations only worked in ADVANCED mode.

also, not everything's exposed in the js version. ahem trustedStrings 😉 [1], [2].

cheers!

[1] google/closure-compiler-js#79
[2] https://github.com/leeoniya/domvm/blob/1da486030be281d1f7599b55ef5aa16ebc89e68e/build.js#L152-L160

[ChadKillingsworth]

@leeoniya Most things are now the same between the GWT version and the Java version. For instance, trusted strings are now set true: https://github.com/google/closure-compiler/blob/master/src/com/google/javascript/jscomp/gwt/client/GwtRunner.java#L686

[leeoniya]

awesome!

[ChadKillingsworth]

Since the @noinline annotation exists, I'm closing this issue.