From ccef521aa73e724d25c40e60ec3c08f2b0863e3b Mon Sep 17 00:00:00 2001 From: nsfisis Date: Fri, 17 Jul 2026 22:12:35 +0900 Subject: perf(sync-executor): drive HTTP fetches through one real top-level runtime Replace sync_executor::block_on's reactor-less busy-spin poller with tokio::task::block_in_place + Handle::current().block_on(), riding a single tokio Runtime entered once in main.rs (falling back to a disposable one when no ambient runtime exists, e.g. in tests). This lets HttpDownloader::dispatch await CurlDownloader::download directly instead of bouncing through the separate curl_runtime() bridge, which is now deleted. Manual create-project verification against the real network caught a concurrency bug this exposed: async_fetch_file held http_downloader's RefMut across the await on add(), which only panics once downloads genuinely overlap. add() only needs &self, so borrow() fixes it. With everything now sharing one real reactor, the FuturesOrdered fan-out added for ComposerRepository::get_security_advisories/ load_async_packages finally overlaps for real: fetching 8 packages' metadata dropped from ~7-40s to a consistent ~3-4s in a before/after comparison, with identical resulting lock files. sync_executor::block_on's call sites are still synchronous rather than async fn propagated up to Command::execute, which remains the end goal (see the TODO(phase-e) in sync_executor.rs) - nested block_on calls elsewhere don't get this same overlap, only prevented panics. --- crates/shirabe/src/util/http_downloader.rs | 25 +------------------------ 1 file changed, 1 insertion(+), 24 deletions(-) (limited to 'crates/shirabe/src/util/http_downloader.rs') diff --git a/crates/shirabe/src/util/http_downloader.rs b/crates/shirabe/src/util/http_downloader.rs index b7173c5e..c9e1e4a6 100644 --- a/crates/shirabe/src/util/http_downloader.rs +++ b/crates/shirabe/src/util/http_downloader.rs @@ -91,29 +91,6 @@ impl Default for HttpDownloaderMockHandler { } } -/// A single-threaded tokio Runtime used only to drive `CurlDownloader::download()` (which needs a -/// real reactor now that it uses the non-blocking `reqwest::Client`, unlike `sync_executor::block_on` -/// which assumes every awaited future resolves synchronously). `current_thread` is used because -/// `block_on` (unlike `spawn`) has no `Send` bound, and `download()`'s future closes over -/// `Rc>` handles that are not `Send`. -/// -/// `get()`/`copy()` bridge into the async core via `sync_executor::block_on` instead of this -/// Runtime (nesting this same Runtime's `block_on` inside itself, on the curl path, would panic -/// with "Cannot start a runtime from within a runtime"); this Runtime is only ever entered at the -/// single point where `CurlDownloader::download()` is awaited. -/// -/// TODO(phase-e): remove this once `HttpDownloader::add`/`get` are driven by `Loop::wait`'s -/// `FuturesUnordered` under a single top-level Runtime (see the async re-architecture design). -fn curl_runtime() -> &'static tokio::runtime::Runtime { - static RT: std::sync::LazyLock = std::sync::LazyLock::new(|| { - tokio::runtime::Builder::new_current_thread() - .enable_all() - .build() - .expect("failed to build the temporary CurlDownloader bridge runtime") - }); - &RT -} - impl HttpDownloader { /// @param IOInterface $io The IO instance /// @param Config $config The config @@ -346,7 +323,7 @@ impl HttpDownloader { } let curl = self.curl.as_ref().unwrap(); - return match curl_runtime().block_on(curl.download(&origin, url, options, copy_to)) { + return match curl.download(&origin, url, options, copy_to).await { Ok(Ok(response)) => Ok(response), Ok(Err(transport_exception)) => Err(transport_exception.into()), Err(e) => Err(e), -- cgit v1.3.1