Opened 5 years ago

Last modified 4 years ago

#9655 new bug

Do not UNPACK strict fields that are very wide

Reported by: simonpj Owned by:
Priority: normal Milestone:
Component: Compiler Version: 7.8.3
Keywords: Cc: jwlato@…
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: Runtime performance bug Test Case:
Blocked By: Blocking:
Related Tickets: Differential Rev(s):
Wiki Page:

Description (last modified by simonpj)

John Lato writes (a propos of another discussion): "This is possibly unrelated, but the setup seems almost identical to a very similar problem we had in some code, i.e. very long compile times (6+ minutes for 1 module) and excessive memory usage when compiling generic serialization instances for some data structures.

In our case, I also thought that INLINE functions were the cause of the problem, but it turns out they were not. We had a nested data structure, e.g.

> data Foo { fooBar :: !Bar, ... }

with Bar a very wide product type (~150 fields).

Even when we explicitly NOINLINE'd the function that serialized Bar, GHC still created a very large helper function of the form:

> serialize_foo :: Int# -> Int#  -> ...

where the arguments were the unboxed fields of the Bar structure, along with the other fields within Foo. It appears that even though the serialization function was NOINLINE'd, it simply created a Builder, and while combining the Builder's ghc saw the full structure. Our serializer uses blaze, but perhaps Binary's builder is similar enough the same thing could happen.

Anyway, in our case the fix was to simply remove the bang pattern from the 'fooBar' record field."

So the question is: should GHC auto-unpack a strict argument of a data constructor, if the argument type is a very wide product type?. I think "no". The unpacking can save allocation (by allocating one object instead of two) but it can also increase it (at a pattern matching site). So it should probably only happen automatically for sufficiently narrow types.

How narrow? We need some testing, but my guess is three or four words. Maybe a flag to set the size? (Maybe not worth the pain.)

Incidentally, the choice can already be manually controlled with {-# UNPACK #-} and {-# NOUNPACK #-} pragmas.

Change History (6)

comment:1 Changed 5 years ago by simonpj

Description: modified (diff)

comment:2 Changed 5 years ago by tibbe

should GHC auto-unpack a strict argument of a data constructor, if the argument type is a very wide product type?

I'm confused. I don't think GHC does unpack strict fields unless either 1) it's of pointer size or 2) you use -funbox-strict-fields or 3) you use UNPACK.

comment:3 Changed 5 years ago by simonpj

Cc: jwlato@… added

I'm guessing that John was using -funbox-strict-fields.

comment:4 Changed 5 years ago by jwlato

Ah, thanks for this ticket, I think it's a good idea. Unfortunately my code isn't using -funbox-strict-fields. To double-check I tried adding a {-# NOUNPACK #-} on the wide strict field and the behavior didn't change, so I think something else is going on with my code. But I do think the suggested behavior is more sensible, I usually avoid -funbox-strict-fields because I don't want types like this auto-unboxed.

comment:5 Changed 5 years ago by simonpj

Ah. In that case I'm puzzled.

The strictness analyser does worker/wrapper split that does not take account of the width of the argument product, which is more defensible than in unpacking wide data type declarations. But in that case

  • I don't think that removing the "!" would make a difference.
  • We might get a worker with lots of arguments but I don't see why that would increase compile times.

John: if you feel motivated, could you boil out a reproducible test case? If you use -dshow-passes on your current set-up, I think you'll see that the size of one of the intermediate stages blows up when you have the "!" vs not having it. So you can use that unexpected size blow-up instead of looking for very long compile times (which can get a bit painful).


comment:6 Changed 4 years ago by thomie

Type of failure: None/UnknownRuntime performance bug
Note: See TracTickets for help on using tickets.