Another attempt at improving memory usage for patch index creation.

I hope I'm getting there now:
 - All the tests pass
 - The memory profile is flat when creating the index on the screened repo
 - darcs annotate darcs.cabal output looks plausible

However, I haven't benchmarked this in general yet so it's possible
it would regress performance in some way.

4 patches for repository darcs-unstable@darcs.net:screened:

Wed Nov 19 17:45:06 GMT 2014  Ganesh Sittampalam <ganesh@earth.li>
  * Move the handling of individual PatchMods into applyPatchMods
  
  This also pushes the 'nubSeq' call to only operate on the [PatchMod].
  This should make no difference as the PatchIds would be unique anyway,
  and it makes the behaviour of applyPatchMods more compositional with
  respect to individual patches.
  

Mon Nov 24 17:59:58 GMT 2014  Ganesh Sittampalam <ganesh@earth.li>
  * Keep the PatchId list in reversed order in memory
  
  This is preparatory for a further patch that builds it up
  incrementally.
  

Mon Nov 24 18:07:39 GMT 2014  Ganesh Sittampalam <ganesh@earth.li>
  * maintain the list of PatchIds inside the PatchIndex structure
  
  This should make it easier to build the two together incrementally
  

Mon Nov 24 18:08:20 GMT 2014  Ganesh Sittampalam <ganesh@earth.li>
  * use the strict state monad for working with PatchIndex
  
  This allows applyPatchMods to consume a list of patches incrementally,
  otherwise the entire list of patches is kept around until it finishes.
  
  It also seems like a reasonable change for any other code that works
  on a PatchIndex, as there's no particular value in it being lazy.

Attachments

• move-the-handling-of-individual-patchmods-into-applypatchmods.dpatch
• patch-preview.txt
• unnamed

Output of annotate is correct on the few files I tried in the darcs.net
repo (darcs.cabal and NEWS), and there is no notable performance
regression on these files.

Memory consumption on PI creation is indeed flat (hurrah!) and time
performance is slightly better. Also each patch make minimal changes so
that's reassuring. I'm screening the bundle for now (not accepting it
yet in case there is a general performance regression).

The patch "Keep the PatchId list in reversed order in memory" makes
"darcs show patch-index-test" fail in the darcs.net repo.

Also we can safely change the on-disk order of patch-ids by bumping the
PI version number.

Agreed re fixing the on-disk format - I think it makes sense to do that 
after fixing the patch-index-test failure. I'll get to that once I 
reach a good stopping point with the encoding work, which I don't want 
to page out at present.

This should resolve the patch-index-test problem.

1 patch for repository darcs-unstable@darcs.net:screened:

Wed Dec 10 23:03:43 GMT 2014  Ganesh Sittampalam <ganesh@earth.li>
  * need to un-reverse the pids for the patch-index-test

Attachments

• need-to-un_reverse-the-pids-for-the-patch_index_test.dpatch
• patch-preview.txt
• unnamed

I'm having another issue when amending a patch:

```
darcs init x
cd x
touch x
darcs rec -lam x
echo content > x
darcs rec -am amend_me
echo content > x
echo ya | darcs amend

darcs failed:  bug at src/Darcs/Repository/PatchIndex.hs:264 compiled
Dec 11 2014 19:15:38
couldn't find ./x in patch index
```

Sorry the second echo should be different from the first one:

```
darcs init x
cd x
touch x
darcs rec -lam x
echo content > x
darcs rec -am amend_me
echo different_content > x
echo ya | darcs amend

darcs failed:  bug at src/Darcs/Repository/PatchIndex.hs:264 compiled
Dec 11 2014 19:15:38
couldn't find ./x in patch index
```

(Wrongly sent the bundle to a new patch)

A fix for the above bug:

removePidSuffix relies on patch ids order (see local functions before
and beforeM),
so each time we run updatePatchIndexOnDisk we wrongly remove a LOT of stuff.

Patch is here (already screened):

http://bugs.darcs.net/file5259/patch-index_-adapt-removepidsuffix-to-the-new-order-of-pids.dpatch

Already a patch index version change! This way we avoid some
computing when loading and storing it on disk.

2 patches for repository http://darcs.net:

patch e4ac703007a23ea345848839a67bddb5bbdf3095
Author: Guillaume Hoffmann <guillaumh@gmail.com>
Date:   Thu Dec 11 19:38:32 ART 2014
  * check on-disk version of patch index at right place

patch e4ad20d9b68738b2203944c3500a713cdef709e9
Author: Guillaume Hoffmann <guillaumh@gmail.com>
Date:   Thu Dec 11 21:26:47 ART 2014
  * patch index: change order of on-disk pids
  this way we don't have to reverse the pids list all the time

Attachments

• check-on_disk-version-of-patch-index-at-right-place.dpatch
• patch-preview.txt
• unnamed

Did you get the last problem you reported before trying to reverse 
the pids on disk? The pids2idx structure is built from the on-disk 
version, so would have been in the correct order before the on-disk 
version was reversed.

Anyway, I think the overall effect of the two patches is correct and 
an improvement.

Yes, it happened before my last two patches (it's easy to reproduce).

And no, pids2idx did not depend on the order of pids returned by the
function loadPatchIndex (ie, only the pids were reversed).

I checked again and I'm fairly sure your repro is fine without both 
of these patches, or with both of them, but fails if either one is 
on its own:

patch index: change order of on-disk pids
patch index: adapt removePidSuffix to the new order of pids

Anyway, we're both happy with the end result, so the intermediate 
details don't matter too much.

Reattaching Guillaume's fix to this patch so it's all in one place
1 patch for repository /home/ganesh/darcs/screened-temp2:

Fri Dec 12 00:01:03 GMT 2014  Guillaume Hoffmann <guillaumh@gmail.com>
  * patch index: adapt removePidSuffix to the new order of pids

Attachments

• patch-index_-adapt-removepidsuffix-to-the-new-order-of-pids.dpatch
• patch-preview.txt
• unnamed

Accepting along with other patch index patches (goes to branch 2.10).