Can NOINLINE fail to prevent inlining?

Learn
1

If a top-level variable foo :: Foo is annotated with NOINLINE, are there any situations where GHC would inline it anyway?

Context

I prototyped a new API for the prometheus package to define counters as top-level variables that are (by default) registered to a global registry. This is an attempt to adhere to Prometheus’s specification that client libraries should follow, specifically:

There MUST be a default CollectorRegistry, the standard metrics MUST by default implicitly register into it with no special work required by the user.

Whether this is a good API that you’d personally use or not aside, is defining top-level counters with unsafePerformIO + NOINLINE sufficient? My coworker (gbaz) claims that we’ve seen cases where the NOINLINE is fragile and the counter is effectively inlined.

2

Yes, funnily enough NOINLINE doesn’t prevent inlining. See this issue.

Use OPAQUE.

3

That issue is about worker-wrapper, not inlining.

I really think NOINLINE is safe for defining a top-level IORef.

4

It is about inlining because the wrapper gets inlined despite NOINLINE on the original definition. I’ve observed it in production, which is how I know about the issue.

The original question was

I answered that.

5

OK yeah, I read the GHC issue thread, it isn’t particularly telling indeed. Here’s a quote from the OPAQUE proposal instead:

image
image1433×379 79.5 KB

6

Indeed, it apparently can inline the wrapper. But it should never inline the worker, which is the part that would contain the actual unsafePerformIO and the initialization of the IORef, so that is still safe.

7

Well it sounds like OPAQUE is exactly NOINLINE + more guarantees, so worst case, it does the same thing as NOINLINE? cc @christiaanb

8

Right, for your use case OPAQUE should be no less “fragile” than NOINLINE.

The goal of OPAQUE is: that every call of f generates, well, a call of f, not of some name-mangled variant.

So the property it provides is something only GHC API users and those trying to create minimal reproducers for GHC bugs, the people interested in the “syntax” of the intermediate Core representation, should really care about.

For everyone else, there should be no difference in the “semantics” of the intermediate Core representation between NOINLINE and OPAQUE, except that OPAQUE is likely to give you worse performance when GHC optimizations are turned on. Precisely because OPAQUE prevents certain optimizations that NOINLINE does allow.

I would even go as far as saying that I would consider it a GHC bug in the treatment of NOINLINE when the semantics of your code are as intended when you use OPAQUE, but not when you use NOINLINE; even in the face of unsafePerformIO.

9

Is that true? If NOINLINE intentionally doesnt prevent inlining with regard to worker/wrapper, then there’s a semantic difference (not a bug) where NOINLINE might inline an unsafePerformIO CAF but OPAQUE doesnt.

10

Isn’t that a text book example of why it’s called unsafe?

11

Took a stab at a proposal: GHC Proposal: Top-level IO initialized bindings

12

The documentation for unsafePerformIO specifically says that NOINLINE is the pragma you should use to prevent the I/O action being performed more than once: System.IO.Unsafe ; so yeah, I would consider it a bug.

Another thing that it says it that you should disable let-floating by using -fno-full-laziness, this has been my experience as well: Number of unique ID's limited to 1024? · Issue #1 · ekmett/concurrent-supply · GitHub

13

Another situation is when “the state hack” gets triggered. At least until someone implements this proposal.

14

No, the wrapper getting inlined only means that unsafePerformIO can get inlined if unsafePerformIO ends up in the wrapper, not the worker. And it doesn’t, since the wrapper only handles coercions and unboxings, and should not do any actual computation.

Over at your GHC proposal thread you mentioned

My immediate motivation for this thread is me wanting to use top-level unsafePerformIO + NOINLINE at work, and hearing concerns about “we’ve seen it inlined in the past, so we don’t trust it”.

I was going to ask: do we have actual evidence for this? Cargo-culting is not particularly helpful. But then I read the documentation for unsafePerformIO, which, in addition to requiring you add NOINLINE and set -fno-cse -fno-full-laziness, also says:

Note that these precautions are necessary for GHC, but may not be sufficient, and other compilers may require different precautions

(emphasis mine). Since we’re entering GHC-specific land with unsafe functions anyway the latter part of the sentence is not so surprising, but the documentation clearly states that GHC will not guarantee the expected behaviour for this pattern even if you do all of these things!

This surprised me a lot, since I have multiple uses of this pattern in various places. Can a GHC expert chime in what this remark refers to? If it is the state hack, perhaps -fno-state-hack should be added to the unsafePerformIO documentation too!

15

Yeah, that’s fair. But as I mentioned somewhere else, even if we’ve found the perfect incantation with all the right flags, there will always be a sliver of a doubt if there’s some edge case it won’t catch, or if a GHC version upgrade breaks it. I’d rather have a semantically guaranteed construct for this, for operations that ought to be safe.