Re: Summary of containers patches

On 24/09/2010 09:41, Thomas Schilling wrote:

On 24 September 2010 06:19, Don Stewart wrote:

...

ivan.miljenovic:

...

On 24 September 2010 14:58, Don Stewart wrote:

...

ivan.miljenovic:

...

On 24 September 2010 14:36, Michael Snoyman wrote:

...

On Fri, Sep 24, 2010 at 6:26 AM, Don Stewart wrote: > Perhaps a containers-inline fork is needed, for those who still need the speed.

Would it be possible to use CPP to turn the INLINE flags into a compile-time argument, ie:

#ifdef INLINE {-# INLINE #-} #endif

Since containers ships with GHC, wouldn't this then require an extra flag being used when building GHC to enable this?

And then to use it, you'd have to build your own GHC rather than using a pre-built binary like just about everyone does...

...

I'd hate to start seeing incompatible Data.Map.Maps floating around.

Agreed. At the very least if there was a fork it would presumably have to be in a different module namespace to avoid namespace collisions, which would make the incompatability obvious.

We're talking about a 3% increase in the size of the Map, a 2% size in the Map.hs benchmark binary, right?

For a 50% increase in Map function performance.

I'm not arguing against it; I'm just arguing against whether or not a fork would be beneficial.

Indeed. I'm surprised we're actually considering rolling back Map specialization -- and the big improvements that gives us -- for what seems to be a small percent in binary size.

Recall Johan's recent user survey: people are asking for more performance, and more reliable performance, not shaving binary sizes.

That would be true if we actually had numbers on how the changes affect programs in the wild. Your performance claims are based on micro benchmarks. Who knows whether code bloat will offset the performance gains on any "real" benchmark. One GHC object file *tripled* in size, for example. This is probably because it's simply wrapping Map, but it's still not clear how it may affect other programs.

It's not a wrapper for Map, in fact. The module is here: http://darcs.haskell.org/ghc/compiler/cmm/CmmStackLayout.hs it's just a client module that happens to call a lot of Map functions. The keys aren't Ints (or at least, most of them aren't). We didn't see any difference in GHC's performance with the first round of Map changes; in fact GHC allocated a bit more afterward. We didn't investigate any further, and of course that's just one data point. The difference wasn't significant enough to warrant backing off.

All I'm saying, be careful with these numbers.

+1 Cheers, Simon