[web] remove obsolete object caches; simplify native object management #40617

yjbanov · 2023-03-25T01:29:49Z

Remove obsolete object caches and introduce a simpler way to manage native objects:

Remove the unused SynchronousSkiaObjectCache.
Introduce new library native_memory.dart that's smaller and simpler than skia_object_cache.dart.
Introduce two types of native object references:
- UniqueRef a reference with a unique Dart object owner.
- CountedRef a ref-counted reference with multiple Dart object owners.
All native references use GC (via FinalizationRegistry) as a back-up.
The new library removes everything related to object resurrection that was needed only in browsers that didn't support FinalizationRegistry. All browsers support it now.
Remove the ad hoc SkParagraph cache that predates the introduction of Paragraph.dispose.
Rewrite CkParagraph in terms of UniqueRef.
Rewrite CkImage in terms of CountedRef; delete SkiaObjectBox.

This PR does not migrate all objects from the old skia_object_cache.dart to native_memory.dart. That would be too big of a change. The migration can be done in multiple smaller PRs.

This also removes a few unnecessary relayouts observed in flutter/flutter#120921, but not all of them (more details in flutter/flutter#120921 (comment))

harryterkelsen

We should add a test for when the finalization registry disposes GC'ed objects to test the line I pointed out in my review

harryterkelsen · 2023-03-27T15:41:54Z

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

+    if (Instrumentation.enabled) {
+      Instrumentation.instance.incrementCounter('$_debugOwnerLabel Created');
+    }
+    _finalizationRegistry.register(owner, UniqueRef);


should this be finalizationRegistry.register(owner, this)?

yjbanov · 2023-03-28T00:49:19Z

We should add a test for when the finalization registry disposes GC'ed objects to test the line I pointed out in my review

Done. PTAL.

harryterkelsen

LGTM

… management (flutter/engine#40617)

…anagement (#40617)" This reverts commit c0140ec.

…anagement" (#40861) Reverts #40617 See b/276167870. This is causing a build breakage to Google testing for all web projects.

flutter-dashboard bot added the platform-web Code specifically for the web engine label Mar 25, 2023

yjbanov force-pushed the refs branch 2 times, most recently from 2c0acf7 to c3bafe0 Compare March 25, 2023 19:22

yjbanov marked this pull request as ready for review March 26, 2023 04:35

yjbanov force-pushed the refs branch from c3bafe0 to 9a02553 Compare March 26, 2023 23:36

yjbanov requested review from eyebrowsoffire and harryterkelsen March 27, 2023 00:20

harryterkelsen suggested changes Mar 27, 2023

View reviewed changes

yjbanov force-pushed the refs branch from 4ec857d to 3e71ae5 Compare March 28, 2023 00:47

yjbanov requested a review from harryterkelsen March 28, 2023 00:48

yjbanov force-pushed the refs branch from 3e71ae5 to 2d49289 Compare March 28, 2023 01:04

harryterkelsen approved these changes Mar 28, 2023

View reviewed changes

yjbanov force-pushed the refs branch from 2d49289 to c28b320 Compare March 28, 2023 19:31

mdebbar approved these changes Mar 28, 2023

View reviewed changes

remove obsolete object caches; simplify native object management

424f0dc

yjbanov force-pushed the refs branch from c28b320 to 424f0dc Compare March 28, 2023 20:32

make native_memory.dart dart2wasm compatible

28f3fe9

yjbanov added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 29, 2023

auto-submit bot merged commit c0140ec into flutter:main Mar 29, 2023

engine-flutter-autoroll mentioned this pull request Mar 29, 2023

Roll Flutter Engine from 1947b95c5060 to 0b4286cf415d (5 revisions) flutter/flutter#123670

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 29, 2023

c0140ec54 [web] remove obsolete object caches; simplify native object…

5726d05

… management (flutter/engine#40617)

engine-flutter-autoroll mentioned this pull request Mar 29, 2023

Roll Flutter Engine from 1947b95c5060 to 86d5521f552d (7 revisions) flutter/flutter#123672

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 29, 2023

c0140ec54 [web] remove obsolete object caches; simplify native object…

123702d

… management (flutter/engine#40617)

engine-flutter-autoroll mentioned this pull request Mar 29, 2023

Roll Flutter Engine from 1947b95c5060 to fa87a42fe941 (8 revisions) flutter/flutter#123676

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 29, 2023

c0140ec54 [web] remove obsolete object caches; simplify native object…

64dbf84

… management (flutter/engine#40617)

engine-flutter-autoroll mentioned this pull request Mar 29, 2023

Roll Flutter Engine from 1947b95c5060 to fa87a42fe941 (8 revisions) flutter/flutter#123678

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 29, 2023

c0140ec54 [web] remove obsolete object caches; simplify native object…

771e51d

… management (flutter/engine#40617)

engine-flutter-autoroll mentioned this pull request Mar 29, 2023

Roll Flutter Engine from 1947b95c5060 to fa87a42fe941 (8 revisions) flutter/flutter#123686

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 29, 2023

c0140ec54 [web] remove obsolete object caches; simplify native object…

f49910d

… management (flutter/engine#40617)

engine-flutter-autoroll mentioned this pull request Mar 29, 2023

Roll Flutter Engine from 1947b95c5060 to 510a10b52203 (9 revisions) flutter/flutter#123703

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 29, 2023

c0140ec54 [web] remove obsolete object caches; simplify native object…

86af7d2

… management (flutter/engine#40617)

engine-flutter-autoroll mentioned this pull request Mar 29, 2023

Roll Flutter Engine from 1947b95c5060 to 22f27ad8be9a (10 revisions) flutter/flutter#123710

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 29, 2023

c0140ec54 [web] remove obsolete object caches; simplify native object…

aa227f8

… management (flutter/engine#40617)

engine-flutter-autoroll mentioned this pull request Mar 29, 2023

Roll Flutter Engine from 1947b95c5060 to 1e737440c147 (11 revisions) flutter/flutter#123717

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 29, 2023

c0140ec54 [web] remove obsolete object caches; simplify native object…

1bcfad7

… management (flutter/engine#40617)

engine-flutter-autoroll mentioned this pull request Mar 29, 2023

Roll Flutter Engine from 1947b95c5060 to 83e22b5d69e5 (12 revisions) flutter/flutter#123718

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 29, 2023

c0140ec54 [web] remove obsolete object caches; simplify native object…

43ddd42

… management (flutter/engine#40617)

engine-flutter-autoroll mentioned this pull request Mar 29, 2023

Roll Flutter Engine from 1947b95c5060 to b2f142788838 (13 revisions) flutter/flutter#123720

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 29, 2023

c0140ec54 [web] remove obsolete object caches; simplify native object…

117320d

… management (flutter/engine#40617)

engine-flutter-autoroll mentioned this pull request Mar 29, 2023

Roll Flutter Engine from 1947b95c5060 to 45467e2a8b19 (15 revisions) flutter/flutter#123722

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 29, 2023

c0140ec54 [web] remove obsolete object caches; simplify native object…

1fdd17c

… management (flutter/engine#40617)

HansMuller mentioned this pull request Mar 31, 2023

[web:canvaskit] excessive text layout in the web engine on material 3 demo flutter/flutter#120921

Closed

CaseyHillers added a commit that referenced this pull request Apr 2, 2023

Revert "[web] remove obsolete object caches; simplify native object m…

bd2e84d

…anagement (#40617)" This reverts commit c0140ec.

CaseyHillers mentioned this pull request Apr 2, 2023

Revert "[web] remove obsolete object caches; simplify native object management" #40861

Merged

yjbanov pushed a commit that referenced this pull request Apr 2, 2023

Revert "[web] remove obsolete object caches; simplify native object m…

d62e1ad

…anagement" (#40861) Reverts #40617 See b/276167870. This is causing a build breakage to Google testing for all web projects.

yjbanov mentioned this pull request Apr 2, 2023

[web] remove obsolete object caches; simplify native object management #40862

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[web] remove obsolete object caches; simplify native object management #40617

[web] remove obsolete object caches; simplify native object management #40617

Uh oh!

yjbanov commented Mar 25, 2023 •

edited

Loading

Uh oh!

harryterkelsen left a comment

Uh oh!

harryterkelsen Mar 27, 2023

Uh oh!

yjbanov Mar 28, 2023

Uh oh!

yjbanov commented Mar 28, 2023

Uh oh!

harryterkelsen left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

[web] remove obsolete object caches; simplify native object management #40617

[web] remove obsolete object caches; simplify native object management #40617

Uh oh!

Conversation

yjbanov commented Mar 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

harryterkelsen left a comment

Choose a reason for hiding this comment

Uh oh!

harryterkelsen Mar 27, 2023

Choose a reason for hiding this comment

Uh oh!

yjbanov Mar 28, 2023

Choose a reason for hiding this comment

Uh oh!

yjbanov commented Mar 28, 2023

Uh oh!

harryterkelsen left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

yjbanov commented Mar 25, 2023 •

edited

Loading