apex-dev-tools / apex-dev-tools/apex-ls

Outline parser scaling is capped by contention inside toFullDeclaration

オープン
#548 コメント 1 件 リアクション 0 件 担当者 0 名 GitHub で見る
enhancement
主要言語
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 はまだ評価されていません。

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。