v3: cut self-host serial stage work and rebalance worker pools - #28899
Conversation
Self-host (`cd vlib/v && ./v3 -nocache -building-v -o v4 -v v.v`), ThinLTO
-prod builds of the previous and this tree, 10 interleaved runs, median (min)
ms. The host (6 Super + 12 Performance cores) was shared with VMs and other
builds at load 25-45, as in the original report; idle, both trees run about
twice as fast (previous: check ~122, transform ~150, cgen ~147).
stage before after
check 258.6 (220.2) 223.2 (176.7)
transform 334.4 (315.7) 307.2 (264.9)
cgen 343.2 (296.5) 305.8 (251.2)
Generated C is byte-identical to the previous compiler on the same input.
Serial work removed or moved off the main thread:
- set_diagnostic_files resolved the working directory and the realpath of
every source file (~280 getcwd + realpath syscalls); resolve the cwd once
and each directory's realpath once, with one lstat per file.
- Selective type imports computed the module display name (a scan of every
file's module) eagerly; compute it only for a diagnostic.
- Duplicate-function detection allocated and sorted a group per function;
only repeated names get a group now.
- Check/transform work splitting sorts compact (rank, index) pairs and fills
the buckets in source order instead of sorting every bucket.
- cgen function selection called realpath twice per file even without cached
program files; skip it then and memoize per file.
- c_escape_into copies runs of plain bytes in one write instead of copying the
literal and writing byte by byte.
- The C output is written with batched writev instead of copying into a
shared file mapping (measured ~2x faster for the 23 MB unit).
- The interface implementer index pulls interfaces from a queue instead of
fixed slices (IError dominates one slice).
- Transform's `$for ... params` reflection prescan and cgen's alias
registration for every function signature now run on the worker pool.
- Transform merge looks up the canonical spelling of every worker-recorded
call name in parallel (read-only symbol table); only still-missing names
are interned serially, in the original order.
- The lazy declaration indexes rebuilt for each frozen type cache scanned
the ~1M transform-appended nodes; the scan now runs once on the pool and the
source-error-embed index walks the top-level index plus that tail instead
of every node.
Parallel phases:
- substitute_ident/substitute_ident_expr record the parent edge of each fresh
copy, so the checker's parent query for `m[it] or {...}` inside filter
lambdas no longer scans the whole node arena (~20 ms per occurrence).
- Pool.run lets the caller run queued tasks while it waits, so a descheduled
worker no longer holds up a phase. Each batch now has its own completion
channel, so a batch never counts another batch's completions.
New [ttime] lines cover previously untimed serial check and cgen phases.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b01e04a3db
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Stay well below IOV_MAX (1024 on macOS and Linux). | ||
| const c_output_iov_batch = 512 |
There was a problem hiding this comment.
On Unix hosts where IOV_MAX is below 512—Solaris commonly limits writev to 16 vectors—a sufficiently parallel C generation produces a batch whose iovcnt exceeds the host limit, so writev returns EINVAL and the compiler aborts while writing generated C. os_specific_suffixes selects _nix.c.v for every non-Windows host, including Solaris, so the batch size must come from sysconf(_SC_IOV_MAX)/IOV_MAX, use a portable conservative limit, or retry with smaller batches.
Useful? React with 👍 / 👎.
| done.close() | ||
| p.async_task_count += u64(submitted) | ||
| p.task_count += u64(tasks.len) |
There was a problem hiding this comment.
When two callers invoke run concurrently, as the new concurrent-batch test now explicitly does, both execute unsynchronized read-modify-write updates to these counters and to queue_wait_ns/worker_run_ns through record_completion. The tasks can all finish correctly while one caller's increments are lost, making tasks_run() and stats() no longer cumulative; protect the shared accounting with a lock or atomics, or merge per-batch totals under synchronization.
Useful? React with 👍 / 👎.
- The C output writer started every writev call with 512 vectors. Hosts whose IOV_MAX is lower (16 on some systems) reject that with EINVAL, which aborted the build. A rejected call writes nothing, so halve the batch and retry until the host accepts it, keeping the size for the rest of the file. This needs no host-specific IOV_MAX constant in the _nix file. - Pool.run updated the shared task and timing counters directly, so two batches running at once could lose increments. Each batch now accumulates its own counters and merges them once under a mutex; tasks_run() and stats() read under the same mutex. - A task that the waiting caller runs itself now reports its queue wait and run time like a worker-run task, instead of an empty completion. The empty completion made worker_run_ns zero when the caller ran every queued task. Tests cover output written in shrinking batches and the counters of two concurrent batches.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 28a92a53f1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| p.jobs <- task | ||
| continue | ||
| } | ||
| run_queued_task(task) |
There was a problem hiding this comment.
When caller stealing is enabled and queued work remains after the caller's synchronous task, this path executes a task on the caller but run_queued_task reports its duration as worker_run_ns. stats() then divides all such time by capacity from only launched_thread_count, so the worker utilization metric reported by driver.v can exceed 1,000,000 ppm and misstate benchmark results. Track whether the completion ran on a persistent worker, or include caller capacity in the denominator.
Useful? React with 👍 / 👎.
A queued task that the waiting Pool.run caller ran itself was recorded as worker_run_ns, while utilization_ppm divides by the capacity of the persistent workers only, so the reported worker utilization could exceed 1,000,000 ppm. Completions now record whether a persistent worker ran them; caller-run time goes to a separate Stats.caller_run_ns and is excluded from utilization_ppm.
1 participant
Speeds up the check, transform and cgen stages of the V3 self-host by removing serial main-thread work and improving worker-pool load balancing.
The generated C is byte-identical to the previous compiler's on the same input (checked on a
git archivesnapshot, ThinLTO and monolithic-prodbuilds).Timings
Self-host:
cd vlib/v && ./v3 -nocache -building-v -o v4 -v v.v. Measured on an M5 Max (6 Super + 12 Performance cores) with ThinLTO-prodbuilds, median of 10 interleaved runs, in ms.¹ Measured before the last cgen changes (
writevoutput and the tail index scan, about 5 ms more); the machine was not idle again to re-measure.Most of the recently reported slowdown (check 173 / transform 310 / cgen 227) comes from host load. On the Sep 17 tree the compiler retired only ~10% fewer instructions per stage, and the compiled source grew ~4% since then.
Serial work removed or moved to workers
set_diagnostic_filescalledgetcwdandrealpathfor each of ~280 files. It now resolves the cwd once, each directory once, and does onelstatper file (13 ms → 1 ms).substitute_identcopies record their parent edge. Before, the checker's parent query forarr.filter((m[it] or {...})...)scanned all ~2.4M nodes, costing ~20 ms per occurrence.realpathper file when no program files are cached.c_escape_intocopies runs of plain bytes in one write.writevinstead of copying into a shared file mapping, measured ~2x faster for the 23 MB unit.$for ... paramsprescanWorker pool
Pool.runlets the caller run queued tasks while it waits, so a descheduled worker no longer holds up a phase.Behaviour and tooling changes
No output changes. The debug env var
V3_NO_MMAP_CGEN_OUTPUTis renamed toV3_NO_WRITEV_CGEN_OUTPUT.-vprints a few new[ttime]lines for previously untimed serial check and cgen phases.Tests
./v -silent vlib/v/compiler_errors_test.v: 1720 passed, 5 skipped../v -silent testovervlib/v/{workers,types,transform,gen/c,driver,markused}: 92/92 passed.vlib/v/types/checker_ownership_alias_test.vwas still compiling after ~55 min locally under heavy load; it takes 30-60 min in other checkouts too. It is left to CI.