906be86990 recently changed to switch from:
self._rbcrevs[rbcrevidx:rbcrevidx + _rbcrecsize] = rec
to
pack_into(_rbcrecfmt, self._rbcrevs, rbcrevidx, node, branchidx)
This causes an exception if rbcrevidx is -1 (i.e. the nullrev). The old code
handled this because python handles out of bound sets to arrays gracefully. The
new code throws because the self._rbcrevs buffer isn't long enough to write 8
bytes to. Normally it would've been resized by the immediately preceding line,
but because the 0 length buffer is greater than the idx (-1) times the size, no
resize happens.
Setting the branch for the nullrev doesn't make sense anyway, so let's skip it.
This was caught by external tests in the Facebook extensions repo, but I've
added a test here that catches the issue.
Now that we have a general try except, we can move the error handling from the
individual writes in it.
Code will be reindented in the next changeset to help this on readability.
We now attempt to acquire a lock and write the branch cache within that lock.
This would prevent cache corruption when multiple processes try to write the cache
at the same time.
The code used self._rbcnamescount as if it was the length of self._names ...
but actually it is just the number of good entries on disk. This caused the
cache to be populated inefficiently. In some cases very inefficiently.
Instead of checking the length before lookup, just try a lookup in self._names
- that is also in most cases faster.
Comments and debug messages are tweaked to help understanding the issue
and the fix.
It was in some cases possible to end up writing to the cache file without
growing it first. The range assignment in _setcachedata would append instead of
writing at the requested position and thus write the new record in the wrong
place.
To fix this, we avoid looking up in too small caches, and when growing the
cache, do it right before writing the new record to it so we know it has been
done correctly.
If the seek for some reason fails (perhaps because the file is too short to
search to the requested position), make sure we seek to the start and rewrite
everything.
It is unknown if this fixes a real problem that ever happened.
New branch names are usually appended to the branch name file. If that fails or
the file has been modified by another process, it is rewritten. That left a
small opportunity that there could be references to non-existent entries in the
file while it was rewritten.
To avoid that, remove the revision branch cache file with the references to the
branch name file before rewriting the branch name file. Worst case, when
interrupted at the wrong time, the cache will be lost and rebuilt next time.
It is unknown if this fixes a real problem that ever happened.
Testing 'node in repo' requires constructing a changectx, which is a little
expensive. Testing 'repo.changelog.hasnode(node)' is notably faster. This
saves 10-20ms off of every command, when testing a few thousand nodes from the
branch cache.
I considered changing the implementation of localrepository.__contains__ so
every place would benefit from the change, but since
localrepository.__contains__ uses changectx to check if the commit exists, it
means it supports a wider range of possible inputs (like revs, hashes, '.',
etc), so it seemed unnecessarily risky.
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.
This is low-level branch map and cache manipulation code. It deserves to
live next to similar code in branchmap.py. Moving it also paves the road
for multiple consumers, such as a bundle2 part handler that receives
branch mappings from a remote.
This is largely a mechanical move, with only variable names and
indentation being changed.
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 .`.
This fixes the crash caused by "branch(null)" revset. No cache should be
necessary for nullrev because changelog.branchinfo(nullrev) does not involve
IO operation.
Note that the problem of "branch(wdir())" isn't addressed by this patch.
"wdir()" will raise TypeError in many places because of None. This is the
reason why "wdir()" is still experimental.
The warning has in some cases incorrectly attributed unrelated problems to rbc.
Instead, just do like the branch head cache does and stay quiet when reading
fails. The cache will be missing the first time a repo is used. It is a normal
situation and there is no reason to make a note of that.
An upcoming patch will establish per-filter tags caches. We'll want
to use the same cache validation logic as the branch cache. Prepare
for that by moving the logic for computing a filtered view hash
to somewhere central.
Previously we would only actually write the revbranchcache to disk if we were in
the middle of a write operation (like commit). Now we will also write it during
any read operation. The cache knows how to invalidate itself, so it shouldn't
become corrupt if multiple writers try at once (and the write-on-read
behavior/risk is the same as all our other caches).
Instead of writing the revbranchcache during updatecache (which often happens
too early, before the cache is even populated), let's run it as part of the
transaction finalizer. It still won't be written for read-only operations, but
that's no worse than it is today.
A future commit will remove the actual write that happens in updatecache().
This is also good prep for when all caches get moved into the transaction.
Previously the cache would populate completely the first time it was accessed.
This could take over a minute on larger repos. This patch changes it to update
incrementally. Only values that are read will be written, and it will only
rewrite as much of the file as strictly necessary.
This adds a magic value of '\0\0\0\0' to represent an empty cache entry. The
probability of this matching an actual commit hash prefix is tiny, so it's ok if
that's always considered a cache miss. This is also BC safe since any existing
entries with '\0\0\0\0' will just be considered misses.
Perf numbers:
Mozilla-central: hg --time log -r 'branch(mobile)' -T.
Cold Cache: 14.7s -> 15.1s (3% worse)
Warm Cache: 1.6s -> 2.1s (30% worse)
Mozilla-cental: hg perfbranchmap
2s -> 2.4s (20% worse)
hg: hg log -r 'branch(stable) & branch(default)'
Cold Cache: 3.1s -> 1.9s (40% better - because the old code missed the cache on
both branch() revset iterations, so it did twice the work)
Warm Cache: 0.2 -> 0.26 (30% worse)
internal huge repo: hg --time log -r 'tip & branch(default)'
Cold Cache: 65.4s -> 0.2s (327x better)
While this change introduces minor regressions when iterating over every commit
in a branch, it massively improves the cold cache time for operations which
touch a single commit. I feel the better O() is worth it in this case.
This moves the actual writing of entries to the cache to a separate function.
This will allow us to use it in multiple places. Ex: in one place we will write
dummy entries, and in another place we will write real data.
Previously we would instantiate the revbranchcache with a repo object, use it
briefly, then require it be passed in every time we wanted to fetch any
information. This seems unnecessary since it's obviously specific to that repo
(since it was constructed with it).
This patch stores the repo on the revbranchcache object, and removes the repo
parameter from the various functions on that class. This has the other nice
benefit of removing the double-revbranchcache-read that existed before (it was
read once for the branch revset, and once for the repo.revbranchcache).
Previously the revbranchcache was a field inside the branchmap. This is bad for
a couple reasons:
1) There can be multiple branchmaps per repo (one for each filter level). There
can only be one revbranchcache per repo. In fact, a revbranchcache could only
exist on a branchmap that was for the unfiltered view, so you could have
branchmaps exist for which you couldn't have a revbranchcache. It was funky.
2) The write lifecycle for the revbranchcache is going to be different from
the branchmap (branchmap is greedily written early on, revbranchcache
should be lazily computed and written).
This patch moves the revbranchcache to live as a field on the localrepo
(alongside self._branchmap). This will allow us to handle it's lifecycle
differently, which will let us move it to be lazily computed in future patches.
Transitioning to Mercurial versions with revision branch cache could be slow as
long as all operations were readonly (revset queries) and the cache would be
populated but not written back.
Instead, fall back to using the consistently slow path when readonly and the
cache doesn't exist yet. That avoids the overhead of populating the cache
without writing it back.
If not readonly, it will still populate all missing entries initially. That
avoids repeated writing of the cache file with small updates, and it also makes
sure a fully populated cache available for the readonly operations.
This change touches every module in which repository.opener was being used, and
changes it for the equivalent repository.vfs. This is meant to make it easier
to split the repository.vfs into several separate vfs.
It should now be possible to remove localrepo.opener.
self._rbcrevslen is used to keep track of the number of good records on disk.
It should thus not be updated before the records actually have been written to
disk.
The cache files are usually append only but will automatically be truncated and
recover in exceptional situations. Add a debug notice when such exceptional
situations are encountered.
The revbranchcache is read on demand before it will be used for updating the
branch map. It is written back when the branchmap is written and it will thus
use the same locking as branchmap. The revbranchcache instance is short-lived;
it is only stored in the branchmap from .update() is invoked and until .write()
is invoked. Branchmap already assume that the repo is locked in that case.
The use of revbranchcache for branch map updates will make sure that the
revbranchcache "always" is kept up-to-date.
The perfbranchmap benchmark is somewhat bogus, especially when we can see that
the caching makes a significant difference between the realistic case of a
first run and the rare case of rerunning it with a full cache. Here are some
'base' numbers on mozilla-central:
Before:
! wall 6.912745 comb 6.910000 user 6.840000 sys 0.070000 (best of 3)
After - initial, cache is empty:
! wall 7.792569 comb 7.790000 user 7.720000 sys 0.070000 (best of 3)
After - cache is full:
! wall 0.879688 comb 0.880000 user 0.870000 sys 0.010000 (best of 4)
The overhead when running with empty cache comes from checking, missing and
updating it every time.
Most of the performance improvement comes from not having to extract the branch
info from the changelog. The last doubling of performance comes from no longer
having to convert all branch names to local encoding but reuse the few already
converted branch names.
On the hg repo:
Before:
! wall 0.715703 comb 0.710000 user 0.710000 sys 0.000000 (best of 14)
After:
! wall 0.105489 comb 0.110000 user 0.110000 sys 0.000000 (best of 87)
It is expensive to retrieve the branch name of a revision. Very expensive when
creating a changectx and calling .branch() every time - slightly less when
using changelog.branchinfo().
Now, to speed things up, provide a way to cache the results on disk in an
efficient format. Each branchname is assigned a number, and for each revision
we store the number of the corresponding branch name. The branch names are
stored in a dedicated file which is strictly append only.
Branch names are usually reused across several revisions, and the total list of
branch names will thus be so small that it is feasible to read the whole set of
names before using the cache. It will however do that it might be more
efficient to use the changelog for retrieving the branch info for a single
revision.
The revision entries are stored in another file. This file is usually append
only, but if the repository has been modified, the file will be truncated and
the relevant parts rewritten on demand.
The entries for each revision are 8 bytes each, and the whole revision file
will thus be 1/8 of 00changelog.i.
Each revision entry contains the first 4 bytes of the corresponding node hash.
This is used as a check sum that always is verified before the entry is used.
That check is relatively expensive but it makes sure history modification is
detected and handled correctly. It will also detect and handle most revision
file corruptions.
This is just a cache. A new format can always be introduced if other
requirements or ideas make that seem like a good idea. Rebuilding the cache is
not really more expensive than it was to run for example 'hg log -b branchname'
before this cache was introduced.
This new method is still unused but promise to make some operations several
times faster once it actually is used.
Abandoning Python 2.4 would make it possible to implement this more efficiently
by using struct classes and pack_into. The Python code could probably also be
micro optimized or it could be implemented very efficiently in C where it would
be easy to control the data access.
We know that topological heads will not be ancestors of anything, so we filter
them out to potentially reduce the range of the ancestors computation.
On a strongly headed repo this gives humble speedup:
from 0.1984 to 0.1629
There is no reason to make multiple calls. This provides a massive speedup for
repo with a lot of heads.
On a strongly headed repo this gives humble speedup in simple case:
from 8.1097 to 5.1051
And massive speedup in other case:
from 7.8787 to 0.1984
We drop iterrevs which are not needed anymore. The know head are never a
descendant of the updated set. It was possible with the old strip code. This
simplification make the code easier to read an update.
