dart-lang / native / 21834574551

75%

Build Type

push

github

Committed by web-flow

Commit Message

[record_use] Remove definition loading unit (#3061)

In `dart2js`, definitions currently include a `loading_unit` property which typically represents the "dominating" loading unit (the shared ancestor of all loading units where that definition is used). 

When assets are associated with a definition, using this definition-level `loading_unit` can lead to assets being loaded too early. For example, if a definition is used in two different deferred loading units, its dominating unit might be the main bundle. Loading the asset in the main bundle defeats the purpose of deferring it.

To fix this, we want to postpone asset loading until one of the actual loading units that uses the definition is loaded. Since every recorded `Call` and `Instance` already includes its own `loading_unit`, the property on the `Definition` itself is redundant and misleading for the web backend.

Closes: https://github.com/dart-lang/native/issues/2986

Issues to be addressed in follow up PRs:

* https://github.com/dart-lang/native/issues/3022
* https://github.com/dart-lang/native/issues/2888

## Changes

### 1. JSON Schema & Syntax
- Removed `loading_unit` from the `Definition` object in `record_use.schema.json`.
- Updated `Recording` to use `identifier` directly as its key property.

### 2. Dart API
- Removed the `Definition` class. `Recordings` now uses `Identifier` as the primary key for the `callsForDefinition` and `instancesForDefinition` maps.
- Added a `TODO` to `Identifier` to rename it to `Definition` in a future refactor ([#2888](https://github.com/dart-lang/native/issues/2888)).

Run Details

44 of 50 new or added lines in 2 files covered. (88.0%)

1 existing line in 1 file now uncovered.

5755 of 8016 relevant lines covered (71.79%)

71.33 hits per line