Currently, changelog reading decodes read values. This is wasteful
because a lot of times consumers aren't interested in some of these
values.
This patch changes description decoding to occur in changectx as
needed.
revsets reading changelog entries appear to speed up slightly:
revset #7: author(lmoscovicz)
plain
0) 0.906329
1) 0.872653
revset #8: author(mpm)
plain
0) 0.903478
1) 0.878037
revset #9: author(lmoscovicz) or author(mpm)
plain
0) 1.817855
1) 1.778680
revset #10: author(mpm) or author(lmoscovicz)
plain
0) 1.837052
1) 1.764568
_filerev depends on the filelog implementation using revlogs and linkrevs.
Alternative implementations, like remotefilelog, do not have rev numbers, so
this call fails. Replacing it with _filenode means it doesn't rely on rev
numbers, and doesn't cost anything extra, since _filerev is using _filenode
under the hood anyway.
When memctx is asked for a manifest, it constructs one by merging the p1
manifest, and the changes that are on top. For the changes on top, it was
previously using p1.node() as the file entries parent, which actually returns
the commit node that the p1 linkrev points at! Which is entirely incorrect.
The fix is to use p1.filenode() instead, which returns the parent file node as
desired.
I don't know how to execute this or make it have a visible effect, so I'm not
sure how to test it. It was noticed because asking for the linkrev is an
expensive operation when using the remotefilelog extension and this was causing
performance regressions with commit.
In a4119550f1e1 (context: clarify why we don't compare file contents
when nodeid differs, 2016-01-12), I also changed "node2 != _newnode"
into "self.rev() is not None". I don't remember why. They are similar,
but the former also catches the case where the file is clean in the
dirstate (so node2 is not _newnode), but different from the "other"
context. This resulted in unnecessary file content comparison a few
lines further down in the code. Let's just back out the code change.
Thanks to Durham Goode for spotting this.
util.cachefunc stores all arguments as the cache key. For filectxfn
functions, the arguments include the memctx instance. This creates a
cycle where memctx._filectxfn references self. This causes a memory
leak.
We break the cycle by implementing our own memoizing function that
only uses the path as the cache key. Since each memctx has its own
cache instance, there is no concern about invalid cache hits.
This backs out 7e679fd51132 (status: change + back out == clean (API),
2016-01-04). Although correct, it turned out that it was just too
slow. For example, 'hg status --rev .~1000 --rev .' on the Mozilla
repo went from <1s to >30s on cold disk. So we go back to reporting
reverted changes as modified instead of clean. These are rare anyway,
as suggested by the fact that it had been broken since before
Mercurial 2.0.
After backing out a change, so the file contents is equal to a
previous revision of itself, we currently report the status between
the two equal revisions as modified. This is because
context._buildstatus() reports any file whose new nodeid is not equal
to _newnode as modified. That magic nodeid is given only to files
added or modified in the working directory, so any file whose nodeid
has changed between two revisions will be reported as modified.
Fix by simply comparing the file contents for all cases where the
nodeid changed, whether they are in the working copy or committed.
Marking with (API) as it subtly changes the semantics of the method.
After just changing the flag on a file, plain 'hg status' will report
the file as modified. However, after reverting a file to a previous
revision's state and changing the flag, it will be reported as clean.
Fix by comparing the flags that were previously ignored in
context._buildstatus().
Before this change, asking for file from history (eg: 'hg cat -r 42 foo/bar')
could fail because of the current content of the working copy (eg: current
"foo" being a symlink). As the working copy state have no influence on the
content of the history, we can safely skip these checks.
The working copy context class have a different 'match'
implementation. That implementation still use the repo.auditor will
still catch symlink traversal.
I've audited all stuff calling "match" and they all go through a ctx
in a sensible way. The most unclear case was diff which still seemed
okay. You raised my paranoid level today and I double checked through
tests. They behave properly.
The odds of someone using the wrong (matching with a changectx for
operation that will eventually touch the file system) is non-zero
because you are never sure of what people will do. But I dunno if we
can fight against that. So I would not commit to "never" for "at this
level" and "in the future" if someone write especially bad code.
However, as a last defense, the vfs itself is running path auditor in
all cases outside of .hg/. So I think anything passing the 'matcher'
for buggy reason would growl at the vfs layer.
Previously, when extdiff was called on two changesets where
a subrepository had been removed, an unexpected KeyError would
be raised.
Now, the missing subrepository will be ignored. This behavior
mirrors the behavior in diffordiffstat from cmdutil.py line
~1138-1153. The KeyError is caught and the revision is
set to None.
try/catch of LookupError around matchmod.narrowmatcher and
sub.status is removed, as LookupError is not raised anywhere
within those methods or deeper calls.
Resolving parents requires reading from the changelog, which is a few
attributes and function calls away. Parents lookup occurs surprisingly
often. Micro optimizing the code to avoid redundant lookups of parents
appears to make `hg log` on my Firefox repo a little faster:
before: 24.91s
after: 23.76s
delta: -1.15s (95.4% of original)
This patch avoids some extra attribute lookups and list mutations.
This micro-optimization seems to result in a minor speedup for `hg log`
on my Firefox repo:
before: 25.35s
after: 24.91s
delta: -0.44s (98% of original)
Not the biggest gain. But every little bit helps.
We've globablly forced stat to return integer times which agrees with
our extension code, so this is no longer needed.
This speeds up status on mozilla-central substantially:
$ hg perfstatus
! wall 0.190179 comb 0.180000 user 0.120000 sys 0.060000 (best of 53)
$ hg perfstatus
! wall 0.275729 comb 0.270000 user 0.210000 sys 0.060000 (best of 36)
This will indicate whether this filectx represents a file that is *not* in a
changectx. This will be used by merge and filemerge code to know about when a
conflict is a change/delete conflict.
While this is kind of hacky, it is the least bad of all the alternatives. Other
options considered but rejected include:
- isinstance(fctx, ...) -- not very Pythonic, doesn't support duck typing
- fctx.size() is None -- the 'size()' call on workingfilectxes causes a disk stat
- fctx.filenode() == nullid -- the semantics around filenode are incredibly
confusing. In particular, for workingfilectxes, filenode() is always None no
matter whether the file is present on disk or in either parent. Having different
behavior for None versus nullid in the merge code is just asking for pain.
Thanks to Pierre-Yves David for early review feedback here.
We're going to introduce other sorts of filectxes very soon, and we'd like the
cmp method to function properly (i.e. commutatively) for them. The only way to
make that happen is for this cmp method to call into that specialized one if
that defines a custom comparator.
Now, 'dirstate.write(tr)' delays writing in-memory changes out, if a
transaction is running.
This may cause treating this revision as "the first bad one" at
bisecting in some cases using external hook process inside transaction
scope, because some external hooks and editor process are still
invoked without HG_PENDING and pending changes aren't visible to them.
'dirstate.write()' callers below in localrepo.py explicitly use 'None'
as 'tr', because they can assume that no transaction is running:
- just before starting transaction
- at closing transaction, or
- at unlocking wlock
af5de4d23fd4 introduced nice hexified display of missing nodes. It did however
also make missing 20 character revision specifications be shown as hex - very
confusing.
Users are often wrong and somehow specify revisions that don't exist. Nodes
will however rarely be missing ... and they will only look like a user provided
revision specification and be all ascii in 1 of 4*10**9.
With this change, missing revisions will only be hexified if they really look
like binary nodes. This change will thus improve the error reporting UI in the
common case and only very rarely make it confusing in the opposite direction of
how it was before.
The home of 'Abort' is 'error' not 'util' however, a lot of code seems to be
confused about that and gives all the credit to 'util' instead of the
hardworking 'error'. In a spirit of equity, we break the cycle of injustice and
give back to 'error' the respect it deserves. And screw that 'util' poser.
For great justice.
To detect change of a file without redundant comparison of file
content, dirstate recognizes a file as certainly clean, if:
(1) it is already known as "normal",
(2) dirstate entry for it has valid (= not "-1") timestamp, and
(3) mode, size and timestamp of it on the filesystem are as same as
ones expected in dirstate
This works as expected in many cases, but doesn't in the corner case
that changing a file keeps mode, size and timestamp of it on the
filesystem.
The timetable below shows steps in one of typical such situations:
---- ----------------------------------- ----------------
timestamp of "f"
----------------
dirstate file-
time action mem file system
---- ----------------------------------- ---- ----- -----
* *** ***
- 'hg transplant REV1 REV2 ...'
- transplanting REV1
....
N
- change "f", but keep size N
(via 'patch.patch()')
- 'dirstate.normal("f")' N ***
(via 'repo.commit()')
- transplanting REV2
- change "f", but keep size N
(via 'patch.patch()')
- aborted while patching
N+1
- release wlock
- 'dirstate.write()' N N N
- 'hg status' shows "r1" as "clean" N N N
---- ----------------------------------- ---- ----- -----
The most important point is that 'dirstate.write()' is executed at N+1
or later. This causes writing dirstate timestamp N of "f" out
successfully. If it is executed at N, 'parsers.pack_dirstate()'
replaces timestamp N with "-1" before actual writing dirstate out.
This issue can occur when 'hg transplant' satisfies conditions below:
- multiple revisions to be transplanted change the same file
- those revisions don't change mode and size of the file, and
- the 2nd or later revision of them fails after changing the file
The root cause of this issue is that files are changed without
flushing in-memory dirstate changes via 'repo.commit()' (even though
omitting 'dirstate.normallookup()' on files changed by 'patch.patch()'
for efficiency also causes this issue).
To detect changes of files correctly, this patch writes in-memory
dirstate changes out explicitly after marking files as clean in
'committablectx.markcommitted()', which is invoked via
'repo.commit()'.
After this change, timetable is changed as below:
---- ----------------------------------- ----------------
timestamp of "f"
----------------
dirstate file-
time action mem file system
---- ----------------------------------- ---- ----- -----
* *** ***
- 'hg transplant REV1 REV2 ...'
- transplanting REV1
....
N
- change "f", but keep size N
(via 'patch.patch()')
- 'dirstate.normal("f")' N ***
(via 'repo.commit()')
----------------------------------- ---- ----- -----
- 'dirsttate.write()' -1 -1
----------------------------------- ---- ----- -----
- transplanting REV2
- change "f", but keep size N
(via 'patch.patch()')
- aborted while patching
N+1
- release wlock
- 'dirstate.write()' -1 -1 N
- 'hg status' shows "r1" as "clean" -1 -1 N
---- ----------------------------------- ---- ----- -----
To reproduce this issue in tests certainly, this patch emulates some
timing critical actions as below:
- change "f" at N
'patch.patch()' with 'fakepatchtime.py' explicitly changes mtime
of patched files to "2000-01-01 00:00" (= N).
- 'dirstate.write()' via 'repo.commit()' at N
'fakedirstatewritetime.py' forces 'pack_dirstate()' to use
"2000-01-01 00:00" as "now", only if 'pack_dirstate()' is invoked
via 'committablectx.markcommitted()'.
- 'dirstate.write()' via releasing wlock at N+1 (or "not at N")
'pack_dirstate()' via releasing wlock uses actual timestamp at
runtime as "now", and it should be different from the "2000-01-01
00:00" of "f".
BTW, this patch doesn't test cases below, even though 'patch.patch()'
is used similarly in these cases:
1. failure of 'hg import' or 'hg qpush'
2. success of 'hg import', 'hg qpush' or 'hg transplant'
Case (1) above doesn't cause this kind of issue, because:
- if patching is aborted by conflicts, changed files are committed
changed files are marked as CLEAN, even though they are partially
patched.
- otherwise, dirstate are fully restored by 'dirstateguard'
For example in timetable above, timestamp of "f" in .hg/dirstate
is restored to -1 (or less than N), and subsequent 'hg status' can
detect changes correctly.
Case (2) always causes 'repo.status()' invocation via 'repo.commit()'
just after changing files inside same wlock scope.
---- ----------------------------------- ----------------
timestamp of "f"
----------------
dirstate file-
time action mem file system
---- ----------------------------------- ---- ----- -----
N *** ***
- make file "f" clean N
- execute 'hg foobar'
....
- 'dirstate.normal("f")' N ***
(e.g. via dirty check
or previous 'repo.commit()')
- change "f", but keep size N
- 'repo.status()' (*1)
(via 'repo.commit()')
---- ----------------------------------- ---- ----- -----
At a glance, 'repo.status()' at (*1) seems to cause similar issue (=
"changed files are treated as clean"), but actually doesn't.
'dirstate._lastnormaltime' should be N at (*1) above, because
'dirstate.normal()' via dirty check is finished at N.
Therefore, "f" changed at N (= 'dirstate._lastnormaltime') is forcibly
treated as "unsure" at (*1), and changes are detected as expected (see
'dirstate.status()' for detail).
If 'hg import' is executed with '--no-commit', 'repo.status()' isn't
invoked just after changing files inside same wlock scope.
But preceding 'dirstate.normal()' is invoked inside another wlock
scope via 'cmdutil.bailifchanged()', and in-memory changes should be
flushed at the end of that scope.
Therefore, timestamp N of clean "f" should be replaced by -1, if
'dirstate.write()' is invoked at N. It means that condition of this
issue isn't satisfied.
To detect change of a file without redundant comparison of file
content, dirstate recognizes a file as certainly clean, if:
(1) it is already known as "normal",
(2) dirstate entry for it has valid (= not "-1") timestamp, and
(3) mode, size and timestamp of it on the filesystem are as same as
ones expected in dirstate
This works as expected in many cases, but doesn't in the corner case
that changing a file keeps mode, size and timestamp of it on the
filesystem.
The timetable below shows steps in one of typical such situations:
---- ----------------------------------- ----------------
timestamp of "f"
----------------
dirstate file-
time action mem file system
---- ----------------------------------- ---- ----- -----
N -1 ***
- make file "f" clean N
- execute 'hg foobar'
- instantiate 'dirstate' -1 -1
- 'dirstate.normal("f")' N -1
(e.g. via dirty check)
- change "f", but keep size N
N+1
- release wlock
- 'dirstate.write()' N N
- 'hg status' shows "f" as "clean" N N N
---- ----------------------------------- ---- ----- -----
The most important point is that 'dirstate.write()' is executed at N+1
or later. This causes writing dirstate timestamp N of "f" out
successfully. If it is executed at N, 'parsers.pack_dirstate()'
replaces timestamp N with "-1" before actual writing dirstate out.
Occasional test failure for unexpected file status is typical example
of this corner case. Batch execution with small working directory is
finished in no time, and rarely satisfies condition (2) above.
This issue can occur in cases below;
- 'hg revert --rev REV' for revisions other than the parent
- failure of 'merge.update()' before 'merge.recordupdates()'
The root cause of this issue is that files are changed without
flushing in-memory dirstate changes via 'repo.commit()' (even though
omitting 'dirstate.normallookup()' on changed files also causes this
issue).
To detect changes of files correctly, this patch writes in-memory
dirstate changes out explicitly after marking files as clean in
'workingctx._checklookup()', which is invoked via 'repo.status()'.
After this change, timetable is changed as below:
---- ----------------------------------- ----------------
timestamp of "f"
----------------
dirstate file-
time action mem file system
---- ----------------------------------- ---- ----- -----
N -1 ***
- make file "f" clean N
- execute 'hg foobar'
- instantiate 'dirstate' -1 -1
- 'dirstate.normal("f")' N -1
(e.g. via dirty check)
----------------------------------- ---- ----- -----
- 'dirsttate.write()' -1 -1
----------------------------------- ---- ----- -----
- change "f", but keep size N
N+1
- release wlock
- 'dirstate.write()' -1 -1
- 'hg status' -1 -1 N
---- ----------------------------------- ---- ----- -----
To reproduce this issue in tests certainly, this patch emulates some
timing critical actions as below:
- timestamp of "f" in '.hg/dirstate' is -1 at the beginning
'hg debugrebuildstate' before command invocation ensures it.
- make file "f" clean at N
- change "f" at N
'touch -t 200001010000' before and after command invocation
changes mtime of "f" to "2000-01-01 00:00" (= N).
- invoke 'dirstate.write()' via 'repo.status()' at N
'fakedirstatewritetime.py' forces 'pack_dirstate()' to use
"2000-01-01 00:00" as "now", only if 'pack_dirstate()' is invoked
via 'workingctx._checklookup()'.
- invoke 'dirstate.write()' via releasing wlock at N+1 (or "not at N")
'pack_dirstate()' via releasing wlock uses actual timestamp at
runtime as "now", and it should be different from the "2000-01-01
00:00" of "f".
BTW, this patch also changes 'test-largefiles-misc.t', because adding
'dirstate.write()' makes recent dirstate changes visible to external
process.
Python 2.6 introduced the "except type as instance" syntax, replacing
the "except type, instance" syntax that came before. Python 3 dropped
support for the latter syntax. Since we no longer support Python 2.4 or
2.5, we have no need to continue supporting the "except type, instance".
This patch mass rewrites the exception syntax to be Python 2.6+ and
Python 3 compatible.
This patch was produced by running `2to3 -f except -w -n .`.
Some code cannot handle a subrepo based on the working directory (e.g.
sub.dirty()), so the caller must opt in. This will be useful for archive, and
perhaps some other commands. The git and svn methods where this is used may
need to be fixed up on a case by case basis.
Since node is None for workingctx, it can't use the base class
implementation of 'hex(self.node())'.
It doesn't appear that there are any current callers of this, but there will be
when archive supports 'wdir()'. My first thought was to use "{p1node}+", but
that would cause headaches elsewhere [1].
We should probably fix up localrepository.__getitem__ to accept this hash for
consistency, as a followup. This works, if the full hash is specified:
@@ -480,7 +480,7 @@
return dirstate.dirstate(self.vfs, self.ui, self.root, validate)
def __getitem__(self, changeid):
- if changeid is None:
+ if changeid is None or changeid == 'ff' * 20:
return context.workingctx(self)
if isinstance(changeid, slice):
return [context.changectx(self, i)
That differs from null, where it will accept any number of 0s, as long as it
isn't ambiguous.
[1] https://www.selenic.com/pipermail/mercurial-devel/2015-June/071166.html
Ultimately, this will be used by scmutil. The subrepo module already imports
it, so it can't import the subrepo module to access the underlying method.
The uncommit command in evolve needs to create memory context with given
extra parameters. This patch allows us to do that instead of always giving them
an empty value and having to override it afterwards.
Previously, the first added test printed the following:
$ hg files -S -r '.^' sub1/sub2/folder
sub1/sub2/folder: no such file in rev 9bb10eebee29
sub1/sub2/folder: no such file in rev 9bb10eebee29
sub1/sub2/folder/test.txt
One warning occured each time a subrepo was crossed into.
The second test ensures that the matcher copy stays in place. Without the copy,
the bad() function becomes an increasingly longer chain, and no message would be
printed out for a file missing in the subrepo because the predicate would match
in one of the replaced methods. Manifest doesn't know anything about subrepos,
so it needs help ignoring subrepos when complaining about bad files.
This will work for any command that creates its matcher via scmutil.match(), but
only the files command is tested here (both workingctx and basectx based tests).
The previous behavior was to completely ignore the files in the subrepo, even
though -S was given.
My first attempt was to teach context.walk() to optionally recurse, but once
that was in place and the complete file list was built up, the predicate test
would fail with 'path in nested repo' when a file in a subrepo was accessed
through the parent context.
There are two slightly surprising behaviors with this functionality. First, any
path provided inside the fileset isn't narrowed when it is passed to the
subrepo. I dont see any clean way to do that in the matcher. Fortunately, the
'subrepo()' fileset is the only one to take a path.
The second surprise is that status predicates are resolved against the subrepo,
not the parent like 'hg status -S' is. I don't see any way to fix that either,
given the path auditor error mentioned above.
This should avoid the bad performance in the following scenario. Before this
patch, on "hg annotate -r10000", p.rev() would walk changelog from 10000 to 3
because _descendantrev was 10000. With this patch, it walks from 5 to 3.
1 -- 2 -- 4 -- 5 -- ... -- 10000
\ 'p' 'f'
- 3 (grafted 3 to 4)
'p'
repo: https://hg.mozilla.org/releases/mozilla-beta/#4f80fecda802
command: hg annotate -r b0a57152fd14 browser/app/profile/firefox.js
before: 83.120 secs
after: 3.820 secs
This patch involves extra calls of narrow _adjustlinkrev(), but the cost of
them seems relatively small compared to wide _adjustlinkrev() calls eliminated
by this patch.
repo: http://selenic.com/repo/hg/#d668bc5b9a06
command: hg annotate mercurial/commands.py
before: 7.380 secs
after: 7.320 secs
repo: https://hg.mozilla.org/mozilla-central/#f214df6ac75f
command: hg annotate layout/generic/nsTextFrame.cpp
before: 5.070 secs
after: 5.050 secs
repo: https://hg.mozilla.org/releases/mozilla-beta/#4f80fecda802
command: hg annotate -r 4954faa47dd0 gfx/thebes/gfxWindowsPlatform.cpp
before: 1.600 secs
after: 1.620 secs
_ancestrycontext is necessary for fast lookup of _changeid. Because we can't
compute the ancestors from wctx, we skip to its parents. 'None' is not needed
to be included in _ancestrycontext because it is used for a membership test
of filelog revisions.
repo: https://hg.mozilla.org/releases/mozilla-beta/#062e49bcb2da
command: hg annotate -r 'wdir()' gfx/thebes/gfxWindowsPlatform.cpp
before: 51.520 sec
after: 1.780 sec
Before this patch, annotating working directory could include wrong revisions
that were hidden or belonged to different branches. This fixes wfctx.parents()
to set _descendantrev so that all ancestors can take advantage of the linkrev
adjustment introduced at a5aaaeedd6cb. _adjustlinkrev() can handle 'None'
revision thanks to bb19d597bbcd.
This series tries to fix wrong ancestry information on annotating working
directory. This change should slightly improves the readability of the next
patch.
