This is an archive of the discontinued Mercurial Phabricator instance.

Differential D1805

sparse: don't materialize sparse files during IMM
AcceptedPublic

Authored by phillco on Jan 4 2018, 1:01 PM.
Tags
None
Subscribers
durham

Details

Reviewers
quark
durham
Group Reviewers
Restricted Project
Summary

There's no need to do this and not doing so speeds up IMM tremendously when the sparse profile had to be adjusted.

Test Plan

I ran an IMM before and after this change. Both succeeded but the later version was
much faster.

Diff Detail

Repository
rFBHGX Facebook Mercurial Extensions
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

phillco created this revision.Jan 4 2018, 1:01 PM
Herald added a reviewer: Restricted Project. · View Herald TranscriptJan 4 2018, 1:01 PM
quark accepted this revision.Jan 4 2018, 1:33 PM
This revision is now accepted and ready to land.Jan 4 2018, 1:33 PM
durham accepted this revision.Jan 4 2018, 2:06 PM
durham added a subscriber: durham.

Discussed this in person. My concerns were that this would allow large action lists to propagate up and cause excess remotefilelog actions. Phil's response was that A) this only applies to rebases for now, and any action in the rebase is going to require downloading anyway (when it the sparse profile gets temporarily expanded below), and B) any action on a in memory context should probably not be filtered by sparseness anyway. I think the invariant in B is reasonable, and if we see it causing problems like A we can reevaluate how to fix that problem to meet invariant B.

phillco added a comment.Jan 4 2018, 2:21 PM

I'll add a comment.

phillco updated this revision to Diff 4690.Jan 4 2018, 3:20 PM
phillco added a comment.Jan 4 2018, 3:22 PM

I added a comment and also filed T24918295 for the wider task around allowing lazy filectx writes (but as Durham mentioned that's a strict improvement on the status quo).

phillco updated this revision to Diff 4691.Jan 4 2018, 3:23 PM
durham added inline comments.Jan 4 2018, 3:27 PM
hgext3rd/fbsparse.py
82–85

Looks fine. It might be nice to come up with some better terminology. I think of "working copy" as always referring to the physical on disk one, so maybe we need some other term for a mutable context (of which working copy would be one type).

phillco added inline comments.Jan 4 2018, 3:42 PM
hgext3rd/fbsparse.py
82–85

maybe "working context" would work

phillco added inline comments.Jan 5 2018, 12:39 AM
hgext3rd/fbsparse.py
82–85

"mutable" is tricky since technically on the on-disk working context is mutable too.

phillco updated this revision to Diff 4701.Jan 5 2018, 12:40 AM

Revision Contents
Changeset List

PathPackages
Mhgext3rd/fbsparse.py (5 lines)
Mhgext3rd/sparse.py (5 lines)

Diff 4701

hgext3rd/fbsparse.py

def _setupupdates(ui): def _setupupdates(ui):
def _calculateupdates(orig, repo, wctx, mctx, ancestors, branchmerge, *arg, def _calculateupdates(orig, repo, wctx, mctx, ancestors, branchmerge, *arg,
**kwargs): **kwargs):
"""Filter updates to only lay out files that match the sparse rules. """Filter updates to only lay out files that match the sparse rules.
""" """
actions, diverge, renamedelete = orig(repo, wctx, mctx, ancestors, actions, diverge, renamedelete = orig(repo, wctx, mctx, ancestors,
branchmerge, *arg, **kwargs) branchmerge, *arg, **kwargs)
if not util.safehasattr(repo, 'sparsematch'): # If the working context is inmemory (virtual), there's no need to apply
# the user's sparse rules at all (and in fact doing so would cause
# unexpected behavior in the real working copy).
if not util.safehasattr(repo, 'sparsematch') or wctx.isinmemory():
durhamUnsubmitted
Not Done

Looks fine. It might be nice to come up with some better terminology. I think of "working copy" as always referring to the physical on disk one, so maybe we need some other term for a mutable context (of which working copy would be one type).

durham: Looks fine. It might be nice to come up with some better terminology. I think of "working copy"…
phillcoAuthorUnsubmitted
Not Done

maybe "working context" would work

phillco: maybe "working context" would work
phillcoAuthorUnsubmitted
Not Done

"mutable" is tricky since technically on the on-disk working context is mutable too.

phillco: "mutable" is tricky since technically on the on-disk working context is mutable too.
return actions, diverge, renamedelete return actions, diverge, renamedelete
files = set() files = set()
prunedactions = {} prunedactions = {}
oldrevs = [pctx.rev() for pctx in wctx.parents()] oldrevs = [pctx.rev() for pctx in wctx.parents()]
oldsparsematch = repo.sparsematch(*oldrevs) oldsparsematch = repo.sparsematch(*oldrevs)
if branchmerge: if branchmerge:

hgext3rd/sparse.py

def _setupupdates(ui): def _setupupdates(ui):
def _calculateupdates(orig, repo, wctx, mctx, ancestors, branchmerge, *arg, def _calculateupdates(orig, repo, wctx, mctx, ancestors, branchmerge, *arg,
**kwargs): **kwargs):
"""Filter updates to only lay out files that match the sparse rules. """Filter updates to only lay out files that match the sparse rules.
""" """
actions, diverge, renamedelete = orig(repo, wctx, mctx, ancestors, actions, diverge, renamedelete = orig(repo, wctx, mctx, ancestors,
branchmerge, *arg, **kwargs) branchmerge, *arg, **kwargs)
if not util.safehasattr(repo, 'sparsematch'): # If the working context is inmemory (virtual), there's no need to apply
# the user's sparse rules at all (and in fact doing so would cause
# unexpected behavior in the real working copy).
if not util.safehasattr(repo, 'sparsematch') or wctx.isinmemory():
return actions, diverge, renamedelete return actions, diverge, renamedelete
files = set() files = set()
prunedactions = {} prunedactions = {}
oldrevs = [pctx.rev() for pctx in wctx.parents()] oldrevs = [pctx.rev() for pctx in wctx.parents()]
oldsparsematch = repo.sparsematch(*oldrevs) oldsparsematch = repo.sparsematch(*oldrevs)
if branchmerge: if branchmerge: