This is an archive of the discontinued Mercurial Phabricator instance.

[RFC] rust-revlog: create nodemap on the fly if it's not persisted
Needs ReviewPublic

Authored by martinvonz on Apr 11 2022, 12:07 PM.


Group Reviewers

We can create a nodemap in memory when there isn't a persistent
one. When listing all commits in the hg repo, it sped up the
equivalent of hg log -T '{node}\n' --hidden (but run with my hobby
project's hg backend) from 123 s to 2.55 s.

What do we actually want to do here? Calculate the map after we've
done a few O(n) node-to-rev requests? I think that's what the C
implementation does.

Diff Detail

rHG Mercurial
No Linters Available
No Unit Test Coverage

Event Timeline

martinvonz created this revision.Apr 11 2022, 12:07 PM

The C implementation indeed builds the nodemap after 3 misses. We will probably want to do the same thing here, however the Rust implementation will require more work because we don't want our getter methods to all take &mut self.

For now, this looks good. Small revlogs will not be affected and large operations on the changelog/manifest call for a persistent nodemap anyway, so why not. If we end up needing to do something more clever down the line (like I suspect we will, given revlogv2 at least), we will think about how to organize our types to encode that API in a clear fashion... or just use interior mutability for the nodemap.


.expect("node should exist in index")

marmoute added a subscriber: marmoute.EditedApr 12 2022, 5:45 AM

Note that this should not be a practical issue right now, as rhg is not accessing multiple revision yet and hg use an intermediate layer that use the C caching if persistent nodemap is missing.
So this is not a problem for mercurial as is. Improving this in general is still a good idea.

So this is not a problem for mercurial as is. Improving this in general is still a good idea.

As mentioned in the commit message, it was when using my hobby project's hg backend. That's

martinvonz updated this revision to Diff 33191.Apr 16 2022, 1:47 AM
martinvonz updated this revision to Diff 33396.Mon, May 9, 3:13 PM