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

Outline parser scaling is capped by contention inside toFullDeclaration

Abierto
#548 1 comentario 0 reacciones 0 asignados Ver en GitHub
enhancement
Lenguaje dominante
Scala
Estrellas
12
Forks
3
Merge medio
13 h 43 min
PR fusionados (30 d)
19

Descripción

## 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.

Guía de contribución

No hay ninguna guía de contribución indexada para este repositorio

Evaluación

Este issue todavía no se ha evaluado.

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.