- User Since
- Jul 1 2017, 5:02 PM (179 w, 3 h)
Wed, Nov 18
Wed, Nov 11
Are the Rust extensions considered stable? I'm honestly unsure. I'd love to ship these. But I'd like confirmation from the people maintaining them before we sign off.
Tue, Nov 10
I think this should work.
Mon, Nov 9
Sat, Nov 7
Let's refactor the playback code to read from the journal file [handle]. That will preserve the order. There should be no meaningful performance implications here.
Why do we have to maintain the in-memory records at all? Don't we write these to the journal and journal.backupfiles files? What's wrong with reading this file handle? We only need to perform a findoffset() when incurring a split revlog or during rollback, right? Is there a noticeable performance overhead to performing that linear scan?
I want a 2nd set of eyes on this.
I _think_ this is safe. But I would appreciate an extra set of eyes on the changed code in revlog.py. Essentially what's happening here is we were previously stuffing the number of revisions in the revlog in the in-memory data structure so we could recover the inline revlog if we had to roll back a .i -> .d revlog split that was incurred as part of the transaction.
I was just attempting to read the transactions code as part of D9274.
Fri, Nov 6
Nov 5 2020
Nov 2 2020
I was skeptical when I saw this patch because complexity in C code in the Mercurial codebase tends to exist for good reasons since we tend to treat C code as toxic and a CVE waiting to happen. Anyway, I traced this cache to https://www.mercurial-scm.org/repo/hg-committed/log/a6fde9d789d9/mercurial/cext/revlog.c?patch=&linerange=349:358 / https://www.mercurial-scm.org/repo/hg-committed/rev/2cdd7e63211b.
@marmoute: both REVISION_FLAG_SIDEDATA and REVISION_FLAG_HASCOPIESINFO need documented in mercurial/helptext/internals/revlogs.txt. Could you please send a patch to ensure the technical specs stay in sync with reality?
Oct 22 2020
Oct 21 2020
Oct 19 2020
Oct 10 2020
Oct 9 2020
Oct 6 2020
Oct 5 2020
Sep 22 2020
This patch breaks my brain because our mucking about with sys.std* is wonky. We are doing really hacky things in both procutil.py and dispatch.py, including reassigning sys.std* in dispatch.py before this patch. I'd strongly prefer to have that logic consolidated.
This looks like a legit bug fix. And when I see patches like this, I wonder what circumstances led to its discovery :p
Wow - I had no clue this functionality existed! This functionality feels brittle and I'll gladly approve changes to simplify the locking code.
I didn't review the tests. But this seems like a decent first implementation of a revlog reader. The Python code for revlog reading is substantially more complex. Although a lot of that complexity has to do with extracting maximum performance out of Python. I am very curious how many of those optimizations we'll need in Rust to obtain a similar level of performance! Decompression often dominates revlog read times and I suspect Rust will enable us to leverage multithreading more easily, eventually allowing us to easily surpass Python's speed.
Sep 17 2020
This is so much cleaner.
Use of delattr is a bit wonky in the first place, as optional instance attributes feel like an anti-pattern to me. This is probably fine.
Before approving this, I'd like to have a quick conversation about the file name and documentation.
I may tweak the language of the user-facing messages in flight to make them slightly more readable.
This seems like useful functionality. However, I expect someone to complain about both the reading the extra config file and prepending its content on unshare. I'm tempted to say we should preemptively make this behavior configurable. Although I hate complexity. So let's wait until someone complains.
Sep 15 2020
This change may break compatibility with older clients. As written, if an old client and modern client write the tags cache file, they will obtain different locks and race to write the tags cache. If we want backwards compatibility, we need to take the store lock in addition to the working directory lock.
Aug 25 2020
I was going to push this but there's a test failure:
I'm approving this as an experimental quality feature. I think the technical direction is sound and the splitting of requirements corrects some long-standing soundness issues and will lead to a better end-user experience for shared repositories.