Sun, 13 Sep 2020 22:14:25 -0400 procutil: avoid using os.fork() to implement runbgcommand
Valentin Gatien-Baron <valentin.gatienbaron@gmail.com> [Sun, 13 Sep 2020 22:14:25 -0400] rev 46889
procutil: avoid using os.fork() to implement runbgcommand We ran into the following deadlock: - some command creates an ssh peer, then raises without explicitly closing the peer (hg id + extension in our case) - dispatch catches the exception, calls ui.log('commandfinish', ..) (the sshpeer is still not closed), which calls logtoprocess, which calls procutil.runbgcommand. - in the child of runbgcommand's fork(), between the fork and the exec, the opening of file descriptors triggers a gc which runs the destructor for sshpeer, which waits on ssh's stderr being closed, which never happens since ssh's stderr is held open by the parent of the fork where said destructor hasn't run Remotefilelog appears to have a hack around this deadlock as well. I don't know if there's more subtlety to it, because even though the problem is determistic, it is very fragile, so I didn't manage to reduce it. I can imagine three ways of tackling this problem: 1. don't run any python between fork and exec in runbgcommand 2. make the finalizer harmless after the fork 3. close the peer without relying on gc behavior This commit goes with 1, as forking without exec'ing is tricky in general in a language with gc finalizers. And maybe it's better in the presence of rust threads. A future commit will try 2 or 3. Performance wise: at low memory usage, it's an improvement. At higher memory usage, it's about 2x faster than before when ensurestart=True, but 2x slower when ensurestart=False. Not sure if that matters. The reason for that last bit is that the subprocess.Popen always waits for the execve to finish, and at high memory usage, execve is slow because it deallocates the large page table. Numbers and script: before after mem=1.0GB, ensurestart=True 52.1ms 26.0ms mem=1.0GB, ensurestart=False 14.7ms 26.0ms mem=0.5GB, ensurestart=True 23.2ms 11.2ms mem=0.5GB, ensurestart=False 6.2ms 11.3ms mem=0.2GB, ensurestart=True 15.7ms 7.4ms mem=0.2GB, ensurestart=False 4.3ms 8.1ms mem=0.0GB, ensurestart=True 2.3ms 0.7ms mem=0.0GB, ensurestart=False 0.8ms 0.8ms import time for memsize in [1_000_000_000, 500_000_000, 250_000_000, 0]: mem = 'a' * memsize for ensurestart in [True, False]: now = time.time() n = 100 for i in range(n): procutil.runbgcommand([b'true'], {}, ensurestart=ensurestart) after = time.time() ms = (after - now) / float(n) * 1000 print(f'mem={memsize / 1e9:.1f}GB, ensurestart={ensurestart} -> {ms:.1f}ms') Differential Revision: https://phab.mercurial-scm.org/D9019
Thu, 08 Apr 2021 18:43:08 -0400 share: store relative share paths with '/' separators
Matt Harbison <matt_harbison@yahoo.com> [Thu, 08 Apr 2021 18:43:08 -0400] rev 46888
share: store relative share paths with '/' separators I created a relative share in Windows and tried to use it in WSL, and it failed: abort: .hg/sharedpath points to nonexistent directory /mnt/c/Users/Matt/hg-review/.hg/..\..\hg\.hg Use `normpath` on the read side so that the code has the usual Windows style paths it always had (I don't think that matters much), but it also eliminates the directory escaping path components in the case where the path is printed. This will not fix repositories that have already been created, but it's trivial enough to hand edit the file to correct it. Differential Revision: https://phab.mercurial-scm.org/D10330
Fri, 09 Apr 2021 12:02:51 +0200 unit-tests: Fix `cargo test` on 32-bit platforms
Simon Sapin <simon.sapin@octobus.net> [Fri, 09 Apr 2021 12:02:51 +0200] rev 46887
unit-tests: Fix `cargo test` on 32-bit platforms Fixes https://bz.mercurial-scm.org/show_bug.cgi?id=6506 This makes `IndexEntryBuilder::build`, which is only used in unit tests, use `u32` or `u64` instead of platform-dependent `usize` when packing binary data to be used at test input. To run Rust unit tests in 32-bit mode in a x86-64 environment, use: rustup target add i686-unknown-linux-gnu # Once (cd rust && cargo test --target i686-unknown-linux-gnu) Differential Revision: https://phab.mercurial-scm.org/D10351
Fri, 09 Apr 2021 08:46:40 -0700 rename: add --forget option and stop suggesting `hg revert` for undoing
Martin von Zweigbergk <martinvonz@google.com> [Fri, 09 Apr 2021 08:46:40 -0700] rev 46886
rename: add --forget option and stop suggesting `hg revert` for undoing Differential Revision: https://phab.mercurial-scm.org/D10355
Fri, 09 Apr 2021 11:32:19 -0400 win32: enable legacy I/O mode to fix missing pager output on Windows with py3
Matt Harbison <matt_harbison@yahoo.com> [Fri, 09 Apr 2021 11:32:19 -0400] rev 46885
win32: enable legacy I/O mode to fix missing pager output on Windows with py3 The equivalent interpreter option is set by wrapper.exe, but this *.bat file is what gets installed in a venv. Without this mode, any command that spins up a pager has no output, unless the pager is explicitly disabled. The variable is set inside the `setlocal` scope to keep it from leaking into the environment after the bat file exits. We should probably still figure out how to ship a compiled hg.exe when installing with `pip`, because the binary does other things like enable long filename support. But this avoids the dangerous and confusing lack of output in the meantime. Differential Revision: https://phab.mercurial-scm.org/D10354
Thu, 14 Jan 2021 04:58:20 +0100 persistent-nodemap: enable the feature by default when using Rust
Pierre-Yves David <pierre-yves.david@octobus.net> [Thu, 14 Jan 2021 04:58:20 +0100] rev 46884
persistent-nodemap: enable the feature by default when using Rust As discussed at the 5.6 sprint, we can make it enabled by default, but only for Rust installation. Differential Revision: https://phab.mercurial-scm.org/D9765
Fri, 09 Apr 2021 03:36:24 +0200 persistent-nodemap: disable it for test-wireproto-content-redirects.t
Pierre-Yves David <pierre-yves.david@octobus.net> [Fri, 09 Apr 2021 03:36:24 +0200] rev 46883
persistent-nodemap: disable it for test-wireproto-content-redirects.t This is not relevant for the test and this will avoid a lot of variations. Differential Revision: https://phab.mercurial-scm.org/D10338
Fri, 09 Apr 2021 03:30:18 +0200 persistent-nodemap: disable it for test-wireproto-command-capabilities.t
Pierre-Yves David <pierre-yves.david@octobus.net> [Fri, 09 Apr 2021 03:30:18 +0200] rev 46882
persistent-nodemap: disable it for test-wireproto-command-capabilities.t This is not relevant for the test and this will avoid a lot of variations. Differential Revision: https://phab.mercurial-scm.org/D10337
Fri, 09 Apr 2021 03:36:07 +0200 persistent-nodemap: disable it unconditionally for test-wireproto-caching.t
Pierre-Yves David <pierre-yves.david@octobus.net> [Fri, 09 Apr 2021 03:36:07 +0200] rev 46881
persistent-nodemap: disable it unconditionally for test-wireproto-caching.t This is not relevant for the test and this will avoid a lot of variations. Differential Revision: https://phab.mercurial-scm.org/D10336
Fri, 09 Apr 2021 03:51:25 +0200 persistent-nodemap: disable it unconditionally for test-ssh-proto-unbundle.t
Pierre-Yves David <pierre-yves.david@octobus.net> [Fri, 09 Apr 2021 03:51:25 +0200] rev 46880
persistent-nodemap: disable it unconditionally for test-ssh-proto-unbundle.t This is not relevant for the test and this will avoid a lot of variations. Differential Revision: https://phab.mercurial-scm.org/D10335
(0) -30000 -10000 -3000 -1000 -300 -100 -10 +10 +100 +300 +1000 +3000 tip