Opened 5 years ago

Last modified 5 years ago

#9992 new bug

Constructor specialization requires eta expansion

Reported by: bgamari Owned by:
Priority: normal Milestone:
Component: Compiler Version: 7.8.4
Keywords: performance Cc: tibbe
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

I recently encountered a slightly tricky performance issue[1] in bytestring's Builder implementation which required explicit eta expansion in order for GHC to perform ConstrSpec.

We have

data BufferRange = BufferRange {-# UNPACK #-} !(Ptr Word8)  -- First byte of range
                               {-# UNPACK #-} !(Ptr Word8)  -- First byte /after/ range

data BuildSignal a = ... -- Just a vanilla ADT
type BuildStep a = BufferRange -> IO (BuildSignal a)

We end up with Core (full version https://gist.github.com/bgamari/091e3dac9c45ee9accf1#file-slow-hs-L1) that looks like this,

Main.$wa 
  :: GHC.Prim.Word#
     -> GHC.Prim.Int#
     -> forall r_aKUt.
        Data.ByteString.Builder.Internal.BuildStep r_aKUt
        -> Data.ByteString.Builder.Internal.BuildStep r_aKUt
Main.$wa =
  \ (ww_s10YU :: GHC.Prim.Word#)
    (ww1_s10YY :: GHC.Prim.Int#)
    (@ r_aKUt)
    (w_s10YR :: Data.ByteString.Builder.Internal.BuildStep r_aKUt) ->
    case ww1_s10YY of wild_XE {
      __DEFAULT -> (
          \ (eta1_X1Q :: Data.ByteString.Builder.Internal.BufferRange) ->
            case eta1_X1Q of BufferRange a b -> 
              -- A bunch of code eventually ending in a recursive call to $wa
      )
      0 -> w_s10YR
    }

If w_s10YR is eta-expanded GHC will run ConstrSpec, eliminating the fields of BufferRange to be unpacked between iterations of $wa and substantially improving performance.

In [1] we had to manually eta-expand the empty case to ensure that this would happen. It would be great if GHC would identify cases like this.

[1] https://github.com/haskell/bytestring/pull/40

Change History (3)

comment:1 Changed 5 years ago by tibbe

Cc: tibbe added

comment:2 Changed 5 years ago by simonpj

Ah, very interesting. I'll investigate

comment:3 Changed 5 years ago by Herbert Valerio Riedel <hvr@…>

In ff4733f4e0355085002a1f9053ba2276e92d2cb6/ghc:

Update bytestring submodule

This pulls in

- https://github.com/haskell/bytestring/pull/40 (which is related to #9992)
- https://github.com/haskell/bytestring/pull/38
Note: See TracTickets for help on using tickets.