darcs

Patch 87 Avoid a redundant read of pending in State.hs

Title Avoid a redundant read of pending in State.hs
Superseder Nosy List dagit, darcs-users, mornfall
Related Issues
Status rejected Assigned To mornfall
Milestone

Created on 2009-11-16.00:57:26 by dagit, last changed 2011-05-10.17:15:51 by darcswatch. Tracked on DarcsWatch.

Files
File name Status Uploaded Type Edit Remove
avoid-a-redundant-read-of-pending-in-state_hs.dpatch dagit, 2009-11-16.00:57:25 text/x-darcs-patch
unnamed dagit, 2009-11-16.00:57:25 text/plain
See mailing list archives for discussion on individual patches.
Messages
msg9378 (view) Author: dagit Date: 2009-11-16.00:57:25
Hello,

I sent a patch just like this to Petr back on Mon, Sep 28, 2009, but
it was not applied.  I don't know why.  Petr, was there some problem
with the changes below?  Note: I did have to recreate the patch
because it was not applicable against HEAD due to an unrename of the
darcs.cabal file (some odd patch dependency issue).

Here is the description of why I created this patch:
[this] patch is a refactor that gives a pretty substantial
performance boost.  I haven't necessarily implemented it in the best
mananer, just the simplest.

We were effectively constructing the sequence of pending patches
twice, or at least some of them.

With profiling enabled:
Before this change:
            322 MB total memory in use (2 MB lost due to
            fragmentation)
 Total time   61.23s  ( 65.00s elapsed)

After this change:
           245 MB total memory in use (2 MB lost due to
           fragmentation)

 Total time   48.28s  ( 59.95s elapsed)

Normal builds:
Before this change:
            223 MB total memory in use (2 MB lost due to
            fragmentation)
 Total time   14.50s  ( 16.17s elapsed)

 %GC time      25.0%  (24.8% elapsed)

 Productivity  75.0% of total user, 67.3% of total elapsed

After this change:
            174 MB total memory in use (1 MB lost due to
            fragmentation)
 Total time   12.52s  ( 14.01s elapsed)

 %GC time      22.8%  (22.4% elapsed)

 Productivity  77.2% of total user, 69.0% of total elapsed

So, it seems to save about 2 seconds of runtime, and close to 50
megs of physical ram.

Please let me know if you have questions!  It's much much better
performance now on this particular use case than the darcs.net source,
which uses over 400 megs of ram and takes closer to a minute.

Thanks,
Jason

Sun Nov 15 16:53:12 PST 2009  Jason Dagit <dagit@codersbase.com>
  * Avoid a redundant read of pending in State.hs
Attachments
msg9379 (view) Author: darcswatch Date: 2009-11-16.01:00:05
This patch bundle (with 1 patch), which can be  applied to the repository http://darcs.net/ is now tracked  on DarcsWatch at http://darcswatch.nomeata.de/repo_http:__darcs.net_.html
msg9390 (view) Author: mornfall Date: 2009-11-16.13:23:24
Hi, sorry it must have fallen through the cracks. Anyway, looking at the patch
now, I suspect why: this doesn't seem to be correct. unrecordedChanges now
returns all of the pending, instead of its relevant subset, which breaks all
commands that take paths and use unrecordedChanges (whatsnew, record, revert,
etc.). This has happened to me before and caused extra changes to be printed in
whatsnew dir/ when there were pending changes outside dir/. Maybe a regression
test is in place...

Thanks,
   Petr.
msg10254 (view) Author: dagit Date: 2010-03-18.02:49:35
Looks like this was rejected but not marked as such.  Fixing that now :)
History
Date User Action Args
2009-11-16 00:57:26dagitcreate
2009-11-16 01:00:06darcswatchsetmessages: + msg9379
2009-11-16 13:23:24mornfallsetstatus: needs-review -> followup-requested
assignedto: mornfall
messages: + msg9390
nosy: + mornfall
2010-03-18 02:49:36dagitsetstatus: followup-requested -> rejected
messages: + msg10254
2011-05-10 17:15:51darcswatchsetdarcswatchurl: http://darcswatch.nomeata.de/repo_http:__darcs.net_reviewed.html#bundle-80bc46d2d2ed81363efb708170d96608b646ded6