( )⚙ D11956 exchange: add fast path for subrepo check on push

This is an archive of the discontinued Mercurial Phabricator instance.

Differential D11956

exchange: add fast path for subrepo check on push
ClosedPublic

Authored by joerg.sonnenberger on Jan 2 2022, 7:29 PM.

Details

Reviewers
Alphare
Group Reviewers
hg-reviewers
Commits
rHG28f0092ec89f: exchange: add fast path for subrepo check on push
Summary

Try to check if .hgsub and .hgsubstate exist at all before looking
for them in every changeset to be pushed. The latter can be quite
expensive for large repositories and the existance check is almost free.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Alphare added a subscriber: Alphare.Jan 3 2022, 5:41 AM
Alphare added inline comments.
mercurial/exchange.py
532

That seems like a very wide net to throw. Do we have a more specific way of falling back?

joerg.sonnenberger added inline comments.Jan 3 2022, 7:00 AM
mercurial/exchange.py
532

The problem here is remotefilelog, which has a custom hag for .hgtags and otherwise throws a runtime error. So no, I don't think there is a more specific way without changing the implementation.

Alphare requested changes to this revision.Jan 4 2022, 11:58 AM
Alphare added inline comments.
mercurial/exchange.py
532

Can't we either change the exception to be more fine-grained in remotefilelog (and still inherit from RuntimeError to not break anything) or detect a remotefilelog repo and early return? I'm really not comfortable catching this wide.

This revision now requires changes to proceed.Jan 4 2022, 11:58 AM
mharbison72 added a subscriber: mharbison72.Jan 5 2022, 10:46 AM
mharbison72 added inline comments.
mercurial/exchange.py
530

I assume this checks if the file exists anywhere in history (since it doesn't mention a rev), to handle the case of the files being removed? (I don't see the file() function on the localrepository class, so I'm not sure what this is doing)

Alphare accepted this revision.Jan 17 2022, 4:24 AM
This revision is now accepted and ready to land.Jan 17 2022, 4:24 AM

Revision Contents
Changeset List

Diff 31659

hgext/remotefilelog/README.md

1. The extension must be used with Mercurial 3.3 (commit d7d08337b3f6) or higher (earlier versions of the extension work with earlier versions of Mercurial though, up to Mercurial 2.7). 1. The extension must be used with Mercurial 3.3 (commit d7d08337b3f6) or higher (earlier versions of the extension work with earlier versions of Mercurial though, up to Mercurial 2.7).
2. remotefilelog has only been tested on linux with case-sensitive filesystems. It should work on other unix systems but may have problems on case-insensitive filesystems. 2. remotefilelog has only been tested on linux with case-sensitive filesystems. It should work on other unix systems but may have problems on case-insensitive filesystems.
3. remotefilelog only works with ssh based Mercurial repos. http based repos are currently not supported, though it shouldn't be too difficult for some motivated individual to implement. 3. remotefilelog only works with ssh based Mercurial repos. http based repos are currently not supported, though it shouldn't be too difficult for some motivated individual to implement.
4. Tags are not supported in completely shallow repos. If you use tags in your repo you will have to specify `excludepattern=.hgtags` in your client configuration to ensure that file is downloaded. The include/excludepattern settings are experimental at the moment and have yet to be deployed in a production environment. 4. Tags are not supported in completely shallow repos. If you use tags in your repo you will have to specify `excludepattern=.hgtags` in your client configuration to ensure that file is downloaded. The include/excludepattern settings are experimental at the moment and have yet to be deployed in a production environment.
5. A few commands will be slower. `hg log <filename>` will be much slower since it has to walk the entire commit history instead of just the filelog. Use `hg log -f <filename>` instead, which remains very fast. 5. Similarly, subrepositories should not be used with completely shallow repos. Use `excludepattern=.hgsub*` in your client configuration to ensure that the files are downloaded.
6. A few commands will be slower. `hg log <filename>` will be much slower since it has to walk the entire commit history instead of just the filelog. Use `hg log -f <filename>` instead, which remains very fast.
Contributing Contributing
============ ============
Patches are welcome as pull requests, though they will be collapsed and rebased to maintain a linear history. Tests can be run via: Patches are welcome as pull requests, though they will be collapsed and rebased to maintain a linear history. Tests can be run via:
:::bash :::bash
cd tests cd tests

hgext/remotefilelog/remotefilelog.py

return nodetext != text return nodetext != text
def __nonzero__(self): def __nonzero__(self):
return True return True
__bool__ = __nonzero__ __bool__ = __nonzero__
def __len__(self): def __len__(self):
if self.filename == b'.hgtags': if self.filename in (b'.hgtags', b'.hgsub', b'.hgsubstate'):
# The length of .hgtags is used to fast path tag checking. # Global tag and subrepository support require access to the
# remotefilelog doesn't support .hgtags since the entire .hgtags # file history for various performance sensitive operations.
# history is needed. Use the excludepattern setting to make # excludepattern should be used for repositories depending on
# .hgtags a normal filelog. # those features to fallback to regular filelog.
return 0 return 0
raise RuntimeError(b"len not supported") raise RuntimeError(b"len not supported")
def heads(self): def heads(self):
# Fake heads of the filelog to satisfy hgweb. # Fake heads of the filelog to satisfy hgweb.
return [] return []

mercurial/exchange.py

"""Run all discovery steps""" """Run all discovery steps"""
for stepname in pushdiscoveryorder: for stepname in pushdiscoveryorder:
step = pushdiscoverymapping[stepname] step = pushdiscoverymapping[stepname]
step(pushop) step(pushop)
def _checksubrepostate(pushop): def _checksubrepostate(pushop):
"""Ensure all outgoing referenced subrepo revisions are present locally""" """Ensure all outgoing referenced subrepo revisions are present locally"""
repo = pushop.repo
# If the repository does not use subrepos, skip the expensive
# manifest checks.
if not len(repo.file(b'.hgsub')) or not len(repo.file(b'.hgsubstate')):
return
mharbison72Unsubmitted
Not Done

I assume this checks if the file exists anywhere in history (since it doesn't mention a rev), to handle the case of the files being removed? (I don't see the file() function on the localrepository class, so I'm not sure what this is doing)

mharbison72: I assume this checks if the file exists anywhere in history (since it doesn't mention a rev)…
for n in pushop.outgoing.missing: for n in pushop.outgoing.missing:
AlphareUnsubmitted
Not Done

That seems like a very wide net to throw. Do we have a more specific way of falling back?

Alphare: That seems like a very wide net to throw. Do we have a more specific way of falling back?
joerg.sonnenbergerAuthorUnsubmitted
Not Done

The problem here is remotefilelog, which has a custom hag for .hgtags and otherwise throws a runtime error. So no, I don't think there is a more specific way without changing the implementation.

joerg.sonnenberger: The problem here is remotefilelog, which has a custom hag for .hgtags and otherwise throws a…
AlphareUnsubmitted
Not Done

Can't we either change the exception to be more fine-grained in remotefilelog (and still inherit from RuntimeError to not break anything) or detect a remotefilelog repo and early return? I'm really not comfortable catching this wide.

Alphare: Can't we either change the exception to be more fine-grained in remotefilelog (and still…
ctx = pushop.repo[n] ctx = repo[n]
if b'.hgsub' in ctx.manifest() and b'.hgsubstate' in ctx.files(): if b'.hgsub' in ctx.manifest() and b'.hgsubstate' in ctx.files():
for subpath in sorted(ctx.substate): for subpath in sorted(ctx.substate):
sub = ctx.sub(subpath) sub = ctx.sub(subpath)
sub.verify(onpush=True) sub.verify(onpush=True)
@pushdiscovery(b'changeset') @pushdiscovery(b'changeset')

relnotes/next

== New Experimental Features == == New Experimental Features ==
== Bug Fixes == == Bug Fixes ==
The `--no-check` and `--no-merge` now properly overwrite the behavior from `commands.update.check`. The `--no-check` and `--no-merge` now properly overwrite the behavior from `commands.update.check`.
== Backwards Compatibility Changes == == Backwards Compatibility Changes ==
The remotefilelog extension now requires an appropiate excludepattern
for subrepositories.
== Internal API Changes == == Internal API Changes ==
The following functions have been removed: The following functions have been removed:
Miscellaneous: Miscellaneous: