Hi,

this patch should fix the convert performance regression and the spurious "We
have conflicts in" messages during convert in 2.4.

I have done it on top of noslurps to avoid further conflict proliferation, but
it could be re-done on top of current mainline and conflicts could be
re-resolved (or I could try amending in the noslurps branch).

Yours,
   Petr.

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

Thu Feb 11 01:13:26 CET 2010  Petr Rockai <me@mornfall.net>
  * Use stock setScriptsExecutable from Darcs.Repository in Commands.Convert.

Wed Apr 28 22:50:30 CEST 2010  Petr Rockai <me@mornfall.net>
  * Resolve conflicts in Commands.Convert.

Thu Apr 29 12:48:11 CEST 2010  Petr Rockai <me@mornfall.net>
  * Resolve issue1760: Fix working directory handling in Commands.Convert.

Attachments

• unnamed
• use-stock-setscriptsexecutable-from-darcs_repository-in-commands_convert_.dpatch

Petr Rockai wrote:

> this patch should fix the convert performance regression and the
> spurious "We have conflicts in" messages during convert in 2.4. 
> 
> I have done it on top of noslurps to avoid further conflict
> proliferation, but it could be re-done on top of current mainline and
> conflicts could be re-resolved (or I could try amending in the
> noslurps branch).   

I'd suggest this patch should go into 2.4, so might be best to redo it
for that branch and then merge onto mainline/noslurps and live with the
conflicts. Reinier/Eric, could you confirm whether you want it for 2.4?

Ganesh

=============================================================================== 
Please access the attached hyperlink for an important electronic communications disclaimer: 
http://www.credit-suisse.com/legal/en/disclaimer_email_ib.html 
===============================================================================

On Thu, Apr 29, 2010 at 17:08:05 +0100, Sittampalam, Ganesh wrote:
> I'd suggest this patch should go into 2.4, so might be best to redo it
> for that branch and then merge onto mainline/noslurps and live with the
> conflicts. Reinier/Eric, could you confirm whether you want it for 2.4?

+1

Seems like a fairly non-invasive change which could people a lot.

-- 
Eric Kow <http://www.nltg.brighton.ac.uk/home/Eric.Kow>
PGP Key ID: 08AC04F9

This is a direct-on-mainline patch for issue1760. I will amend the noslurps
branch when this is pushed onto mainline.

Yours,
   Petr.

1 patch for repository http://darcs.net:

Sun May  2 12:57:06 CEST 2010  Petr Rockai <me@mornfall.net>
  * Resolve issue1760: Fix working directory handling in Commands.Convert.

Attachments

• resolve-issue1760_-fix-working-directory-handling-in-commands_convert_.dpatch
• unnamed

I confirm that this applies to branch-2.4.  Could you provide a tiny bit
more explanation, Petr?

How does this solve the performance regression (and also eliminate the
spurious conflicts message?)

Oh, I think all the explanation I'm looking for is in Ganesh's comment
that "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. "

On Sun, May 02, 2010 at 11:10:41 +0000, Petr Ročkai wrote:
> -          applySome xs = do tentativelyMergePatches repository "convert" (AllowConflicts:opts) NilFL xs
> +          applySome xs = do Sealed pw <- tentativelyMergePatches repository "convert" (AllowConflicts:opts) NilFL xs
>                              finalizeRepositoryChanges repository -- this is to clean out pristine.hashed
>                              revertRepositoryChanges repository
> hunk ./src/Darcs/Commands/Convert.lhs 217
> +                            revertable $ applyToWorking repository opts pw
> +                            invalidateIndex repository

Petr has blessed my reading of this patch on IRC, so I'm pushing it in
just to get as much Darcs 2.4.2 work out of the way as soon as possible.

Eric: is it OK for me to push the issue1760 (convert regression) patch?
      I just saw that it applies to working on the fly (which if I understand
      correctly, avoids conflicts building up in working)
      and invalidates the index (which seems to make sense as a consequence
      of applying to working)

Petr: That's the gist. In the end, however, it would still
      be better to use Ganesh's tentativelyAddPatch approach instead of the
      current one. Possibly for 2.5.

-- 
Eric Kow <http://www.nltg.brighton.ac.uk/home/Eric.Kow>
PGP Key ID: 08AC04F9

This patch bundle (with 1 patches) was just applied to the repository http://darcs.net/.
This message was brought to you by DarcsWatch
http://darcswatch.nomeata.de/repo_http:__darcs.net_.html#bundle-aa86b24b9f15869f3dcfc92a3a5a9a5c8ccc5b5d

This patch bundle (with 1 patches) was just applied to the repository http://darcs.net/reviewed.
This message was brought to you by DarcsWatch
http://darcswatch.nomeata.de/repo_http:__darcs.net_reviewed.html#bundle-aa86b24b9f15869f3dcfc92a3a5a9a5c8ccc5b5d