Commit Graph

9 Commits

Author SHA1 Message Date
Jens Johansen 9481e3615c [analyzer] BinaryWriter has a smaller buffer
Previously whenever a BinaryWriter was created it immediately created a
128 kb Uint8List. When compiling analysis server et al (all sources used
to compile the analysis server) this results in allocating ~2.7GB (with
`takeBytes` returning a total of <54MB in this case).

This CL reduces this buffer to 5KB instead, reducing the overhead on GC.
With a 5KB buffer (about 94% `takeBytes` calls return less than 5KB)
we instead allocate about 130MB in this case.

It being mostly a GC pressure change (and that it will change up when
GC happens) benchmarking it isn't great, nevertheless this is what I
got:

Analyzing CFE et al (without dart2wasm stuff):

```
msec task-clock:u: -1.9193% +/- 0.5797% (-166.76 +/- 50.37) (8688.58 -> 8521.82)
page-faults:u: -7.8067% +/- 0.0517% (-12297.80 +/- 81.48) (157529.64 -> 145231.84)
cycles:u: -1.6454% +/- 0.6021% (-600019148.06 +/- 219543431.36) (36465937621.72 -> 35865918473.66)
instructions:u: -1.1447% +/- 0.4261% (-498854632.50 +/- 185671028.34) (43579529159.92 -> 43080674527.42)
branch-misses:u: -2.8986% +/- 1.9360% (-4171049.68 +/- 2785886.58) (143898017.08 -> 139726967.40)
seconds time elapsed: -1.9201% +/- 0.5796% (-0.17 +/- 0.05) (8.69 -> 8.53)
seconds user: -1.7259% +/- 0.6256% (-0.14 +/- 0.05) (8.37 -> 8.23)
seconds sys: -7.0216% +/- 4.0546% (-0.02 +/- 0.01) (0.32 -> 0.29)

Comparing GC data:
Scavenge(   new space) goes from 176 to 138
MarkSweep(   old space) goes from 1 to 2
Evacuate(store buffer) goes from 1 to 0
Notice combined GC time goes from 2811 ms to 2959 ms (notice only 1 run each).
```

Analysis server et al:

```
msec task-clock:u: -2.6522% +/- 0.5847% (-467.52 +/- 103.07) (17627.23 -> 17159.71)
page-faults:u: -3.5245% +/- 0.0502% (-10456.22 +/- 148.92) (296669.38 -> 286213.16)
cycles:u: -2.5953% +/- 0.5919% (-1921850003.72 +/- 438308899.83) (74050606844.82 -> 72128756841.10)
instructions:u: -1.8325% +/- 0.0006% (-1578705053.84 +/- 483572.09) (86149571116.38 -> 84570866062.54)
branch-misses:u: -4.0379% +/- 2.0070% (-12275829.02 +/- 6101639.40) (304013449.34 -> 291737620.32)
seconds time elapsed: -2.6510% +/- 0.5845% (-0.47 +/- 0.10) (17.64 -> 17.17)
seconds user: -2.5518% +/- 0.6162% (-0.43 +/- 0.10) (17.00 -> 16.56)
seconds sys: -5.3499% +/- 2.5859% (-0.03 +/- 0.02) (0.63 -> 0.60)

Comparing GC data:
Scavenge(   new space) goes from 363 to 273
MarkSweep(   promotion) goes from 21 to 20
Evacuate(store buffer) goes from 1 to 0
Notice combined GC time goes from 5415 ms to 5542 ms (notice only 1 run each).
```

Change-Id: I78b1a9a1c882ad486eff97b3aee1ed5f29235891
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/458560
Commit-Queue: Jens Johansen <jensj@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
2025-10-31 04:44:40 -07:00
Konstantin Shcheglov a66ef95427 Fine. Deduplicate manifest IDs in binary format
Introduce a manifest-ID table to deduplicate `ManifestItemId`s during
serialization and shrink stored artifacts. IDs are now written once per
blob and referenced by varint indices, reducing bundle size with a small
write-time overhead.

Key changes:
- Add `ManifestIdTableBuilder` and encode IDs via table indices.
- Add `BinaryWriter.writeManifestItemId` / `BinaryReader.readManifestItemId`;
  redirect `ManifestItemId.write/read` to these helpers.
- Replace `StringIndexer` with `StringTableBuilder` (same behavior, new name).
- Introduce a unified trailer:
  - Layout: `<payload><manifest_id_table><string_table><u32 idOff><u32 strOff>`
  - New APIs: `BinaryWriter.writeTableTrailer()` and
    `BinaryReader.initFromTableTrailer()`.
  - Update all call sites that write/read analyzer bundles
    (e.g. `LinkedBundleProvider`, `LibraryDiagnosticsBundle`,
    manifest/requirements serializers) to the new trailer.
- Keep summary2 bundle format unchanged; it still uses its own four-u32
  footer. Calls there switch to `initStringTableAt(stringsOffset)` only.
- Simplify `ManifestItemId.hashCode` for faster lookups.
- Propagate tables across `BinaryReader.fork()` to avoid reinitialization.
- Bump data format: `AnalysisDriver.DATA_VERSION` 561 → 562.

Impact (bundleProvider.put):
- Size: 42,694,174 → 33,591,531 bytes (−9,102,643; −21.32%).
- Write time: 179.650 ms → 201.271 ms (+21.621 ms; +12.04%).

Motivation: Consolidating manifest IDs removes repeated 64-bit values
from payloads, cutting I/O and storage while keeping deserialization
simple and fast.

Change-Id: I886a8d5a745f81a94c7bca126193cb1460d99fae
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/451321
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
2025-09-25 11:22:50 -07:00
Konstantin Shcheglov 17abf40e0f Fine. Write URIs and lookup names as string refs in manifests
Switch serialization of heavily repeated strings (URIs, `LookupName`s,
and select identifiers) from inline UTF-8 blobs to string references
backed by a per-blob string table. Readers now initialize the table from
the end of the buffer, and writers emit it once at the end.

This cuts duplication in library/requirements/diagnostics bundles and
improves both size and speed when analyzing the analyzer:
- size: 112 MB → 62 MB (~45% smaller)
- time: 450 ms → 310 ms (~31% faster)

Key changes
- Use `writeStringReference` / `readStringReference` for:
  - URIs (`writeUri`/`readUri`); parse via `uriCache.parse`.
  - `LookupName`, `BaseName`, and various manifest/type strings
    (e.g., token buffers, member/top-level names, record field names,
    named parameter names).
- Add `writeStringTableAtEnd()` in all writers that produce persisted
  blobs, and `initializeStringTableFromEnd()` in matching readers:
  - `LinkedBundleProvider` (get/put paths)
  - `LibraryDiagnosticsBundle` (toBytes/fromBytes)
  - Manifest and requirements assert-serialization paths
- Bump `AnalysisDriver.DATA_VERSION` to 561 to invalidate old caches.

Why this works
- Manifests and requirements repeat the same URIs and names across many
  entries. Interning them through a string table removes redundancy and
  reduces I/O.
- `uriCache.parse` avoids repeated `Uri` allocations and speeds up
  lookup.

Trade-offs
- Writers must call `writeStringTableAtEnd()` exactly once after all
  payload writes and before `takeBytes()`.
- Readers that consume string references must initialize the table
  before any reference reads.

Change-Id: I2346e02fa34e0f421b6f15ccd485a0d19b00062d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/451126
Reviewed-by: Paul Berry <paulberry@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
2025-09-24 13:05:22 -07:00
Konstantin Shcheglov eb938ee4bb Fine. Integrate string table support into binary I/O
Fine-grained manifests previously wrote strings as raw UTF-8, which
increased bytes on the wire and added CPU overhead. This change
integrates string tables into the core BinaryWriter/BinaryReader so
manifests (and other formats) can use string references instead of
duplicated UTF-8 payloads.

What changed
- BinaryWriter
  - Add a shared StringIndexer and expose:
    - writeStringReference / writeStringList /
      writeOptionalStringReference
    - writeStringTableAtEnd() to emit the table and a trailing u32
      offset.
  - Add clone() that shares the StringIndexer with child writers for
    lazy/layered sections.
- BinaryReader
  - Add initializeStringTableAtOffset(offset) (rename from
    createStringTable).
  - Add initializeStringTableFromEnd(), the counterpart to
    writeStringTableAtEnd(), which reads the trailing u32 to find the
    table.
- Bundle writer/reader
  - Switch to writer string-table APIs and use
    initializeStringTableAtOffset() when reading.
  - ResolutionSink now extends BinaryWriter (removes the custom
    _SummaryDataWriter) and AstBinaryWriter delegates string writes to
    the sink.
  - Replace ad-hoc “_writeStringReference/_writeStringList” helpers
    with BinaryWriter methods; move small helpers to extensions.
- Data format
  - Bump AnalysisDriver.DATA_VERSION to 560 to invalidate old caches.

Why
- Reduces duplicate string storage across manifests and resolution
  chunks.
- Lowers serialization cost by replacing repeated UTF-8 blobs with
  compact integer references.
- Unifies string handling across summaries and manifests, simplifying
  future readers/writers.

Trade-offs
- Writers must finalize output correctly (emit the table before
  takeBytes when using the “table at end” mode); misuse will break the
  format.
- Sharing one StringIndexer across cloned writers slightly increases
  table size when many unique strings are produced, but the dedup win
  typically dominates.

Compatibility
- Existing bundle format continues to store four trailing u32 offsets
  (base/resolved/libraries/references/stringTable). The new “table at
  end” read path is added for formats that adopt it; bundle reading
  remains compatible by using the explicit offsets.

Change-Id: I3648a59eeb6af453f2744415c13f864e9b49a167
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/450972
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Paul Berry <paulberry@google.com>
2025-09-23 17:09:57 -07:00
Konstantin Shcheglov 9a720bb27a Fine. Rename binary I/O types to BinaryReader/Writer
Replace the generic names `SummaryDataReader` and `BufferedSink` with
clearer `BinaryReader` and `BinaryWriter`. The new names reflect their
actual roles (reading/writing the analyzer’s binary formats), reduce
confusion with “summary” artifacts, and make call sites easier to scan.
No encoding or behavioral changes are intended.

Key changes
- Introduce `BinaryReader` and `BinaryWriter` and update class docs.
- Rename constructor, method, and factory signatures across the codebase
  to use the new types (e.g., `read(...)`, `write(...)`, `fork(...)`).
- Update extensions to match the new names (`BinaryReaderExtension`,
  `BinaryWriterExtension`) and migrate helper methods accordingly.
- Adjust `string_table.dart` to take `BinaryWriter` and update `_writeWtf8`.
- Update readers/writers throughout analyzer components:
  - library context/diagnostics, unlinked data, manifests, IDs, items,
    types, requirements, and informative data.
  - utilities for `EnumSet.read`/`write`.
  - summary2 bundle reader/writer and package bundle format.
- Keep `_SummaryDataWriter` as an internal adapter now extending
  `BinaryWriter`.

Compatibility
- Binary format is unchanged; this is a pure rename refactor.
- Endianness, variable-length encodings, and offsets remain identical.

Change-Id: I97c07a708c1bd649084fc78a7c60c583964109ca
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/450950
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
2025-09-23 09:16:36 -07:00
Konstantin Shcheglov 78697d9af9 Fine. Tracking for LibraryElementImpl.exportedLibraries
Change-Id: Ia2a74667f97efbbb45dde7b95e4649edbc1a55dc
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/449681
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
2025-09-16 07:23:21 -07:00
Konstantin Shcheglov a387b8b9b1 Fine. Tracking for LibraryElementImpl.featureSet and languageVersion.
Change-Id: If5d0e4e4060af91a3e5f315c98bf5c8ce17e0acc
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/449387
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
2025-09-15 11:39:50 -07:00
Konstantin Shcheglov ea8da90d20 Elements. Standardize to Uint30/Uint32 in binary I/O.
Unify method names to use `Uint` (not `UInt`) for 30- and 32-bit helpers
across analyzer binaries. This aligns with Dart’s `dart:typed_data`
naming (e.g., `Uint8List`) and removes mixed spelling that caused
confusion. Behavior and wire format are unchanged; this is a pure API
rename with call-site updates.

Key renames (old → new):
- `readUInt30` → `readUint30`
- `readOptionalUInt30` → `readOptionalUint30`
- `readUInt30List` → `readUint30List`
- `readUInt32` → `readUint32`
- `readUInt32List` → `readUint32List`
- `writeUInt30` → `writeUint30`
- `writeOptionalUInt30` → `writeOptionalUint30`
- `writeUInt32` → `writeUint32`

Internal helpers were updated similarly (e.g., `_readUInt32` →
`_readUint32`, `_writeUInt30` → `_writeUint30`), and all uses in
`binary_reader.dart`, `binary_writer.dart`, string tables, unlinked
data, fine-manifest code, and summary2 readers/writers were adjusted.

Rationale:
- Consistent API surface that matches Dart conventions.
- Clearer intent for unsigned-width encodings.
- Zero impact on serialization format or runtime behavior.

Change-Id: I0d76239da8809b0187e8db975ec35cb46d08aab0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/449144
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
2025-09-12 09:17:59 -07:00
Konstantin Shcheglov 92395a1fe4 Elements. Move binary sink / reader out of summaries.
They are used for more than only summaries.

I will rename classes, and do other changes in separate CLs.

Change-Id: Ia4dec125743a8cf63df11974f8bd42037b1b81da
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/448800
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
2025-09-08 12:18:10 -07:00