Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[native_assets_cli] link hook resources.json format #1093

Open
dcharkes opened this issue Apr 17, 2024 · 0 comments
Open

[native_assets_cli] link hook resources.json format #1093

dcharkes opened this issue Apr 17, 2024 · 0 comments

Comments

@dcharkes
Copy link
Collaborator

dcharkes commented Apr 17, 2024

For tree-shaking code and data assets, the link hook needs to get the information from AOT-compiled Dart code about what assets are referenced.

To identify uses of assets, static methods are annotated with @ResourceIdentifier (or @pragma('dart2js:resource-identifier'). Subsequently all call sites of such methods "resource references" are collected.

Design goals for use in hook/link.dart

Design goals within the context of native assets:

  • JSON serialization
    • Stable, unrelated changes should not cause the serialization to change. Caching of link hooks depends on the input.
    • Complete.

Current situation

The current implementation for the JSON serialization for dart2js is in and for the VM.

The current serialization:

{
  "_comment": "Resources referenced by annotated resource identifers",
  "identifiers": [
    {
      "name": "loadDeferredLibrary",
      "uri": "org-dartlang-sdk:///lib/_internal/js_runtime/lib/js_helper.dart",
      "id": "id",
      "nonconstant": false,
      "files": [
        {
          "filename": "o.js",
          "references": [
            {
              "@": {
                "uri": "benchmarks/OmnibusDeferred/dart/OmnibusDeferred.dart",
                "line": 14,
                "column": 48
              },
              "1": "lib_SHA1",
              "2": 0
            },
            {
              "@": {
                "uri": "benchmarks/OmnibusDeferred/dart/OmnibusDeferred.dart",
                "line": 15,
                "column": 52
              },
              "1": "lib_SHA256",
              "2": 0
            }
          ]
        },
        {
          "filename": "o.js_37",
          "references": [
            {
              "@": {
                "uri": "benchmarks/OmnibusDeferred/dart/OmnibusDeferred_helper.dart",
                "line": 6,
                "column": 17
              },
              "1": "lib_BigIntParsePrint",
              "2": 0
            }
          ]
        }
      ]
    }
  ]
}

The current format is problematic for stability:

  • Line numbers and columns can change due to arbitrary other changes, leading to recompilations.
    • I believe line number and column should be removed.
    • Without line numbers and colums the source file might be useless information, so maybe it should be omitted.
  • Loading units might arbitrarily change (they are computed automatically based on a partitioning by the compiler).

The current format is imprecise:

  • Method names are currently not scoped by their surrounding class or extension, leading to ambiguities.

Versioning:

  • The format should contain a version field. Version skew between the Dart/Flutter SDK and the invoked hook/link.dart means that the parser used in the link hook needs to be able to deal with version changes. (Moreover, we can't do breaking changes, only additive changes. The link hook invoker does not know what version the link hook expects.)

(Also the two implementations don't agree on nonconstant vs nonConstant.)

The current API for the JSON format which should be re-exposed for users is here at line 157.

It puts loading-units front and centre, but it is not yet clear whether having that information is useful for link hooks.

Proposed solution

{
  "_comment": "Resources referenced by annotated resource identifers",
  "_version": [
    1,
    0,
    0
  ],
  "identifiers": [
    {
      "uri": "org-dartlang-sdk:///lib/_internal/js_runtime/lib/js_helper.dart",
      "containerType": "class",
      "container": "MyClass",
      "name": "loadDeferredLibrary",
      "metadata": { "some-structured": "data" },
      "constant": true,
      "references": [
        {
          "arguments": {
            "1": "lib_SHA1",
            "2": 0
          }
        },
        {
          "arguments": {
            "1": "lib_SHA256",
            "2": 0
          }
        },
        {
          "arguments": {
            "1": "lib_BigIntParsePrint",
            "2": 0
          }
        }
      ]
    }
  ]
}

If it turns out that loading units are useful in some situations, maybe we should support having them optionally in the format, so that the user of the format can decide to not include them. And ditto for line/column/uri.

The resource reference uri is a string, while the resource identifier uri is a real uri (with scheme). This should be consistent!

{
  "_comment": "Resources referenced by annotated resource identifers",
  "_version": [
    1,
    0,
    0
  ],
  "identifiers": [
    {
      "uri": "org-dartlang-sdk:///lib/_internal/js_runtime/lib/js_helper.dart",
      "containerType": "class",
      "container": "MyClass",
      "name": "loadDeferredLibrary",
      "constant": true,
      "references": [
        {
          "arguments": {
            "1": "lib_SHA1",
            "2": 0
          },
          "loadingUnit": "o.js",
          "@": {
            "uri": "file://benchmarks/OmnibusDeferred/dart/OmnibusDeferred.dart",
            "line": 14,
             "column": 48
          }
        },
      ]
    }
  ]
}

The containerType can be "top-level", "class", "extension", "extension-type".
The container is not provided for top-level, but is for the other

The Dart API should then not have loading units as front and center and change accordingly:

final class ResourceIdentifier {
  Uri get uri;
  String get containerType;
  String get container;
  String get name;

  Object get metadata;

  bool get constant;

  Iterable<ResourceReference> get references;
}

final class ResourceReference {
  Map<String, Object?> get arguments;
  
  String? get loadingUnit;

  Uri? get uri;
  int? get line;
  int? column;
}

For the link hook we'd like don't want to provide the loadingUnit, uri, line, and column to aid caching.

Should hook/link.dart have its own format, or share it with dart2js?

One question we should answer is whether the format we pass to link hooks should be identical to what dart2js outputs. Do we have compatible requirements for the format?

If we add support for data and wasm assets at some point to dart2js, it might make sense if it is shared.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant