[darcs-users] darcs patch: Move a couple of low-level pending bits ... (and 2 more)

Petr Rockai me at mornfall.net
Wed Oct 14 07:14:09 UTC 2009


Jason Dagit <dagit at codersbase.com> writes:

> What is the distinction between LowLevel and Internal?  I don't understand how
> this change improves things.  Internal is already meant to be private and
> whatnot.  Now I'm confused by the distinction between LowLevel and Internal. 
> Could you clarify the rationale?
Sure, the Internal module is a big mish-mash of interdependent
functionality. The problem is that separating parts of the Internal
functionality in State led to a situation, where State depends on
Internal. However, Internal now relies on readRecorded from Repository.State,
and therefore State cannot import Internal anymore, lest a dependency cycle
forms. Therefore, I've moved things that are needed by Repository.State but do
not need anything from Internal to a separate module. I intend to move more
such bits into that module later.

For reference, I first tried to separate the functions for handling "pending"
state into a separate Repository.Pending, but this would again lead to
dependency cycles: make_new_pending needs readRecorded from State,
unrecordedChanges in State need read_pending. Fortunately, read_pending is a
very low-level piece of code that basically doesn't do anything other than
deserialize the primitive patches in the pending file.

>     Tue Oct 13 12:45:23 CEST 2009  Petr Rockai <me at mornfall.net>
>      * Port list_* in Darcs.Arguments to Tree (away from Slurpy).
>
> Merely a suggestion, but if you're going to reimplement the list_* functions,
> then you should camelCase them :)  I'm not really sure if the new
> implementations do the same thing; although they seem reasonable enough.
Eventually, I guess so. Probably as a separate patch though.

>     Tue Oct 13 14:02:42 CEST 2009  Petr Rockai <me at mornfall.net>
>      * Convert contextual patch printing from Slurpy to TreeIO.
>
>      This obliterates another major source of SlurpDirectory usage in darcs. I
>     agree
>      the code has become a little more hairy, although this should be
>     addressable by
>      a slight refactor of virtualTreeIO and friends in hashed-storage (there's
>      currently no reasonable way to create a sub-monad in TreeIO).
>
>
> The only hairiness that I saw was some functions that used to be pure are now
> IO.  I don't think that's so bad though.
Well, it was only pure in so much as to hide any IO it did before in
unsafeInterleaveIO. The changes here expose that IO in the type signatures.

In general, I think darcs IO has been designed "upside down" -- it's built
around unsafeInterleaveIO, meaning that "pure" code would trigger opening and
reading files, and moreover (on windows) cause observable side-effects. This of
course breaks equational reasoning and blurries the edge between pure and IO.

I believe that after removing the IO interleaving (i.e. SlurpDirectory), we'll
be able to factor out a lot of pure code from the result, that will be
*actually* pure. E.g. I can right away see that coolContextHunk could be
refactored in such a way: the only IO bit it does is reading the file for
getting the context: you can of course lift that into a parameter and you have
a clean separation of pure worker and an IO wrapper.

> Would it be possible to see before and after benchmark numbers for this patch? 
> Of course the hard part is probably finding test cases that exercise the code
> which is changing here.  Maybe someone besides Petr could volunteer to do this?
>   I think it's mainly the construction of patch bundles and the pretty printer
> that need to be benchmarked.
I guess so, although I wouldn't expect any serious impact in either direction.

Yours,
   Petr.


More information about the darcs-users mailing list