[dart2wasm] Don't use IDs as keys for export name mapping.

We have means of serializing References and Constants directly, so it is
unnecessary to generate IDs as indices. This does cause the export name
mapping to get split up a little (just as callable reference IDs were
stored separately from constant export IDs).

This is because the kernel metadata repositories are serialized by the
kernel's BinarySink, which correctly handles Constants, but cannot
serialize References from the TFA'd component to the metadata of the
un-TFA'd component. Meanwhile, the dart2wasm DataSerializer can
serialize these References, but does not handle Constants.

Therefore, the callable reference mapping is stored on the main module
metadata (just as callable reference IDs were), while the constant
mapping is stored in a metadata repository (just as constant IDs were).

The exporter must then be aware of both of these to be used as a
centralized interface.

Change-Id: I1c0fc3eed5a96b3c3e755826749ebd29c3fa2b4b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/446990
Reviewed-by: Nate Biggs <[email protected]>

Author: fishythefish

pkg/dart2wasm/lib/constants.dart

@@ -13,8 +13,6 @@ import 'package:wasm_builder/wasm_builder.dart' as w;
 import 'class_info.dart';
 import 'closures.dart';
 import 'code_generator.dart';
-import 'dynamic_module_kernel_metadata.dart'
-    show DynamicModuleConstantRepository;
 import 'dynamic_modules.dart';
 import 'param_info.dart';
 import 'translator.dart';
@@ -72,10 +70,6 @@ typedef ConstantCodeGenerator = void Function(w.InstructionsBuilder);
 class Constants {
   final Translator translator;
   final Map<Constant, ConstantInfo> constantInfo = {};
-  late final Map<Constant, int>? dynamicMainModuleConstantId = (translator
-              .component.metadata[DynamicModuleConstantRepository.repositoryTag]
-          as DynamicModuleConstantRepository?)
-      ?.mapping[translator.component] ??= {};
   w.DataSegmentBuilder? int32Segment;
   late final ClassInfo typeInfo = translator.classInfo[translator.typeClass]!;
 
@@ -498,10 +492,6 @@ class ConstantCreator extends ConstantVisitor<ConstantInfo?>
     return info;
   }
 
-  static String _dynamicModuleConstantExportName(int id) => '#constant$id';
-  static String _dynamicModuleInitFunctionExportName(int id) =>
-      '#constantFunction$id';
-
   static int _nextGlobalId = 0;
   String _constantName(Constant constant) {
     final id = _nextGlobalId++;
@@ -557,16 +547,19 @@ class ConstantCreator extends ConstantVisitor<ConstantInfo?>
     assert(!type.nullable);
 
     // This function is only called once per [Constant]. If we compile a dynamic
-    // submodule then the [dynamicModuleConstantIdMap] is pre-populated and
+    // submodule then `translator.dynamicModuleConstants` is pre-populated and
     // we may find an export name. If we compile the main module, then the id
     // will be `null`.
-    final dynamicModuleConstantIdMap = constants.dynamicMainModuleConstantId;
-    final mainModuleExportId = dynamicModuleConstantIdMap?[constant];
-    final isShareableAcrossModules = dynamicModuleConstantIdMap != null &&
-        constant.accept(_ConstantDynamicModuleSharedChecker(translator));
+    final isExportedFromMainModule = translator
+            .dynamicModuleConstants?.constantNames
+            .containsKey(constant) ??
+        false;
+    final isShareableAcrossModules =
+        translator.dynamicModuleConstants != null &&
+            constant.accept(_ConstantDynamicModuleSharedChecker(translator));
     final needsRuntimeCanonicalization = isShareableAcrossModules &&
         translator.isDynamicSubmodule &&
-        mainModuleExportId == null;
+        !isExportedFromMainModule;
 
     if (lazy || needsRuntimeCanonicalization) {
       // Create uninitialized global and function to initialize it.
@@ -577,16 +570,15 @@ class ConstantCreator extends ConstantVisitor<ConstantInfo?>
       w.FunctionType ftype =
           translator.typesBuilder.defineFunction(const [], [type]);
 
-      if (mainModuleExportId != null) {
+      if (isExportedFromMainModule) {
         global = targetModule.globals.import(
             translator.mainModule.moduleName,
-            translator.dynamicModuleExports!
-                .getConstantName(mainModuleExportId)!,
+            translator.dynamicModuleConstants!.constantNames[constant]!,
             globalType);
         initFunction = targetModule.functions.import(
             translator.mainModule.moduleName,
-            translator.dynamicModuleExports!
-                .getConstantInitializerName(mainModuleExportId)!,
+            translator
+                .dynamicModuleConstants!.constantInitializerNames[constant]!,
             ftype);
       } else {
         final name = _constantName(constant);
@@ -599,19 +591,9 @@ class ConstantCreator extends ConstantVisitor<ConstantInfo?>
             targetModule.functions.define(ftype, '$name (lazy initializer)}');
 
         if (isShareableAcrossModules) {
-          final exportId = dynamicModuleConstantIdMap[constant] =
-              dynamicModuleConstantIdMap.length;
-
-          translator.exporter.exportConstant(
-              targetModule,
-              _dynamicModuleConstantExportName(exportId),
-              definedGlobal,
-              exportId);
-          translator.exporter.exportConstantInitializer(
-              targetModule,
-              _dynamicModuleInitFunctionExportName(exportId),
-              function,
-              exportId);
+          translator.exporter.exportDynamicConstant(
+              targetModule, constant, definedGlobal,
+              initializer: function);
         }
         final b2 = function.body;
         generator(b2);
@@ -632,11 +614,10 @@ class ConstantCreator extends ConstantVisitor<ConstantInfo?>
       assert(!constants.currentlyCreating);
       final globalType = w.GlobalType(type, mutable: false);
       w.Global global;
-      if (mainModuleExportId != null) {
+      if (isExportedFromMainModule) {
         global = targetModule.globals.import(
             translator.mainModule.moduleName,
-            translator.dynamicModuleExports!
-                .getConstantName(mainModuleExportId)!,
+            translator.dynamicModuleConstants!.constantNames[constant]!,
             globalType);
       } else {
         constants.currentlyCreating = true;
@@ -647,14 +628,8 @@ class ConstantCreator extends ConstantVisitor<ConstantInfo?>
         constants.currentlyCreating = false;
 
         if (isShareableAcrossModules) {
-          final exportId = dynamicModuleConstantIdMap[constant] =
-              dynamicModuleConstantIdMap.length;
-
-          translator.exporter.exportConstant(
-              targetModule,
-              _dynamicModuleConstantExportName(exportId),
-              definedGlobal,
-              exportId);
+          translator.exporter
+              .exportDynamicConstant(targetModule, constant, definedGlobal);
         }
       }
 

pkg/dart2wasm/lib/dynamic_module_kernel_metadata.dart

@@ -27,7 +27,6 @@ import 'class_info.dart';
 import 'compiler_options.dart';
 import 'dispatch_table.dart';
 import 'dynamic_modules.dart';
-import 'exports.dart';
 import 'js/method_collector.dart' show JSMethods;
 import 'serialization.dart';
 import 'translator.dart';
@@ -62,34 +61,57 @@ class DynamicModuleGlobalIdRepository extends MetadataRepository<int> {
 
 /// Repository for kernel constants.
 class DynamicModuleConstantRepository
-    extends MetadataRepository<Map<Constant, int>> {
+    extends MetadataRepository<DynamicModuleConstants> {
   static const repositoryTag = 'wasm.dynamic-modules.constants';
 
   @override
   final String tag = repositoryTag;
 
   @override
-  final Map<TreeNode, Map<Constant, int>> mapping = {};
+  final Map<TreeNode, DynamicModuleConstants> mapping = {};
 
   @override
-  Map<Constant, int> readFromBinary(Node node, BinarySource source) {
-    final length = source.readUInt30();
-    final Map<Constant, int> constants = {};
-    for (int i = 0; i < length; i++) {
-      final constant = source.readConstantReference();
-      final id = source.readUInt30();
-      constants[constant] = id;
+  DynamicModuleConstants readFromBinary(_, BinarySource source) =>
+      DynamicModuleConstants._readFromBinary(source);
+
+  @override
+  void writeToBinary(DynamicModuleConstants data, _, BinarySink sink) =>
+      data._writeToBinary(sink);
+}
+
+class DynamicModuleConstants {
+  final Map<Constant, String> constantNames = {};
+  final Map<Constant, String> constantInitializerNames = {};
+
+  DynamicModuleConstants();
+
+  factory DynamicModuleConstants._readFromBinary(BinarySource source) {
+    void readMap(Map<Constant, String> map) {
+      final length = source.readUInt30();
+      for (int i = 0; i < length; i++) {
+        final constant = source.readConstantReference();
+        final name = source.readStringReference();
+        map[constant] = name;
+      }
     }
-    return constants;
+
+    final exports = DynamicModuleConstants();
+    readMap(exports.constantNames);
+    readMap(exports.constantInitializerNames);
+    return exports;
   }
 
-  @override
-  void writeToBinary(Map<Constant, int> constants, Node node, BinarySink sink) {
-    sink.writeUInt30(constants.length);
-    constants.forEach((constant, id) {
-      sink.writeConstantReference(constant);
-      sink.writeUInt30(id);
-    });
+  void _writeToBinary(BinarySink sink) {
+    void writeMap(Map<Constant, String> map) {
+      sink.writeUInt30(map.length);
+      map.forEach((key, name) {
+        sink.writeConstantReference(key);
+        sink.writeStringReference(name);
+      });
+    }
+
+    writeMap(constantNames);
+    writeMap(constantInitializerNames);
   }
 }
 
@@ -215,9 +237,8 @@ class MainModuleMetadata {
   /// Class to metadata about the class.
   final Map<Class, ClassMetadata> classMetadata;
 
-  /// Maps dynamic callable references to a unique ID that is used to generate
-  /// the export name for the reference.
-  final Map<Reference, int> callableReferenceIds;
+  /// Maps dynamic callable references to export names.
+  final Map<Reference, String> callableReferenceNames;
 
   late final DispatchTable dispatchTable;
 
@@ -239,7 +260,7 @@ class MainModuleMetadata {
 
   MainModuleMetadata._(
       this.classMetadata,
-      this.callableReferenceIds,
+      this.callableReferenceNames,
       this.dispatchTable,
       this.invokedOverridableReferences,
       this.keyInvocationToIndex,
@@ -250,7 +271,7 @@ class MainModuleMetadata {
   MainModuleMetadata.empty(
       this.mainModuleTranslatorOptions, this.mainModuleEnvironment)
       : classMetadata = {},
-        callableReferenceIds = {},
+        callableReferenceNames = {},
         invokedOverridableReferences = {},
         keyInvocationToIndex = {},
         dfsOrderClassIds = [];
@@ -296,7 +317,8 @@ class MainModuleMetadata {
 
     sink.writeMap(classMetadata, sink.writeClass, (m) => m.serialize(sink));
 
-    sink.writeMap(callableReferenceIds, sink.writeReference, sink.writeInt);
+    sink.writeMap(
+        callableReferenceNames, sink.writeReference, sink.writeString);
 
     dispatchTable.serialize(sink);
 
@@ -314,8 +336,8 @@ class MainModuleMetadata {
     final classMetadata = source.readMap(
         source.readClass, () => ClassMetadata.deserialize(source));
 
-    final callableReferenceIds =
-        source.readMap(source.readReference, source.readInt);
+    final callableReferenceNames =
+        source.readMap(source.readReference, source.readString);
 
     final dispatchTable = DispatchTable.deserialize(source);
 
@@ -331,7 +353,7 @@ class MainModuleMetadata {
 
     final metadata = MainModuleMetadata._(
         classMetadata,
-        callableReferenceIds,
+        callableReferenceNames,
         dispatchTable,
         invokedReferences,
         keyInvocationToIndex,
@@ -475,7 +497,6 @@ Future<(Component, JSMethods)> generateDynamicSubmoduleComponent(
   final newComponent = Component()
     ..addMetadataRepository(DynamicModuleGlobalIdRepository())
     ..addMetadataRepository(DynamicModuleConstantRepository())
-    ..addMetadataRepository(DynamicModuleExportRepository())
     ..addMetadataRepository(ProcedureAttributesMetadataRepository())
     ..addMetadataRepository(TableSelectorMetadataRepository())
     ..addMetadataRepository(DirectCallMetadataRepository())

pkg/dart2wasm/lib/dynamic_modules.dart

@@ -22,7 +22,6 @@ import 'compiler_options.dart';
 import 'constants.dart' show maxArrayNewFixedLength;
 import 'dispatch_table.dart';
 import 'dynamic_module_kernel_metadata.dart';
-import 'exports.dart';
 import 'intrinsics.dart' show MemberIntrinsic;
 import 'kernel_nodes.dart';
 import 'modules.dart';
@@ -130,7 +129,6 @@ class DynamicMainModuleStrategy extends ModuleStrategy with KernelNodes {
 
     component.addMetadataRepository(DynamicModuleConstantRepository());
     component.addMetadataRepository(DynamicModuleGlobalIdRepository());
-    component.addMetadataRepository(DynamicModuleExportRepository());
   }
 
   @override
@@ -520,8 +518,6 @@ class DynamicModuleInfo {
     final classId =
         (translator.classInfo[cls]!.classId as AbsoluteClassId).value;
 
-    metadata.callableReferenceIds[target] ??=
-        metadata.callableReferenceIds.length;
     // The class must be allocated in order for the target to be live.
     translator.functions.recordClassTargetUse(classId, target);
   }
@@ -537,8 +533,6 @@ class DynamicModuleInfo {
 
     // Generate static members immediately since they are unconditionally
     // callable.
-    metadata.callableReferenceIds[target] ??=
-        metadata.callableReferenceIds.length;
     translator.functions.getFunction(target);
   }