Skip to content

Commit 5821ed0

Browse files
dhellmannclaude
andcommitted
perf(bootstrapper): reduce disk I/O with throttled stack writes, async graph writes, deferred build-order write
Three file-write hotspots in the bootstrap loop were generating O(n²) disk I/O on the main thread: 1. `_record_stack_state` was called on every loop iteration, serializing and writing the full stack to bootstrap-stack.json. Throttled to at most once every 5 seconds (force-flush in `finalize()`). 2. `add_to_graph` → `write_to_graph_to_file()` wrote the full graph on every dependency edge. Graph writes are now moved to a single-threaded background `_write_pool` via `_write_graph_async()`, keeping serialization on the main thread for a consistent snapshot but eliminating blocking I/O from the critical path. 3. `add_to_build_order` rewrote build-order.json on every package. The disk write is removed entirely; data stays in `_build_stack` in memory and is written once in `finalize()`. Also renames `write_to_graph_to_file()` → `write_graph_file()` in `WorkContext` for clarity. Tests updated to reflect deferred build-order write (call `finalize()` before reading the file) and the stack-write throttle. Two new tests added: one verifying the throttle suppresses rapid writes, one verifying `finalize()` drains the write pool and produces both output files. Co-Authored-By: Claude <claude@anthropic.com> Signed-off-by: Doug Hellmann <dhellmann@redhat.com>
1 parent fdf55b3 commit 5821ed0

4 files changed

Lines changed: 98 additions & 12 deletions

File tree

src/fromager/bootstrapper/_bootstrapper.py

Lines changed: 48 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,14 @@
33
import concurrent.futures
44
import contextlib
55
import datetime
6+
import io
67
import json
78
import logging
89
import operator
910
import pathlib
1011
import shutil
1112
import tempfile
13+
import time
1214
import typing
1315

1416
from packaging.requirements import Requirement
@@ -104,6 +106,16 @@ def __init__(
104106
self._stack_filename = self.ctx.work_dir / "bootstrap-stack.json"
105107
logger.info("recording bootstrap stack state to %s", self._stack_filename)
106108

109+
# Single-threaded pool for background file writes (serialized, never interleave)
110+
self._write_pool: concurrent.futures.ThreadPoolExecutor | None = (
111+
concurrent.futures.ThreadPoolExecutor(
112+
max_workers=1, thread_name_prefix="fromager-writes"
113+
)
114+
)
115+
# Throttle stack-state writes to at most once per interval
116+
self._last_stack_write: float = 0.0
117+
self._STACK_WRITE_INTERVAL: float = 5.0
118+
107119
# Track failed packages in test mode (list of typed dicts for JSON export)
108120
self.failed_packages: list[FailureRecord] = []
109121

@@ -850,7 +862,7 @@ def add_to_graph(
850862
pre_built=pbi.pre_built,
851863
constraint=self.ctx.constraints.get_constraint(req.name),
852864
)
853-
self.ctx.write_to_graph_to_file()
865+
self._write_graph_async()
854866

855867
def _sort_requirements(
856868
self,
@@ -954,21 +966,31 @@ def add_to_build_order(
954966
if req.url:
955967
info["source_url"] = req.url
956968
self._build_stack.append(info)
957-
with open(self._build_order_filename, "w") as f:
958-
# Set default=str because the why value includes
959-
# Requirement and Version instances that can't be
960-
# converted to JSON without help.
961-
json.dump(self._build_stack, f, indent=2, default=str)
969+
970+
def _schedule_write(self, path: pathlib.Path, content: str) -> None:
971+
"""Submit a pre-serialized write to the background write pool."""
972+
if self._write_pool is not None:
973+
self._write_pool.submit(path.write_text, content, "utf-8")
974+
975+
def _write_graph_async(self) -> None:
976+
"""Serialize the dependency graph on the main thread, write it in background."""
977+
buf = io.StringIO()
978+
self.ctx.dependency_graph.serialize(buf)
979+
self._schedule_write(self.ctx.graph_file, buf.getvalue())
962980

963981
def _record_stack_state(self, stack: list[Phase]) -> None:
964982
"""Write the current bootstrap stack to `self._stack_filename`.
965983
966984
Index 0 in the output corresponds to `stack[-1]`, the next item to be
967-
processed. Overwrites the file on each call.
985+
processed. Throttled to at most once every ``_STACK_WRITE_INTERVAL`` seconds.
968986
"""
987+
now = time.monotonic()
988+
if now - self._last_stack_write < self._STACK_WRITE_INTERVAL:
989+
return
969990
records = [item.as_json() for item in reversed(stack)]
970991
with open(self._stack_filename, "w") as f:
971992
json.dump(records, f, indent=2, default=str)
993+
self._last_stack_write = now
972994

973995
# ---- Iterative bootstrap: phase handlers and helpers ----
974996

@@ -1198,7 +1220,7 @@ def _handle_phase_error(
11981220
self._seen_requirements.discard(
11991221
self._resolved_key(wi.req, wi.resolved_version, "wheel")
12001222
)
1201-
self.ctx.write_to_graph_to_file()
1223+
self._write_graph_async()
12021224
return []
12031225

12041226
# Normal mode: fail-fast
@@ -1246,6 +1268,21 @@ def finalize(self) -> int:
12461268
self._bg_pool.shutdown(wait=True, cancel_futures=True)
12471269
self._bg_pool = None
12481270

1271+
# Write build-order once at the end (data was buffered in _build_stack)
1272+
with open(self._build_order_filename, "w") as f:
1273+
# Set default=str because values may include Requirement and Version
1274+
# instances that can't be converted to JSON without help.
1275+
json.dump(self._build_stack, f, indent=2, default=str)
1276+
1277+
# Final graph write and stack flush, then drain the write pool
1278+
self._write_graph_async()
1279+
records: list[typing.Any] = []
1280+
with open(self._stack_filename, "w") as f:
1281+
json.dump(records, f, indent=2)
1282+
if self._write_pool is not None:
1283+
self._write_pool.shutdown(wait=True, cancel_futures=False)
1284+
self._write_pool = None
1285+
12491286
if self.multiple_versions and self._failed_versions:
12501287
self._log_failed_versions_table()
12511288

@@ -1284,3 +1321,6 @@ def __exit__(
12841321
if self._bg_pool is not None:
12851322
self._bg_pool.shutdown(wait=False, cancel_futures=True)
12861323
self._bg_pool = None
1324+
if self._write_pool is not None:
1325+
self._write_pool.shutdown(wait=False, cancel_futures=True)
1326+
self._write_pool = None

src/fromager/context.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,8 @@ def uv_clean_cache(self, *reqs: Requirement) -> None:
156156
cmd.extend(req_list)
157157
external_commands.run(cmd, extra_environ=extra_environ)
158158

159-
def write_to_graph_to_file(self) -> None:
159+
def write_graph_file(self) -> None:
160+
"""Write the dependency graph to ``self.graph_file``."""
160161
with self.graph_file.open("w", encoding="utf-8") as f:
161162
self.dependency_graph.serialize(f)
162163

tests/test_bootstrapper.py

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,7 @@ def test_build_order(tmp_context: WorkContext) -> None:
129129
source_url="url",
130130
source_type=SourceType.SDIST,
131131
)
132+
bt.finalize()
132133
contents_str = bt._build_order_filename.read_text()
133134
contents = json.loads(contents_str)
134135
expected = [
@@ -174,6 +175,7 @@ def test_build_order_repeats(tmp_context: WorkContext) -> None:
174175
"url",
175176
SourceType.SDIST,
176177
)
178+
bt.finalize()
177179
contents_str = bt._build_order_filename.read_text()
178180
contents = json.loads(contents_str)
179181
expected = [
@@ -204,6 +206,7 @@ def test_build_order_name_canonicalization(tmp_context: WorkContext) -> None:
204206
"url",
205207
SourceType.SDIST,
206208
)
209+
bt.finalize()
207210
contents_str = bt._build_order_filename.read_text()
208211
contents = json.loads(contents_str)
209212
expected = [
@@ -764,12 +767,14 @@ def test_record_stack_state_ordering(tmp_context: WorkContext) -> None:
764767

765768

766769
def test_record_stack_state_overwrites_each_call(tmp_context: WorkContext) -> None:
767-
"""Second call replaces first call's content."""
770+
"""Second call replaces first call's content when throttle interval has elapsed."""
768771
bt = bootstrapper.Bootstrapper(tmp_context)
769772

770773
bt._record_stack_state([_make_resolve_item("pkga"), _make_resolve_item("pkgb")])
771774
first_content = bt._stack_filename.read_text()
772775

776+
# Reset throttle so the second call is not suppressed
777+
bt._last_stack_write = 0.0
773778
bt._record_stack_state([_make_resolve_item("pkgc")])
774779
second_content = bt._stack_filename.read_text()
775780

@@ -779,6 +784,46 @@ def test_record_stack_state_overwrites_each_call(tmp_context: WorkContext) -> No
779784
assert contents[0]["req"] == "pkgc"
780785

781786

787+
def test_record_stack_state_throttled_when_called_rapidly(
788+
tmp_context: WorkContext,
789+
) -> None:
790+
"""Rapid successive calls do not overwrite the file (throttle active)."""
791+
bt = bootstrapper.Bootstrapper(tmp_context)
792+
stack: list[Phase] = [_make_resolve_item("pkga"), _make_resolve_item("pkgb")]
793+
794+
# First call writes (interval has elapsed from epoch)
795+
bt._record_stack_state(stack)
796+
first_mtime = bt._stack_filename.stat().st_mtime
797+
798+
# Second call is throttled — file should not change
799+
bt._record_stack_state([_make_resolve_item("pkgc")])
800+
second_mtime = bt._stack_filename.stat().st_mtime
801+
802+
assert first_mtime == second_mtime
803+
804+
805+
def test_finalize_writes_build_order_and_graph(tmp_context: WorkContext) -> None:
806+
"""finalize() writes build-order.json and graph.json, and drains the write pool."""
807+
bt = bootstrapper.Bootstrapper(tmp_context)
808+
bt.add_to_build_order(
809+
req=Requirement("mypkg==1.0"),
810+
version=Version("1.0"),
811+
source_url="https://pypi.test/mypkg-1.0.tar.gz",
812+
source_type=SourceType.SDIST,
813+
)
814+
815+
assert not bt._build_order_filename.exists()
816+
817+
bt.finalize()
818+
819+
assert bt._build_order_filename.exists()
820+
assert bt._write_pool is None # pool was drained and closed
821+
822+
contents = json.loads(bt._build_order_filename.read_text())
823+
assert len(contents) == 1
824+
assert contents[0]["dist"] == "mypkg"
825+
826+
782827
def test_bootstrap_calls_record_stack_state(tmp_context: WorkContext) -> None:
783828
"""`_record_stack_state` is called at least once during `bootstrap()`."""
784829
bt = bootstrapper.Bootstrapper(tmp_context)

tests/test_context.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -155,12 +155,12 @@ def test_wheels_build_parallel(tmp_context: context.WorkContext) -> None:
155155
assert result.is_dir()
156156

157157

158-
def test_write_to_graph_to_file(tmp_path: pathlib.Path) -> None:
158+
def test_write_graph_file(tmp_path: pathlib.Path) -> None:
159159
ctx = _make_context(tmp_path)
160160
ctx.setup()
161161

162162
with patch.object(ctx.dependency_graph, "serialize") as mock_serialize:
163-
ctx.write_to_graph_to_file()
163+
ctx.write_graph_file()
164164

165165
mock_serialize.assert_called_once()
166166
assert ctx.graph_file.exists()

0 commit comments

Comments
 (0)