darcs

Issue 1760 Very slow darcs convert with 2.4 (fine with 2.3.0)

Title Very slow darcs convert with 2.4 (fine with 2.3.0)
Priority bug Status resolved
Milestone 2.4.x Resolved in 2.4.x
Superseder Nosy List alexsuraci, attila.lendvai, darcs-devel, dino, dmitry.kurochkin, ganesh, kowey, mornfall
Assigned To mornfall
Topics Performance, Regression

Created on 2010-03-10.00:07:21 by alexsuraci, last changed 2010-06-16.14:38:46 by kowey.

Messages
msg10149 (view) Author: alexsuraci Date: 2010-03-10.00:07:18
With this repository: http://johnmacfarlane.net/highlighting-kate

Running darcs convert with 2.4 on this repository seems to take exponentially 
longer and longer for each patch and pegs the CPU usage at 100%.

Converting the same repository with darcs 2.3.0 using the same arguments works 
nearly instantly.
msg10151 (view) Author: kowey Date: 2010-03-10.11:08:38
Actually, I think I've seen somebody else report a recent performance
regression in darcs convert.  The last time this happened, I advised the
person that using darcs 2.3.x convert is just fine.

Hmph! N steps forward, M steps back.

Thanks for the report!  We just need somebody to systematically
investigate this and figure out why this went wrong.
msg10169 (view) Author: attila.lendvai Date: 2010-03-12.11:17:25
i'm also bit by this when converting a repo with quite a few conflicts.

to make life more fun, i think the resulting darcs2 repo converted by 
2.3.1 is corrupt: http://bugs.darcs.net/issue1763
msg10732 (view) Author: mornfall Date: 2010-04-14.15:43:43
I have done a little experiment using GHC profiler. For all I can say, the Convert code did not 
change at all since 2.3, and I see this:

           darcs-HEAD+prof +RTS -P -RTS convert mmap mmap-darcs2

        total time  =       84.16 secs   (4208 ticks @ 20 ms)
        total alloc = 16,109,995,604 bytes  (excludes profiling overheads)


COST CENTRE                    MODULE               %time %alloc  ticks     bytes
commute_filedir                Darcs.Patch.Prim      14.5    0.0    612     15723
speedy_commute                 Darcs.Patch.Prim      13.7    0.1    577   4345525
commute_a72FB                  Darcs.Patch.Real      10.6    0.0    448       243
==_a4RA9                       Darcs.Patch.Prim       8.3   22.6    351 910917284
commuteHunk                    Darcs.Patch.Prim       7.4   14.2    310 569936181
commute_no_conflicts_a72SH     Darcs.Patch.Real       6.7   10.1    280 407613613
unsafeCompare_a4RIe            Darcs.Patch.Prim       4.0    0.0    170         0
is_filepatch                   Darcs.Patch.Prim       3.4    5.4    143 217615670
headPermutationsFL             Darcs.Patch.Permutations   2.7    7.8    114 313186829
everything_else_commute        Darcs.Patch.Prim       2.6    0.0    110         0
assertConsistent               Darcs.Patch.Real       2.5    0.0    104         0
commute_a4RIE                  Darcs.Patch.Prim       2.4    2.7    100 109586920
lengthFL                       Darcs.Witnesses.Ordered   2.1    4.4     89 178085076

The same conversion with darcs-2.3 is instantaneous (compared to half a minute without profiling 
and well over a minute with profiling on darcs-2.4/HEAD). For all I can say, something went 
horribly wrong in our commutation functions. (All this gets forced in tentativelyMergePatches, in 
this "merge": pend' :/\: pw <- return $ merge (pc :\/: anonpend :>: NilFL)).

It *might* be due to pend being something else in 2.4 than it is in 2.3, I'll investigate that 
later. I have expected it to be empty, but it's not in 2.4... I'll check with 2.3 as soon as I 
manage to build it.
msg10741 (view) Author: dino Date: 2010-04-16.16:21:37
Looking into this a bit I found:

- Using the HTab test d1 repo attached to issue 1232 I see this problem
happening

- I can still see the problem after obl all but 20 of the 131 patches
from HTab

- I was unable so far to make a new repository from scratch (tried 10
patches) that exhibits the problem with either --hashed or
--old-fashioned-inventory
msg10875 (view) Author: ganesh Date: 2010-04-29.04:46:49
Also, darcs convert is producing spurious messages about conflicts. This 
is caused by the working copy not being updated after each chunk of 
patches is converted, so the next chunk of patches causes a conflict 
with working. The message is harmless as it's only the working copy that 
is left in a mess, and at the very end it gets reverted anyway, but the 
profile below suggests that these conflicts may also be the cause of the 
massive slowdown.

My suspicion is that something has changed in the merging code during 
some refactoring. Also, I don't think darcs convert actually needs to do 
a merge anyway - see http://lists.osuosl.org/pipermail/darcs-users/2010-
April/023787.html
msg10881 (view) Author: kowey Date: 2010-04-30.13:01:07
Petr has a patch for this; I think it just needs some minor tweaking and
resubmsission for the 2.4 line
msg10927 (view) Author: kowey Date: 2010-05-04.21:49:16
Can you do us a favour, Alex?

Could you apply patch226 to HEAD darcs and see if it converts faster?
msg10943 (view) Author: mornfall Date: 2010-05-05.14:58:56
The following patch updated the status of issue1760 to be resolved:

* Resolve issue1760: Fix working directory handling in Commands.Convert. 
Ignore-this: cf8e9c87fce4e5a0b6f26a3a265a087d
msg11037 (view) Author: alexsuraci Date: 2010-05-10.18:31:44
Sorry for the delay, but I can confirm that this seems to be fixed.
msg11386 (view) Author: mornfall Date: 2010-06-13.17:07:14
The following patch updated the status of issue1760 to be resolved-in-stable:

* Resolve issue1760: Fix working directory handling in Commands.Convert. 
Ignore-this: cf8e9c87fce4e5a0b6f26a3a265a087d
History
Date User Action Args
2010-03-10 00:07:21alexsuracicreate
2010-03-10 11:08:46koweysetstatus: unknown -> needs-reproduction
nosy: + kowey
topic: + Performance, Regression
messages: + msg10151
2010-03-12 11:17:27attila.lendvaisetnosy: + attila.lendvai
messages: + msg10169
2010-04-14 15:43:44mornfallsetnosy: + mornfall
messages: + msg10732
2010-04-16 16:21:38dinosetnosy: + dino
messages: + msg10741
2010-04-29 04:46:50ganeshsetnosy: + ganesh
messages: + msg10875
2010-04-30 13:01:09koweysetstatus: needs-reproduction -> has-patch
assignedto: mornfall
topic: + Target-2.4
messages: + msg10881
2010-05-04 21:49:16koweysetmessages: + msg10927
2010-05-05 14:58:57mornfallsetstatus: has-patch -> resolved
messages: + msg10943
2010-05-10 18:31:45alexsuracisetstatus: resolved -> unknown
messages: + msg11037
2010-05-10 18:53:08mornfallsetstatus: unknown -> resolved
2010-06-13 17:07:14mornfallsetstatus: resolved -> resolved-in-stable
messages: + msg11386
2010-06-15 21:31:33adminsetmilestone: 2.4.x
2010-06-15 21:31:35adminsettopic: - Target-2.4
2010-06-15 22:14:24adminsetstatus: resolved-in-stable -> resolved
2010-06-15 22:14:25adminsetresolvedin: 2.5.0
2010-06-16 14:38:46koweysetresolvedin: 2.5.0 -> 2.4.x