On Thu, Oct 30, 2008 at 01:09:36PM -0700, Jason Dagit wrote: > On Thu, Oct 30, 2008 at 1:00 PM, David Roundy <[EMAIL PROTECTED]> 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. -- David Roundy http://www.darcs.net _______________________________________________ darcs-users mailing list [email protected] http://lists.osuosl.org/mailman/listinfo/darcs-users
