OK I think we've pretty much confirmed it's the additional function call, not reloading the P.

What's preventing getMCache from inlining seems to be the throw inside. If that gets treated like a regular call, then it's still not quite enough. But a throw is like a panic, but strictly rarer because a throw can't be recovered from. Perhaps it should be cheaper?

CC @dr2chase who knows all about mid-stack inlining and inlining costs.

Aside: Fixing #24686 might let us get rid of mcache entirely. I never managed to get it implemented, but I don't know the runtime as well as you. :)

There are some failures in last builds for AIX which seem to related to this issue:
https://build.golang.org/log/fa90b60048ac7f99bc84523d236948a1f090b4c3
https://build.golang.org/log/a5515473b2094744ca0ff0d19c330fa66ca6184a

I've searched really quickly through the buildfarm dashboard but it didn't find any other appearance.

There are some failures in last builds for AIX which seem to related to this issue:
https://build.golang.org/log/fa90b60048ac7f99bc84523d236948a1f090b4c3
https://build.golang.org/log/a5515473b2094744ca0ff0d19c330fa66ca6184a

I've searched really quickly through the buildfarm dashboard but it didn't find any other appearance.

Huh. That is a problem (write barrier without a P...?) but separate from this one. Just the fact that allocSpan now actually cares that we have a P.

@Helflym Could you file a new issue for that and assign it to me?

@Helflym Could you file a new issue for that and assign it to me?

Done with #42339. But it can't assign it to you. I don't have the permissions I guess

So there are a number of ways to fix this regression. First is to make throw have cost 1, so getMCache gets inlined. Second is to manually inline getMCache and leave a comment. Third is to reorganize getMCache to put the throw in some //go:noinline function and hope that the call cost (57) is low enough that it still gets inlined.

For (1) and (3), we should add getMCache to the list of functions we want to make sure get inlined. I'm leaning toward (2) as the safest solution for the 1.16 release, though (3) is fine also (albeit a bit ugly).

@prattmic @dr2chase WDYT?

A fourth alternative is to change getMCache to return nil if it can't get one instead of throwing. Then it would be up to the caller to throw if necessary. This would be useful to handle #42339 also since apparently allocSpan needs to be able to run without a P. It requires updating each callsite, but because it also helps with another issue, this seems like a good idea to me.

I think I'm gonna go with 4.

(Credit to @prattmic for coming up with option 4 last week.)

Change https://golang.org/cl/267157 mentions this issue: runtime: make getMCache inlineable