Opened 4 years ago

Last modified 4 years ago

#10482 new bug

Not enough unboxing happens on data-family function argument

Reported by: akio Owned by:
Priority: normal Milestone:
Component: Compiler Version: 7.10.1
Keywords: TypeFamilies Cc: maoe@…
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

In the following code, foo and foo' have isomorphic types, but the worker-wrapper pass does less unboxing for foo:

{-# LANGUAGE BangPatterns #-}
{-# LANGUAGE TypeFamilies #-}
module Foo where

data family Foo a
data instance Foo (a, b) = FooPair !(Foo a) !(Foo b)
newtype instance Foo Int = Foo Int

foo :: Foo ((Int, Int), Int) -> Int -> Int
foo !f k =
  if k == 0 then 0
  else if even k then foo f (k-1)
  else case f of
    FooPair (FooPair (Foo n) _) _ -> n

data Foo0 a b c = Foo0 !(Foo1 a b) !c
data Foo1 a b = Foo1 !a !b

foo' :: Foo0 Int Int Int -> Int -> Int
foo' !f k =
  if k == 0 then 0
  else if even k then foo' f (k-1)
  else case f of
    Foo0 (Foo1 n _) _ -> n

The core generated by ghc -ddump-simpl -O2 ww.hs contains the following functions:

Foo.foo_$s$wfoo [Occ=LoopBreaker]
  :: Foo Int
     -> Foo Int
     -> Foo.R:Foo(,) Int Int ~R# Foo (Int, Int)
     -> Foo Int
     -> GHC.Prim.Int#
     -> Int

Foo.$wfoo [InlPrag=[0]]
  :: Foo (Int, Int) -> Foo Int -> GHC.Prim.Int# -> Int

Foo.$wfoo' [InlPrag=[0], Occ=LoopBreaker]
  :: GHC.Prim.Int# -> Int -> Int -> GHC.Prim.Int# -> Int

The first argument of Foo.foo_$s$wfoo could be Int#, but it takes a boxed value.

In practice this happens with unboxed vectors from the vector package.

Change History (7)

comment:1 Changed 4 years ago by maoe

Cc: maoe@… added

comment:2 Changed 4 years ago by Simon Peyton Jones <simonpj@…>

In 0b7e538a09bc958474ec704063eaa08836e9270e/ghc:

Allow recursive unwrapping of data families

When doing strictness analysis, we need to look inside products.
To avoid unpacking infinitely, we must be careful about
infinite types.  That in turn is controlled by TyCon.checkRecTc.

For data families like
   data instance T (a,b) = MkT a (T b)
we want to unpack the thing recursively for types like
  T (Int, (Int, (Int, Int)))

This patch elaborates the checkRecTc mechanism in TyCon, to
maintain a *count* of how many times a TyCon has shown up,
rather than just a boolean.

A simple change, and a useful one.  Fixes Trac #10482.

comment:3 Changed 4 years ago by Simon Peyton Jones <simonpj@…>

In 0696fc6d4de28cb589f6c751b8491911a5baf774/ghc:

Improve CPR behavior for strict constructors

When working on Trac #10482 I noticed that we could give constructor
arguments the CPR property if they are use strictly.

This is documented carefully in
    Note [CPR in a product case alternative]
and also
    Note [Initial CPR for strict binders]

There are a bunch of intersting examples in
    Note [CPR examples]
which I have added to the test suite as T10482a.

I also added a test for #10482 itself.

comment:4 Changed 4 years ago by simonpj

The patch in comment:2 fixes the original bug report but gives perf failures on

> 	haddock.Cabal
> 	haddock.compiler

Joachim says: I can confirm this, if you look at https://perf.haskell.org/ghc/ you see that that commit is the only in red

Here is the report for that commit: https://perf.haskell.org/ghc/#revision/0b7e538a09bc958474ec704063eaa08836e9270e and you can fetch the numbers from there, or from the full build log at https://raw.githubusercontent.com/nomeata/ghc-speed-logs/master/0b7e538a09bc958474ec704063eaa08836e9270e.log

It looks very reproducible as well: https://perf.haskell.org/ghc/#graph/tests/alloc/haddock.Cabal;hl=0b7e538a09bc958474ec704063eaa08836e9270e

So we have to discover why this is happening. Sigh.

comment:5 Changed 4 years ago by Joachim Breitner <mail@…>

In 9b5df2a401ba8f033cbbc80331f16ac8cf827c92/ghc:

Update performance numbers due to #10482

The fix in 0b7e538a has regressed these benchmarks. I have recentered
them rather than marking them as broken(10482), because nobody
systematically watches the broken test cases, and we want to catch
future regressions (or improvements!). #10482 is currently still open,
presumably because this needs investigating.

comment:6 Changed 4 years ago by simonpj

Yes, I want this ticket to stay open because it seems likely that the main commit caused the around 7% regression in allocation numbers for these two Haddock tests. I found that surprising and want to investigate.

Simon

comment:7 Changed 4 years ago by thomie

Keywords: TypeFamilies added
Note: See TracTickets for help on using tickets.