This is an archive of the discontinued Mercurial Phabricator instance.

Differential D4582

censor: use a reasonable amount of memory
ClosedPublic

Authored by valentin.gatienbaron on Sep 13 2018, 4:31 PM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Commits
rHGa658f97c1ce4: censor: use a reasonable amount of memory
Summary

Before this change, trying to censor some random revision uses an ever
increasing amount of memory (I stopped at 20GB, but it was by no means
finished), presumably because these contexts have a lot of
information that is kept alive.
After this change, the memory usage plateaus quickly.

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

indygreg added a subscriber: indygreg.Sep 13 2018, 4:49 PM

This effectively changes things from a list to a generator. That means we can only iterate headctxs once.

We do only iterate headctxs once, so this is safe. But it isn't a good practice to leave a generator variable around in the local scope.

Perhaps we could rewrite this as a normal for loop that iteratively builds up heads using .append().

durin42 added a subscriber: durin42.Sep 13 2018, 7:01 PM
In D4582#69941, @indygreg wrote:

This effectively changes things from a list to a generator. That means we can only iterate headctxs once.
We do only iterate headctxs once, so this is safe. But it isn't a good practice to leave a generator variable around in the local scope.
Perhaps we could rewrite this as a normal for loop that iteratively builds up heads using .append().

I'm honestly fine with this change as-stated. Would you feel better if it was called headctxsgen instead?

(the for-loop-and-append is probably slightly slower, but I haven't benchmarked it)

Closed by commit rHGa658f97c1ce4: censor: use a reasonable amount of memory (authored by valentin.gatienbaron). · Explain WhySep 14 2018, 6:11 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mhgext/censor.py (7 lines)

Diff 11049

hgext/censor.py

raise error.Abort(_('invalid revision identifier %s') % rev) raise error.Abort(_('invalid revision identifier %s') % rev)
try: try:
fctx = ctx.filectx(path) fctx = ctx.filectx(path)
except error.LookupError: except error.LookupError:
raise error.Abort(_('file does not exist at revision %s') % rev) raise error.Abort(_('file does not exist at revision %s') % rev)
fnode = fctx.filenode() fnode = fctx.filenode()
headctxs = [repo[c] for c in repo.heads()] heads = []
heads = [c for c in headctxs if path in c and c.filenode(path) == fnode] for headnode in repo.heads():
c = repo[headnode]
if path in c and c.filenode(path) == fnode:
heads.append(c)
if heads: if heads:
headlist = ', '.join([short(c.node()) for c in heads]) headlist = ', '.join([short(c.node()) for c in heads])
raise error.Abort(_('cannot censor file in heads (%s)') % headlist, raise error.Abort(_('cannot censor file in heads (%s)') % headlist,
hint=_('clean/delete and commit first')) hint=_('clean/delete and commit first'))
wp = wctx.parents() wp = wctx.parents()
if ctx.node() in [p.node() for p in wp]: if ctx.node() in [p.node() for p in wp]:
raise error.Abort(_('cannot censor working directory'), raise error.Abort(_('cannot censor working directory'),