Repository navigation
[Tech Debt]: Assembler.Synchronize is O(n²) — 89 s to assemble a 50 000-element model #347
Description
Description
Assembler.Synchronize resolves each new DTO with dtos.Single(x => x.Id == identifier) inside a
per-element loop (SysML2.NET.Dal/Assembler.cs:110). Single cannot short-circuit — it scans the
whole sequence to prove uniqueness — so on a cold cache this is n scans of n elements: ~2.5 billion
comparisons at 50 000 elements.
The identifier is already on the DTO being sought, so the lookup is entirely avoidable.
Measured impact
CoreCLR, .NET 10.0.11 Release, synthetic SysML v2 containment tree via Serializer.MessagePack:
| Elements | Deserialize | Synchronize |
|---|---|---|
| 4 999 | 2 ms | 202 ms |
| 9 999 | 196 ms | 0.96 s |
| 19 999 | 100 ms | 4.9 s |
| 49 999 | 199 ms | 89 s |
Assembly is ~450× the cost of deserialization at 50 000 elements, growing at roughly n^2.4–n^3.2.
This is runtime-independent — the same code under Blazor WebAssembly takes 3.11 s for just 4 999
elements.
Proposed fix
Replace the Except + Single combination with a single pass over the DTOs:
foreach (var dto in dtoList)
{
if (this.Cache.ContainsKey(dto.Id)) continue;
var poco = elementFactory.Create(dto);
this.Cache.AddOrUpdate(poco.Id, new Lazy<Core.POCO.Root.Elements.IElement>(() => poco), (key, oldValue) => oldValue);
addedCount++;
}existingIdentifiers, dtoIdentifiers and newIdentifiers all become dead. Should be O(n) and
sub-second at 50 000.
Behaviour change to confirm: duplicate Ids currently throw InvalidOperationException from
Single; under this loop the first occurrence wins. Preferable to a hard failure mid-assembly, but
worth an explicit decision.
Related smaller problems in the same method
- Line 115 —
newIdentifiers.Count()re-runs the lazyExcept. Arguments are evaluated even
whenDebuglogging is off, so this full extra pass is paid in production. Use a counter. - Lines 92–99 — when
TryRemovereturnsfalseit assignsnulltolazyPoco, and theelse
branch then dereferenceslazyPoco.Value.GetType().Name→NullReferenceException. Reachable via
a duplicate indeletedIdentifiers. - Line 86 —
"A total of {0} identifiers..."is passed theList<Guid>instead of its.Count. dtosis enumerated 4+ times (lines 77, 106, 110×n, 119). Materialise once; a lazy caller
sequence is currently re-evaluated on every pass.
Tasks
- Materialise
dtosonce; replace theExcept/Singlelookup with the single-pass loop. - Use a counter instead of
newIdentifiers.Count(). - Decide and document duplicate-
Idbehaviour. - Fix the
NullReferenceExceptionand the{0}argument in the deletion loop. - Add a regression test asserting linear scaling, so a reintroduced quadratic scan fails CI.
-
SysML2.NET.Dal.Testsgreen, in particularAssemblerTestFixture.
Out of scope
Parallelising assembly; changes to ElementFactory, UpdateReferenceProperties or the Cache type;
the ~522 MB managed-memory footprint at 50 000 elements (separate investigation).
Activity
fix #347