D674: filemerge: use arbitraryfilectx for backup files

phillco (Phil Cohen) phabricator at mercurial-scm.org
Wed Sep 13 01:49:14 EDT 2017


phillco added inline comments.

INLINE COMMENTS

> martinvonz wrote in filemerge.py:744
> The documentation for filecmp.cmp() says
> 
>   Unless shallow is given and is false, files with identical os.stat() signatures are taken to be equal.
> 
> Are we losing out on that optimization with this patch (when not doing in-memory merge). Do you have any sense of how relevant that optimization is?

Yes, and it's a bigger impact than past changes of this nature, since each merged file, and its backup file, will be read again. So I think we need some way to reintroduce this fast-path for two disk-backed files inside `cmp`.

I'd propose adding something like this to `filectx`:

  def ondisk():
     """Returns True iff this filectx is directly backed by a file in the filesystem and not some other abstraction. If so, callers can run system file functions on it for better performance.
     """

It'd be True only for `workingfilectx`s and `abstractfilectx`s.

Then, inside `abstractfilectx.cmp()`, check if both the caller and other are on-disk and use filecmp in that case.

It's a naive first take though, so improvements are appreciated.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D674

To: phillco, #hg-reviewers
Cc: sid0, martinvonz, mercurial-devel


More information about the Mercurial-devel mailing list