[darcs-users] darcs patch: add exception to haskell_policy.sh for D... (and 61 more)

[Don Stewart]

droundy:
> On Thu, Oct 30, 2008 at 01:09:36PM -0700, Jason Dagit wrote:
> > On Thu, Oct 30, 2008 at 1:00 PM, David Roundy <droundy at darcs.net> wrote:
> > > On Wed, Oct 29, 2008 at 07:50:32PM +0000, Eric Kow wrote:
> > >> On Wed, Oct 29, 2008 at 10:30:12 -0700, Jason Dagit wrote:
> > >> > I want to see benchmarks too, but I thought I would justify why we
> > >> > expect this to be no slower than the previous code...Everything below
> > >> > is stuff that we discussed during the Sprint.
> > >>
> > >> Well, attached is a second set of comparative timing tests, sorry, only
> > >> run once and with no nice output yet.  Hopefully you can use a graphical
> > >> diff tool to do side by side comparison.
> > >>
> > >> A nice little summariser script, maybe using the Haskell tabular
> > >> library might be handy
> > >
> > > I've run my own set of timings, which give considerably more dramatic
> > > differences than yours show, perhaps because I ran my tests on large
> > > repositories?
> > >
> > > I'll summarize my results up here, but you can look below for a more
> > > verbose summary.  The new code is almost always faster by my reckoning, and
> > > often *much* faster (as much as a factor of three!).
> > 
> > Very nice.  Just curious, how many trial runs did you do?
> 
> Three runs with each configuration.  The variation in almost every case was
> much less than the difference between the three configurations.
> 
> > > I do, however, observe a performance regression on darcs annotate Setup.hs
> > > (run in the darcs repository).  It's a small regression (smaller than
> > > Eric's tests show), but reproducible.  And it's all the more striking given
> > > the dramatic improvement the new code shows in all the other tests, which
> > > suggest there may be a single function that has a large performance
> > > regression (since it seems likely that parts of the annotate command have
> > > been sped up in the new version).  I don't have the time or inclination to
> > > track down this issue, but I do hope Don is interested enough to look into
> > > it!
> > 
> > If we're going to accept this, it may be better to look at the
> > annotate issue after Ganesh's patches go in.  Otherwise there is
> > potential to waste effort optimizing something that is going to be
> > removed or rewritten, right?
> 
> No, the point is that there's a bytestring primitive that is significantly
> slower than it ought to be, and we ought to take this chance to figure out
> what that primitive is.  The smart approach would have been to *first*
> optimize the bytestring implementation to be as fast as the
> OldFastPackedString implementation and *then* remove the old
> implementation.  It's quite possible that putting the OldFastPackedString
> code back in will give us yet another speedup, because most of the
> optimizations Don did were independent of the switch to bytestring
> (mostly just fixing stupid things that I wrote).
> 
> As it is, the optimizations to the darcs code are mixed up with
> pessimizations in the bytestring implementation, and it's close to
> impossible to find those pessimizations.  True, they're not all that huge
> (maybe no worse than a factor of two or three?), but it's still be nice to
> avoid slowing the code down at all, particularly when there's little to no
> benefit.  I'm pretty much convinced (having looked at many of the changes
> Don made) that the speedup isn't due to the OldFastPackedString ->
> bytestring change, but rather to simple optimizations that would affect
> either library.

So would you like me to look at what changed with annotate, in
particular, or where there other things?

-- Don