apex-dev-tools / apex-dev-tools/apex-ls
Outline parser scaling is capped by contention inside toFullDeclaration
- 主要言語
- Scala
- スター
- 12
- フォーク
- 3
- 平均マージ
- 13時間 43分
- マージ済み PR(30日)
- 19
説明
## Context
#547 fixed `StreamDeployer` so that `OutlineParserMultithreaded` really does parse on more than one
thread — the loop iterated `classes.par.iterator`, which is a sequential splitter. The parse pool was
deliberately bounded to `min(availableProcessors, 4)` rather than left to the machine, because parse
does not scale past a handful of threads.
This issue is to find out why.
## Evidence
Thread sweep on the public `apex-samples` Cumulus workspace (1,060 types), fresh JVM per run, cache
disabled, `OutlineMulti`, 4 repetitions with the first discarded, `-Dscala.concurrent.context.maxThreads`
setting the level:
| Threads | Total load | Parse CPU (summed across threads) | Parse wall clock |
|---|---|---|---|
| 1 | 7,439ms | 1,029ms | ~1,030ms |
| 2 | 7,531ms | 1,451ms | ~730ms |
| 4 | 7,479ms | 2,761ms | ~690ms |
| 8 | 7,493ms | 6,286ms | ~790ms |
Wall clock stops improving after two threads and gets worse by eight, while the summed cost of parsing
the same 1,034 files inflates six-fold. Per-file parse cost roughly doubles from one thread to four.
Total load time is flat throughout because parse is only ~10% of cold load on this workspace even when
parsed perfectly (see #540).
Something inside `OutlineParserFullDeclaration.toFullDeclaration` does not scale.
## Candidate explanation, to be confirmed by measurement
CST construction goes through several process-wide caches, each a plain unsynchronised
`scala.collection.mutable.HashMap`/`Map` updated via `getOrElseUpdate`:
- `Names.nameCache` — every interned identifier
- `TypeNames` and `ModifierResults` (both `InternCache`)
- `PlatformTypes.typeCache`
Every parsed file hits all of these, hard. Shared mutable maps written from several threads are a
plausible source of the per-file inflation, but this must be confirmed with JFR rather than assumed —
allocation rate and GC are equally plausible and the two are hard to separate by inspection.
## Correctness risk in the same code — worth acting on regardless
`getOrElseUpdate` on `mutable.HashMap` is not safe to call concurrently. Concurrent writers can lose
entries, and a write racing a resize can corrupt the table or spin. The caches above are now written
from several threads on every cold load.
Two mitigating facts, neither of which makes it safe:
- This exposure is not new. `loadClassesFromCache` has used an unbounded `classes.par.foreach` for the
summary-cache path for some time, so these caches were already being written concurrently there.
#547 widened the exposure to the cold parse path.
- A regression run over a set of larger workspaces after #547 produced diagnostics byte-identical to
those from unmodified `main`, so it is not biting in practice today. `Name` equality is by value, so
a lost interning is benign; a corrupted table would not be.
## Scope
1. Profile parse under JFR at 1, 2, 4 and 8 threads and name the actual scaling limit at method level.
2. Make the shared caches safe for concurrent use, or confine them. Measure the result — a concurrent
map may fix the safety question without moving the scaling one.
3. Re-measure the sweep and revisit the bound of four in `StreamDeployer.parseThreads` if the knee moves.
## Out of scope
Validation performance — that is by far the larger share of cold load and is tracked separately.
コントリビューションガイド
このリポジトリのコントリビューションガイドは索引されていません
評価
この issue はまだ評価されていません。