[darcs-devel] darcs patch: Change a can't happen error to an imposs... (and 5 more)

[David Roundy]

On Mon, Apr 11, 2005 at 11:08:54PM -0400, Ian Lynagh wrote:
> 
> One problem we have left is that the skipped list is keeping the list
> around while we work on the selected patches. If in Record.do_record you
> change skipped to _skipped and pass [] in its place then memory usage is
> far better (and time improves again too).
> 
> Unfortunately I can't see a nice fix right now, so I'm going to bed
> instead  :-)

Hmmmm.  Sleep good!

We only need skipped in order to write pending changes, so we could pass
(sift_for_pending skipped) to do_record, but I'm not sure that would help.
I'm not really clear on why skipped is causing other patches to be held in
memory... you *are* referring to tests where --all is specified?

I guess maybe it has to do with how in with_any_selected_changes we handle
the --all case.  Perhaps we need to special-case the situation where 

All `elem` opts && null fs

since in this case other_ps is guaranteed to be [], and shouldn't "hold on"
to any of the changes.  Also, this is quite a common "special" case, when
you record --all without specifying a patch name.

But perhaps there is a non-special-casing way we could rewrite the
(ps_to_consider, other_ps) bit to make it better-behaved...


Actually, my timing test just finished running, and my "kernel add
everything" test now runs in about 435m max RAM, and takes another 20% less
wallclock time.  This is about as good as the improved "get" performance,
so it looks like you've already gotten "record" down to the point where
it's no longer a memory bottleneck.  That isn't to say that we shouldn't
improve it any more, but at this stage I'd say the fundamental problems
have been solved--at least for the record of an initial patch, which was
the horribly-misbehaving case.
-- 
David Roundy
http://www.darcs.net