Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit fda5c69

Browse files
authored
[web] Avoid using js_util.{jsify,dartify}() in dart2wasm for converting to JS wrappers (#51375)
The `js_util.jsify()` related code shows up in CPU profile of wonderous. => Any `SkwasmObjectWrapper` object invokes this logic in the constructor and dispose method. This PR * makes `DomFinalizationRegistryExtension` accept `JSAny` types instead of Dart types and internally converting => Callsites can call more precise `<>.toJS*` extension methods => Will avoids extra type checks on the objects when we can call `Object.toJSBox` directly * makes us use a `toJSWrapper` / `fromJSWrapper` which will not delegate to a recursive `jsify()` but simply externalize the wasm gc object => We cannot use `Object.toJSBox` due to it being slower to create JS boxes as it semantically does something different atm (see issue below) => Instead use conditional import of `dart:_wasm` which provides the necessary primitives => Similar for going from JS to Dart. * Avoid converting from Dart object to `JSAny` more than needed (we did the operation twice for each registration and once for unregistration) Issue dart-lang/sdk#55183
1 parent 89df726 commit fda5c69

File tree

9 files changed

+75
-31
lines changed

9 files changed

+75
-31
lines changed

ci/licenses_golden/licenses_flutter

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40598,6 +40598,7 @@ ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/shadow.dart + ../../../flutte
4059840598
ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/skwasm/skwasm_impl.dart + ../../../flutter/LICENSE
4059940599
ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/canvas.dart + ../../../flutter/LICENSE
4060040600
ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/codecs.dart + ../../../flutter/LICENSE
40601+
ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/dart_js_conversion.dart + ../../../flutter/LICENSE
4060140602
ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/filters.dart + ../../../flutter/LICENSE
4060240603
ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/font_collection.dart + ../../../flutter/LICENSE
4060340604
ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/image.dart + ../../../flutter/LICENSE
@@ -40634,6 +40635,7 @@ ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/shaders.da
4063440635
ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/surface.dart + ../../../flutter/LICENSE
4063540636
ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/vertices.dart + ../../../flutter/LICENSE
4063640637
ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/skwasm/skwasm_stub.dart + ../../../flutter/LICENSE
40638+
ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/skwasm/skwasm_stub/dart_js_conversion.dart + ../../../flutter/LICENSE
4063740639
ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/skwasm/skwasm_stub/renderer.dart + ../../../flutter/LICENSE
4063840640
ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/svg.dart + ../../../flutter/LICENSE
4063940641
ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/test_embedding.dart + ../../../flutter/LICENSE
@@ -43471,6 +43473,7 @@ FILE: ../../../flutter/lib/web_ui/lib/src/engine/shadow.dart
4347143473
FILE: ../../../flutter/lib/web_ui/lib/src/engine/skwasm/skwasm_impl.dart
4347243474
FILE: ../../../flutter/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/canvas.dart
4347343475
FILE: ../../../flutter/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/codecs.dart
43476+
FILE: ../../../flutter/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/dart_js_conversion.dart
4347443477
FILE: ../../../flutter/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/filters.dart
4347543478
FILE: ../../../flutter/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/font_collection.dart
4347643479
FILE: ../../../flutter/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/image.dart
@@ -43507,6 +43510,7 @@ FILE: ../../../flutter/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/shaders.dart
4350743510
FILE: ../../../flutter/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/surface.dart
4350843511
FILE: ../../../flutter/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/vertices.dart
4350943512
FILE: ../../../flutter/lib/web_ui/lib/src/engine/skwasm/skwasm_stub.dart
43513+
FILE: ../../../flutter/lib/web_ui/lib/src/engine/skwasm/skwasm_stub/dart_js_conversion.dart
4351043514
FILE: ../../../flutter/lib/web_ui/lib/src/engine/skwasm/skwasm_stub/renderer.dart
4351143515
FILE: ../../../flutter/lib/web_ui/lib/src/engine/svg.dart
4351243516
FILE: ../../../flutter/lib/web_ui/lib/src/engine/test_embedding.dart

lib/web_ui/lib/src/engine/canvaskit/native_memory.dart

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,9 @@ import 'package:ui/src/engine.dart';
2020
/// 4. GC decides to perform a GC cycle and collects CkPaint.
2121
/// 5. The finalizer function is called with the SkPaint as the sole argument.
2222
/// 6. We call `delete` on SkPaint.
23-
DomFinalizationRegistry _finalizationRegistry = createDomFinalizationRegistry(
23+
DomFinalizationRegistry _finalizationRegistry = DomFinalizationRegistry(
2424
(JSBoxedDartObject boxedUniq) {
25-
final UniqueRef<Object> uniq = boxedUniq.toDart as UniqueRef<Object>;
25+
final UniqueRef<Object> uniq = boxedUniq.fromJSWrapper as UniqueRef<Object>;
2626
uniq.collect();
2727
}.toJS
2828
);
@@ -34,7 +34,7 @@ NativeMemoryFinalizationRegistry nativeMemoryFinalizationRegistry = NativeMemory
3434
class NativeMemoryFinalizationRegistry {
3535
void register(Object owner, UniqueRef<Object> ref) {
3636
if (browserSupportsFinalizationRegistry) {
37-
_finalizationRegistry.register(owner, ref.toJSBox);
37+
_finalizationRegistry.register(owner.toJSWrapper, ref.toJSWrapper);
3838
}
3939
}
4040
}

lib/web_ui/lib/src/engine/dom.dart

Lines changed: 29 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import 'dart:typed_data';
99

1010
import 'package:js/js_util.dart' as js_util;
1111
import 'package:meta/meta.dart';
12+
import 'package:ui/src/engine/skwasm/skwasm_stub.dart' if (dart.library.ffi) 'package:ui/src/engine/skwasm/skwasm_impl.dart';
1213

1314
import 'browser_detection.dart';
1415

@@ -37,6 +38,15 @@ import 'browser_detection.dart';
3738
/// used carefully and only on types that are known to not contains `JSNull` and
3839
/// `JSUndefined`.
3940
extension ObjectToJSAnyExtension on Object {
41+
// Once `Object.toJSBox` is faster (see
42+
// https://github.com/dart-lang/sdk/issues/55183) we can remove this
43+
// backend-specific workaround.
44+
@pragma('wasm:prefer-inline')
45+
@pragma('dart2js:tryInline')
46+
JSAny get toJSWrapper => dartToJsWrapper(this);
47+
48+
@pragma('wasm:prefer-inline')
49+
@pragma('dart2js:tryInline')
4050
JSAny get toJSAnyShallow {
4151
if (isWasm) {
4252
return toJSAnyDeep;
@@ -45,10 +55,18 @@ extension ObjectToJSAnyExtension on Object {
4555
}
4656
}
4757

58+
@pragma('wasm:prefer-inline')
59+
@pragma('dart2js:tryInline')
4860
JSAny get toJSAnyDeep => js_util.jsify(this) as JSAny;
4961
}
5062

5163
extension JSAnyToObjectExtension on JSAny {
64+
@pragma('wasm:prefer-inline')
65+
@pragma('dart2js:tryInline')
66+
Object get fromJSWrapper => jsWrapperToDart(this);
67+
68+
@pragma('wasm:prefer-inline')
69+
@pragma('dart2js:tryInline')
5270
Object get toObjectShallow {
5371
if (isWasm) {
5472
return toObjectDeep;
@@ -57,6 +75,8 @@ extension JSAnyToObjectExtension on JSAny {
5775
}
5876
}
5977

78+
@pragma('wasm:prefer-inline')
79+
@pragma('dart2js:tryInline')
6080
Object get toObjectDeep => js_util.dartify(this)!;
6181
}
6282

@@ -3648,38 +3668,24 @@ extension DomTextDecoderExtension on DomTextDecoder {
36483668

36493669
@JS('window.FinalizationRegistry')
36503670
@staticInterop
3651-
class DomFinalizationRegistry {}
3652-
3653-
@JS('window.FinalizationRegistry')
3654-
external JSAny? get _finalizationRegistryConstructor;
3655-
3656-
// Note: We don't use a factory constructor here because there is an issue in
3657-
// dart2js that causes a crash in the Google3 build if we do use a factory
3658-
// constructor. See b/284478971
3659-
DomFinalizationRegistry createDomFinalizationRegistry(JSFunction cleanup) =>
3660-
js_util.callConstructor(
3661-
_finalizationRegistryConstructor!.toObjectShallow, <Object>[cleanup]);
3671+
class DomFinalizationRegistry {
3672+
external factory DomFinalizationRegistry(JSFunction cleanup);
3673+
}
36623674

36633675
extension DomFinalizationRegistryExtension on DomFinalizationRegistry {
36643676
@JS('register')
3665-
external JSVoid _register1(JSAny target, JSAny value);
3677+
external JSVoid register(JSAny target, JSAny value);
36663678

36673679
@JS('register')
3668-
external JSVoid _register2(JSAny target, JSAny value, JSAny token);
3669-
void register(Object target, Object value, [Object? token]) {
3670-
if (token != null) {
3671-
_register2(
3672-
target.toJSAnyShallow, value.toJSAnyShallow, token.toJSAnyShallow);
3673-
} else {
3674-
_register1(target.toJSAnyShallow, value.toJSAnyShallow);
3675-
}
3676-
}
3680+
external JSVoid registerWithToken(JSAny target, JSAny value, JSAny token);
36773681

36783682
@JS('unregister')
3679-
external JSVoid _unregister(JSAny token);
3680-
void unregister(Object token) => _unregister(token.toJSAnyShallow);
3683+
external JSVoid unregister(JSAny token);
36813684
}
36823685

3686+
@JS('window.FinalizationRegistry')
3687+
external JSAny? get _finalizationRegistryConstructor;
3688+
36833689
/// Whether the current browser supports `FinalizationRegistry`.
36843690
bool browserSupportsFinalizationRegistry =
36853691
_finalizationRegistryConstructor != null;

lib/web_ui/lib/src/engine/skwasm/skwasm_impl.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import 'dart:ffi';
1111

1212
export 'skwasm_impl/canvas.dart';
1313
export 'skwasm_impl/codecs.dart';
14+
export 'skwasm_impl/dart_js_conversion.dart';
1415
export 'skwasm_impl/filters.dart';
1516
export 'skwasm_impl/font_collection.dart';
1617
export 'skwasm_impl/image.dart';
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
// Copyright 2013 The Flutter Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
import 'dart:_wasm';
6+
import 'dart:js_interop';
7+
8+
@pragma('wasm:prefer-inline')
9+
@pragma('dart2js:tryInline')
10+
JSAny dartToJsWrapper(Object object) =>
11+
WasmAnyRef.fromObject(object).externalize().toJS;
12+
13+
@pragma('wasm:prefer-inline')
14+
@pragma('dart2js:tryInline')
15+
Object jsWrapperToDart(JSAny jsWrapper) =>
16+
externRefForJSAny(jsWrapper).internalize()!.toObject();

lib/web_ui/lib/src/engine/skwasm/skwasm_impl/memory.dart

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,19 +28,22 @@ typedef DisposeFunction<T extends NativeType> = void Function(Pointer<T>);
2828

2929
class SkwasmFinalizationRegistry<T extends NativeType> {
3030
SkwasmFinalizationRegistry(this.dispose)
31-
: registry = createDomFinalizationRegistry(((JSNumber address) =>
31+
: registry = DomFinalizationRegistry(((JSNumber address) =>
3232
dispose(Pointer<T>.fromAddress(address.toDartDouble.toInt()))
3333
).toJS);
3434

3535
final DomFinalizationRegistry registry;
3636
final DisposeFunction<T> dispose;
3737

3838
void register(SkwasmObjectWrapper<T> wrapper) {
39-
registry.register(wrapper, wrapper.handle.address, wrapper);
39+
final JSAny jsWrapper = wrapper.toJSWrapper;
40+
registry.registerWithToken(
41+
jsWrapper, wrapper.handle.address.toJS, jsWrapper);
4042
}
4143

4244
void evict(SkwasmObjectWrapper<T> wrapper) {
43-
registry.unregister(wrapper);
45+
final JSAny jsWrapper = wrapper.toJSWrapper;
46+
registry.unregister(jsWrapper);
4447
dispose(wrapper.handle);
4548
}
4649
}

lib/web_ui/lib/src/engine/skwasm/skwasm_stub.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,4 +6,5 @@
66
// ignore: unnecessary_library_directive
77
library skwasm_stub;
88

9+
export 'skwasm_stub/dart_js_conversion.dart';
910
export 'skwasm_stub/renderer.dart';
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// Copyright 2013 The Flutter Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
import 'dart:js_interop';
6+
7+
@pragma('wasm:prefer-inline')
8+
@pragma('dart2js:tryInline')
9+
JSAny dartToJsWrapper(Object object) => object as JSAny;
10+
11+
@pragma('wasm:prefer-inline')
12+
@pragma('dart2js:tryInline')
13+
Object jsWrapperToDart(JSAny jsWrapper) => jsWrapper;

lib/web_ui/test/canvaskit/canvaskit_api_test.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1928,8 +1928,8 @@ void _paragraphTests() {
19281928
// FinalizationRegistry because it depends on GC, which cannot be controlled,
19291929
// So the test simply tests that a FinalizationRegistry can be constructed
19301930
// and its `register` method can be called.
1931-
final DomFinalizationRegistry registry = createDomFinalizationRegistry((String arg) {}.toJS);
1932-
registry.register(Object(), Object());
1931+
final DomFinalizationRegistry registry = DomFinalizationRegistry((String arg) {}.toJS);
1932+
registry.register(Object().toJSWrapper, Object().toJSWrapper);
19331933
});
19341934
}
19351935

0 commit comments

Comments
 (0)