[darcs-devel] darcs patch: Do not reread freshly written patch when recording.

[Ian Lynagh]

On Mon, Jan 16, 2006 at 05:09:08PM +0100, Juliusz Chroboczek wrote:
> > Sat Jan 14 04:22:52 PST 2006  Jason Dagit <dagit at codersbase.com>
> >   * Do not reread freshly written patch when recording.
> 
> I've looked at the code again -- and I'm puzzled.  I s'pose it's
> linesPS biting us again.

We need to do linesPS in order to make the patch anyway, which is why
I'm a bit surprised that the parsing is so much slower (I don't know if
the 50x was due to swapping, but even the 10x is a bit surprising).
Nevertheless, with the new patch format the parsing will be a constant
time operation (read a handful of Ints, skip <one of the Ints> bytes,
read end of patch) so this ought to fix itself in due course.

(if we are using compression we'll also have a gunzip to do, but that
ought to be faster than the gzip we're doing anyway so shouldn't be
worse than a factor of 2).

It would still be good to fix the non-constant-space regression if it
affects the many-small-files case, though. This should also give time
improvements as the GCer has less to worry about, and even more so if
you hit swap.

That said, it seems Jason's numbers are including mmapped files, so it's
not clear exactly what's going on.


Incidentally, I think it would be worth looking at having the changes
selection code working its way down the resultant list writing things to
a selected or unselected temporary file as appropriate, and then reading
them back in, rather than using the hacks like pull_only_firsts. This
should make it easier to reason about what's going on, and I think mean
we won't have to worry about profiling affecting the space behaviour
(i.e. I think making that change would mean we can drop the Record and
SelectChanges auto-all-filtering).

> I'm going
> to wait for Ian to work out what's wrong with the current code before
> committing this thing.

I'm not actively looking into this (due to spending what darcs time I
have trying to think about conflictors).

> Ian, please let me know if you decide you're happy with this patch.

I still think this patch is a step in the wrong direction, and I doubt
this will ever change.


Thanks
Ian