[dart:ffi] Only operate on transitive dependencies of dart:ffi
This CL does two things:
* "Reenable" the skipping of the dart:ffi transformation entirely when
the application does not use dart:ffi. This was originally added in
ad596359a676 but was logically disabled in 455811210525 when dart:core
started depending on it. Here we now ignore (real) dart:* libraries
when looking for dependencies of dart:ffi.
* Find the subset of the just compiled libraries that transitively
depend on dart:ffi and just run the dart:ffi transformation on those
libraries.
For dart2js that doesn't use dart:ffi at all we go from:
```
$ out/ReleaseX64/dart pkg/front_end/tool/_fasta/compile.dart -v pkg/compiler/bin/dart2js.dart | grep -i "ffi"
0:00:09.003428: Transformed ffi natives in 51ms.
0:00:09.203442: Transformed ffi annotations in 199ms.
$ out/ReleaseX64/dart pkg/front_end/tool/_fasta/compile.dart -v pkg/compiler/bin/dart2js.dart | grep -i "ffi"
0:00:09.098175: Transformed ffi natives in 52ms.
0:00:09.296847: Transformed ffi annotations in 198ms.
$ out/ReleaseX64/dart pkg/front_end/tool/_fasta/compile.dart -v pkg/compiler/bin/dart2js.dart | grep -i "ffi"
0:00:08.854341: Transformed ffi natives in 57ms.
0:00:09.061804: Transformed ffi annotations in 207ms.
```
to
```
$ out/ReleaseX64/dart pkg/front_end/tool/_fasta/compile.dart -v pkg/compiler/bin/dart2js.dart | grep -i "ffi"
0:00:09.138651: Skipped ffi transformation in 1ms.
$ out/ReleaseX64/dart pkg/front_end/tool/_fasta/compile.dart -v pkg/compiler/bin/dart2js.dart | grep -i "ffi"
0:00:09.242590: Skipped ffi transformation in 1ms.
$ out/ReleaseX64/dart pkg/front_end/tool/_fasta/compile.dart -v pkg/compiler/bin/dart2js.dart | grep -i "ffi"
0:00:09.163885: Skipped ffi transformation in 1ms.
```
When compiling the flutter gallery app that does depend on dart:ffi we go from:
```
$ out/ReleaseX64/dart pkg/front_end/tool/_fasta/compile.dart -v --target=flutter --platform=/tmp/tmp.6KV5psVGIG/flutter_patched_sdk/platform_strong.dill /tmp/tmp.6KV5psVGIG/gallery/lib/main.dart | grep -i "ffi"
0:00:16.344955: Transformed ffi natives in 78ms.
0:00:16.777218: Transformed ffi annotations in 432ms.
$ out/ReleaseX64/dart pkg/front_end/tool/_fasta/compile.dart -v --target=flutter --platform=/tmp/tmp.6KV5psVGIG/flutter_patched_sdk/platform_strong.dill /tmp/tmp.6KV5psVGIG/gallery/lib/main.dart | grep -i "ffi"
0:00:15.994674: Transformed ffi natives in 80ms.
0:00:16.472779: Transformed ffi annotations in 478ms.
$ out/ReleaseX64/dart pkg/front_end/tool/_fasta/compile.dart -v --target=flutter --platform=/tmp/tmp.6KV5psVGIG/flutter_patched_sdk/platform_strong.dill /tmp/tmp.6KV5psVGIG/gallery/lib/main.dart | grep -i "ffi"
0:00:16.027488: Transformed ffi natives in 81ms.
0:00:16.491362: Transformed ffi annotations in 463ms.
```
to
```
$ out/ReleaseX64/dart pkg/front_end/tool/_fasta/compile.dart -v --target=flutter --platform=/tmp/tmp.6KV5psVGIG/flutter_patched_sdk/platform_strong.dill /tmp/tmp.6KV5psVGIG/gallery/lib/main.dart | grep -i "ffi"
0:00:15.991628: Transformed ffi natives in 30ms.
0:00:16.226564: Transformed ffi annotations in 234ms.
$ out/ReleaseX64/dart pkg/front_end/tool/_fasta/compile.dart -v --target=flutter --platform=/tmp/tmp.6KV5psVGIG/flutter_patched_sdk/platform_strong.dill /tmp/tmp.6KV5psVGIG/gallery/lib/main.dart | grep -i "ffi"
0:00:16.184984: Transformed ffi natives in 29ms.
0:00:16.404439: Transformed ffi annotations in 219ms.
$ out/ReleaseX64/dart pkg/front_end/tool/_fasta/compile.dart -v --target=flutter --platform=/tmp/tmp.6KV5psVGIG/flutter_patched_sdk/platform_strong.dill /tmp/tmp.6KV5psVGIG/gallery/lib/main.dart | grep -i "ffi"
0:00:15.933513: Transformed ffi natives in 27ms.
0:00:16.145640: Transformed ffi annotations in 212ms.
```
TEST=Assuming tests already exists.
Change-Id: Ie13be1924d3439cb710c782af81e1e42cda2a838
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/228001
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Daco Harkes <dacoharkes@google.com>
Commit-Queue: Jens Johansen <jensj@google.com>
diff --git a/pkg/kernel/lib/util/graph.dart b/pkg/kernel/lib/util/graph.dart
index 105b170b..eef4b6b 100644
--- a/pkg/kernel/lib/util/graph.dart
+++ b/pkg/kernel/lib/util/graph.dart
@@ -208,3 +208,37 @@
/// That is, `layers[i]` contain the list of vertices with index `i`.
final List<List<T>> layers = [];
}
+
+/// Find vertices in [graph] that either is in [vertices], has a neighbor in
+/// [vertices], has a neighbor of a neighbor (etc) in [vertices].
+Set<T> calculateTransitiveDependenciesOf<T>(Graph<T> graph, Set<T> vertices) {
+ // Compute direct dependencies for each vertex (the reverse of the
+ // edges returned by `graph.neighborsOf`).
+ Map<T, Set<T>> directDependencies = <T, Set<T>>{};
+ List<T> workList = [];
+ {
+ for (T vertex in graph.vertices) {
+ if (vertices.contains(vertex)) workList.add(vertex);
+ for (T neighbor in graph.neighborsOf(vertex)) {
+ (directDependencies[neighbor] ??= new Set<T>()).add(vertex);
+ if (vertices.contains(neighbor)) workList.add(vertex);
+ }
+ }
+ }
+
+ // Collect and remove all dependencies.
+ Set<T> left = new Set<T>.from(graph.vertices);
+ Set<T> transitive = {};
+ while (workList.isNotEmpty) {
+ T removed = workList.removeLast();
+ if (left.remove(removed)) {
+ Set<T>? s = directDependencies[removed];
+ if (s != null) {
+ // [s] is null for leaves.
+ workList.addAll(s);
+ }
+ transitive.add(removed);
+ }
+ }
+ return transitive;
+}
diff --git a/pkg/kernel/test/graph_test.dart b/pkg/kernel/test/graph_test.dart
index 83827be..cbcb0d1 100644
--- a/pkg/kernel/test/graph_test.dart
+++ b/pkg/kernel/test/graph_test.dart
@@ -556,4 +556,103 @@
[F]
]
]);
+
+ testTransitiveDependencies(graphData: {
+ A: [B],
+ B: [A],
+ }, of: {
+ A
+ }, expected: {
+ A,
+ B,
+ });
+
+ testTransitiveDependencies(graphData: {}, of: {
+ A,
+ B,
+ C,
+ D,
+ E,
+ F,
+ }, expected: {});
+
+ {
+ String MAIN = "MAIN";
+ String DARTFFI = "DARTFFI";
+ testTransitiveDependencies(graphData: {
+ MAIN: [DARTFFI],
+ }, of: {
+ DARTFFI
+ }, expected: {
+ MAIN,
+ });
+ }
+
+ testTransitiveDependencies(graphData: {
+ A: [B],
+ B: [C],
+ C: [B, D],
+ D: [C],
+ E: [A],
+ F: [B],
+ }, of: {
+ A
+ }, expected: {
+ A,
+ E
+ });
+
+ {
+ String MAIN = "MAIN";
+ String TARGET = "TARGET";
+
+ testTransitiveDependencies(graphData: {
+ MAIN: [A, E],
+ A: [B],
+ B: [A, C],
+ C: [TARGET],
+ D: [],
+ E: [D],
+ }, of: {
+ TARGET,
+ }, expected: {
+ C,
+ B,
+ A,
+ MAIN,
+ });
+
+ testTransitiveDependencies(graphData: {
+ MAIN: [A, E],
+ A: [B],
+ B: [A, C],
+ C: [],
+ D: [],
+ E: [D],
+ }, of: {
+ TARGET,
+ }, expected: {});
+
+ testTransitiveDependencies(graphData: {
+ MAIN: [A, C, E, TARGET],
+ A: [B],
+ B: [A, C],
+ C: [],
+ D: [],
+ E: [D],
+ }, of: {
+ TARGET,
+ }, expected: {
+ MAIN,
+ });
+ }
+}
+
+void testTransitiveDependencies(
+ {required Set<String> of,
+ required Set<String> expected,
+ required Map<String, List<String>> graphData}) {
+ Graph<String> graph = new TestGraph(graphData);
+ Set<String> actual = calculateTransitiveDependenciesOf(graph, of);
+ Expect.setEquals(expected, actual);
}
diff --git a/pkg/vm/lib/target/vm.dart b/pkg/vm/lib/target/vm.dart
index da77a9e..b1a60d8 100644
--- a/pkg/vm/lib/target/vm.dart
+++ b/pkg/vm/lib/target/vm.dart
@@ -18,7 +18,8 @@
import '../transformations/call_site_annotator.dart' as callSiteAnnotator;
import '../transformations/lowering.dart' as lowering
show transformLibraries, transformProcedure;
-import '../transformations/ffi/common.dart' as ffiHelper show importsFfi;
+import '../transformations/ffi/common.dart' as ffiHelper
+ show calculateTransitiveImportsOfDartFfiIfUsed;
import '../transformations/ffi/definitions.dart' as transformFfiDefinitions
show transformLibraries;
import '../transformations/ffi/native.dart' as transformFfiNative
@@ -154,7 +155,9 @@
this, coreTypes, hierarchy, libraries, referenceFromIndex);
logger?.call("Transformed mixin applications");
- if (!ffiHelper.importsFfi(component, libraries)) {
+ List<Library>? transitiveImportingDartFfi = ffiHelper
+ .calculateTransitiveImportsOfDartFfiIfUsed(component, libraries);
+ if (transitiveImportingDartFfi == null) {
logger?.call("Skipped ffi transformation");
} else {
// Transform @FfiNative(..) functions into FFI native call functions.
@@ -163,21 +166,19 @@
// Transform arguments that extend NativeFieldWrapperClass1 to Pointer if
// the native function expects Pointer (to avoid Handle overhead).
transformFfiNative.transformLibraries(component, coreTypes, hierarchy,
- libraries, diagnosticReporter, referenceFromIndex);
+ transitiveImportingDartFfi, diagnosticReporter, referenceFromIndex);
logger?.call("Transformed ffi natives");
- // TODO(jensj/dacoharkes): We can probably limit the transformations to
- // libraries that transitivley depend on dart:ffi.
transformFfiDefinitions.transformLibraries(
component,
coreTypes,
hierarchy,
- libraries,
+ transitiveImportingDartFfi,
diagnosticReporter,
referenceFromIndex,
changedStructureNotifier);
transformFfiUseSites.transformLibraries(component, coreTypes, hierarchy,
- libraries, diagnosticReporter, referenceFromIndex);
+ transitiveImportingDartFfi, diagnosticReporter, referenceFromIndex);
logger?.call("Transformed ffi annotations");
}
diff --git a/pkg/vm/lib/transformations/ffi/common.dart b/pkg/vm/lib/transformations/ffi/common.dart
index 4446159..c23bddb 100644
--- a/pkg/vm/lib/transformations/ffi/common.dart
+++ b/pkg/vm/lib/transformations/ffi/common.dart
@@ -16,6 +16,7 @@
import 'package:kernel/type_algebra.dart' show Substitution;
import 'package:kernel/type_environment.dart'
show TypeEnvironment, SubtypeCheckMode;
+import 'package:kernel/util/graph.dart' as kernelGraph;
import 'abi.dart';
import 'native_type_cfe.dart';
@@ -980,18 +981,61 @@
}
}
+/// Returns all libraries including the ones from component except for platform
+/// libraries that are only in component.
+Set<Library> _getAllRelevantLibraries(
+ Component component, List<Library> libraries) {
+ Set<Library> allLibs = {};
+ allLibs.addAll(libraries);
+ for (Library lib in component.libraries) {
+ // Skip real dart: libraries. dart:core imports dart:ffi, but that doesn't
+ // mean we have to transform anything.
+ if (lib.importUri.scheme == "dart" && !lib.isSynthetic) continue;
+ allLibs.add(lib);
+ }
+ return allLibs;
+}
+
/// Checks if any library depends on dart:ffi.
-bool importsFfi(Component component, List<Library> libraries) {
- Set<Library> allLibs = {...component.libraries, ...libraries};
+Library? importsFfi(Component component, List<Library> libraries) {
final Uri dartFfiUri = Uri.parse("dart:ffi");
+ Set<Library> allLibs = _getAllRelevantLibraries(component, libraries);
for (Library lib in allLibs) {
for (LibraryDependency dependency in lib.dependencies) {
- if (dependency.targetLibrary.importUri == dartFfiUri) {
- return true;
+ Library targetLibrary = dependency.targetLibrary;
+ if (targetLibrary.importUri == dartFfiUri) {
+ return targetLibrary;
}
}
}
- return false;
+ return null;
+}
+
+/// Calculates the libraries in [libraries] that transitively imports dart:ffi.
+///
+/// Returns null if dart:ffi is not imported.
+List<Library>? calculateTransitiveImportsOfDartFfiIfUsed(
+ Component component, List<Library> libraries) {
+ Set<Library> allLibs = _getAllRelevantLibraries(component, libraries);
+
+ final Uri dartFfiUri = Uri.parse("dart:ffi");
+ Library? dartFfi;
+ canFind:
+ for (Library lib in allLibs) {
+ for (LibraryDependency dependency in lib.dependencies) {
+ Library targetLibrary = dependency.targetLibrary;
+ if (targetLibrary.importUri == dartFfiUri) {
+ dartFfi = targetLibrary;
+ break canFind;
+ }
+ }
+ }
+ if (dartFfi == null) return null;
+
+ kernelGraph.LibraryGraph graph = new kernelGraph.LibraryGraph(allLibs);
+ Set<Library> result =
+ kernelGraph.calculateTransitiveDependenciesOf(graph, {dartFfi});
+ return (result..retainAll(libraries)).toList();
}
extension on Map<Abi, Object?> {
