Drop dependency on multiprocess - #140
Merged
Merged
Conversation
bbannier
force-pushed
the
topic/bbannier/asyncio
branch
from
July 23, 2026 21:33
ee2e167 to
909392a
Compare
awelzel
reviewed
Jul 24, 2026
Contributor
There was a problem hiding this comment.
This is clever. I like this.
I only looked at the final code though. Some minor nits and ideas while skimming the complete diff, but generally 👍
Drops the only dependency btest has, removes 168 lines of code, and running its own test-suite with -j4 gets a 1.6x 1.7x speed-up in elapsed time for me locally (15.42/8.97 = 1.72, not sure how I got to 1.6x before...).
$ taskset -c 0,1,2,3 /usr/bin/time btest -j 4 -d testing/
91 tests successful, 3 skipped
22.99user 12.30system 0:15.42elapsed 228%CPU (0avgtext+0avgdata 62100maxresident)k
$ taskset -c 0,1,2,3 /usr/bin/time btest -j 4 -d testing/
91 tests successful, 3 skipped
26.54user 7.01system 0:08.97elapsed 373%CPU (0avgtext+0avgdata 61900maxresident)k
awelzel
reviewed
Jul 24, 2026
bbannier
force-pushed
the
topic/bbannier/asyncio
branch
2 times, most recently
from
July 31, 2026 10:56
b4dec11 to
497a883
Compare
…ss_mp` shim Isolates the multiprocessing implementation behind a single callable so later commits can swap it without touching callers.
The process-per-command pattern existed solely to contain `KeyboardInterrupt`. Replace with a plain `Popen`+wait loop that kills the child on interrupt and re-raises, removing the `mp.Queue` and `mp.Process` machinery from the hot path.
Moves all mutable state (counters, test queue, port pool, output handler, lock) and the methods that operate on it out of `TestManager` into a plain `RunState` class. `TestManager` delegates to it; the `SyncManager` proxy machinery is otherwise unchanged.
Pulls the `test.run()`+`testReplayOutput()` body out of `threadRun` into a top-level `run_test(test, state)` function, making the worker loop a thin scheduler and preparing the seam for an async replacement.
…_progress_monitor` Encapsulates the `threading.Thread`+`Event` lifecycle into two methods on `Test`, hiding the threading details and preparing the seam for replacing the thread with an asyncio task.
Adds `runSubprocess` and `runTestCommandLine` (with an async `LinuxTimer.timeSubprocess` for the timing path; a matching stub is added to `TimerBase`). `execute()` now calls `asyncio.run(runTestCommandLine(...))`, a sync shim that a later commit replaces with a plain `await` once the call stack goes async. The new helpers take explicit `cwd`, `env`, `stdout`, `stderr` parameters rather than `**kwargs`, which also drops the `shell=True` flag from the `execute()` call site; `asyncio.create_subprocess_shell` preserves the shell-expansion behaviour on non-Windows.
bbannier
force-pushed
the
topic/bbannier/asyncio
branch
from
August 19, 2026 14:13
497a883 to
2bbcc36
Compare
Member
|
@bbannier Are you still planning on bringing this in? I'm a big fan. The multiprocess stuff really sucks to work with. |
Under asyncio, spawning a synchronous subprocess triggers "Unknown child process" warnings from the child watcher. `shutil.rmtree` does the same work without a child process.
Makes `execute()`, `run()`, `run_cmdseq()`, and `run_test()` all async. The `asyncio.run()` shim introduced in the previous commit becomes a plain `await`; the progress monitor thread becomes an asyncio `Task`; and `run_tests()` drives the worker pool via `asyncio.run(_async_run_all)`. The `rc==200` (user-initiated abort) path no longer calls `sys.exit()` from inside a coroutine; it raises `Abort` instead, which `run_tests()` catches via a `user_abort` flag so the test summary can print before exiting. The process exits with code 1 when a user-abort occurs. The `--abort-on-failure` path still re-raises `Abort` to the top-level handler. The threads test is simplified: the old baseline verified that `@TEST-SERIALIZE` co-located tests 4 and 5 on the same thread; under asyncio that thread-identity check no longer applies, so the test now only verifies that all five tests complete successfully with `-j 5`.
Workers are now coroutines in a single process, so cross-process atomic integers are no longer needed. Drop the `mp_sharedctypes` imports.
Workers are coroutines in the same process, so globals are already shared. The proxy dict and the code that pushed it into child-process namespaces is dead.
`TestManager` is now a plain class. Replace proxy list/lock objects with plain list and `threading.Lock`, remove the `SyncManager` socket address setup, remove `mp_managers` imports, and drop the `mgr.shutdown()` call.
`UPDATE_INTERACTIVE` now uses the same `asyncio.run` path as normal mode. Remove `threadRun` and the `mp.set_start_method` calls that were only needed for multiprocessing worker spawning. This also removes the Python >=3.8 version-detection block that forced the `"fork"` start method on macOS (the workaround for the regression noted in issue #26); it is no longer relevant once multiprocessing is gone. `import signal` is also removed as it was only used inside `threadRun`.
…orts `mp.cpu_count()` -> `os.cpu_count()`, `mp.current_process().pid` -> `os.getpid()`. Remove the multiprocess/multiprocessing import block and the `deepcopy` of tests in `main()`, no longer needed in a single-process model.
All test execution is now single-threaded (asyncio event loop), so mutual exclusion is no longer needed. Drop `threading.Lock` from `RunState`, the `progress_lock` from `Test`, and the sync progress monitor thread along with `import threading`. Also remove the `asyncio.Semaphore(n)` from `run_all_tests`: each worker already processes its assigned tests sequentially, so the semaphore was redundant and inadvertently serialized all workers.
`TestManager` was a pure delegation wrapper over `RunState` with no independent state. Replace it with a top-level `run_tests()` function, pass `RunState` directly as `mgr` to tests, and drop the `prepare(mgr)` parameter since no handler uses it.
These comments referenced child worker processes and Windows multiprocess spawn constraints that no longer apply after the asyncio migration.
The asyncio migration removed TestMgr and all locking; the docstring claiming handlers are called under a lock is no longer accurate.
The last caller of `runSubprocess`, `LinuxTimer.available()`, can use `subprocess.call()` directly now that the sync wrapper is gone. Remove `runSubprocess` and the `shlex` import it needed.
All affected functions are already `async def`; the prefix adds no information now that the sync counterparts are gone.
The concurrency model is now asyncio workers, not OS threads. Update variable names, method names, and docstrings to match. User-visible names (--threads flag, Options.threads) are unchanged.
`TestManager` is gone; the parameter and attribute now hold a `RunState`.
bbannier
force-pushed
the
topic/bbannier/asyncio
branch
from
September 10, 2026 06:11
2bbcc36 to
0a5f0fc
Compare
Member
Author
Yes, absolutely. I have been using exactly this state (well #141) locally without any issues, only just pushed an update which avoids a spurious error message; other than that this seems to work well. The only issue is that I haven't had found time to do a careful review of the code, so I have not been comfortable calling this "Ready for review". Let me try to do that today or tomorrow. |
bbannier
marked this pull request as ready for review
September 11, 2026 08:03
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Warning
All code changes in this branch were generated with Claude Sonnet 4.6. I reviewed all changes and am accountable.