cmd/compile: go:linkname prevents inlining

[dvyukov]

go version devel +33c3477039 Tue Apr 18 03:56:16 2017 +0000 linux/amd64

sync_runtime_procPin is marked with //go:linkname sync_runtime_procPin sync.runtime_procPin. Runtime marks it as inlinable (which is good). However, sync fails to inline it with cannot inline runtime_canSpin: no function body.
There are more functions exposed from runtime to sync, sync/atomic, reflect, time, os.
Can we inline them?

[dvyukov]

@randall77 @aclements @RLH @valyala

[valyala]

cc'ing @josharian

[josharian]

At first glance, looks like an import/export issue--the function body is not associated correctly (or perhaps not exported at all) when linkname is present.

cc @mdempsky @griesemer

[dvyukov]

It's possible to quickly estimate the benefits -- make procPin/Unpin public in runtime and directly call from sync.Pool. @valyala care to prototype?

[cherrymui]

@josharian If I understand correctly, linkname is resolved at link time (as the name suggests). The runtime package doesn't export those names.

[valyala]

It's possible to quickly estimate the benefits -- make procPin/Unpin public in runtime and directly call from sync.Pool. @valyala care to prototype?

@dvyukov , sure:

GOMAXPROCS=4:

name            old time/op  new time/op  delta
Pool-4          10.5ns ± 9%   8.8ns ± 9%  -15.59%  (p=0.000 n=10+10)
PoolOverflow-4  2.10µs ± 3%  1.95µs ± 2%   -7.29%  (p=0.000 n=10+10)

GOMAXPROCS=1:

name          old time/op  new time/op  delta
Pool          20.3ns ± 2%  16.9ns ± 3%  -17.06%  (p=0.000 n=10+10)
PoolOverflow  4.96µs ± 2%  4.75µs ± 1%   -4.16%  (p=0.000 n=10+9)

[josharian]

@cherrymui but I believe we do have inlining information in the export data for non-exported function, specifically for (multi-level) inlining.

[cherrymui]

@josharian you're right, thanks!
But the package that have functions implemented by another package may not import that package. sync/atomic doesn't import runtime, and the compiler doesn't even read runtime.a when compiling sync/atomic. For this to work, we'll need to add import _ "runtime" in sync/atomic?

[josharian]

@cherrymui true. And that doesn't seem too much to ask, if you're already using go:linkname. :) Although this also reproduces with package sync (in pool.go), which does already import package runtime.

[mdempsky]

I think the issue is just in bimport.go and trivially fixable. @dvyukov do you have an easy standalone test case for this?

[dvyukov]

@mdempsky no, I don't. I was looking at sync_runtime_procPin.

[mdempsky]

Thanks. I can repro the issue, but realized it's more involved than simply tweaking bimport.go.

Currently, we associate function inlining bodies only at the individual Node level. To fix this, we'll need to cross-reference them via LSyms.

[josharian]

Not sure that's worth it at the moment.