[web:canvaskit] switch to temporary SkPaint objects

[yjbanov]

Do not eagerly create an SkPaint object that's strongly referenced by CkPaint. Instead, when a Canvas.draw* is called, create a temporary SkPaint object, pass it to Skia, then immediately delete it. This way there are no persistent SkPaint handles lurking in the system that transitively hold onto expensive native resources.

Addresses the Paint issue in flutter/flutter#153678 in CanvasKit

Spot checking some benchmarks. Here's the effect of this PR on draw_rect_variable_paint. It's a bit of a stress test as it creates 300K distinct Paint objects to render 600 pictures (a typical ratio does not normally exceed ten paints to one picture). Even so, the effect of creating an extra SkPaint on every draw* command does not look significant. However, removing a dependency on the GC for 300K objects looks like a good trade-off.

Before

Allocation stats:

  Paint Created: 300000
  Paint Deleted: 300000
  Paint Leaked: 300000
  Picture Created: 600
  Picture Deleted: 599
  Picture Leaked: 599

Performance stats:

windowRenderDuration: (samples: 98 clean/2 outliers/100 measured/300 total)
 | average: 4679.551020408163 μs
 | outlier average: 5100 μs
 | outlier/clean ratio: 1.0898481452084188x
 | noise: 3.11%

sceneBuildDuration: (samples: 98 clean/2 outliers/100 measured/300 total)
 | average: 4689.765306122449 μs
 | outlier average: 5100 μs
 | outlier/clean ratio: 1.087474461321549x
 | noise: 3.19%

drawFrameDuration: (samples: 97 clean/3 outliers/100 measured/300 total)
 | average: 8447.474226804125 μs
 | outlier average: 9332.666666666666 μs
 | outlier/clean ratio: 1.1047878236850721x
 | noise: 3.52%

After

Allocation stats:

  Picture Created: 600
  Picture Deleted: 599
  Picture Leaked: 599

Performance stats:

windowRenderDuration: (samples: 97 clean/3 outliers/100 measured/300 total)
 | average: 4780.40206185567 μs
 | outlier average: 5133.666666666667 μs
 | outlier/clean ratio: 1.0738985131877936x
 | noise: 2.70%

sceneBuildDuration: (samples: 97 clean/3 outliers/100 measured/300 total)
 | average: 4787.6082474226805 μs
 | outlier average: 5133.666666666667 μs
 | outlier/clean ratio: 1.0722821085936345x
 | noise: 2.72%

drawFrameDuration: (samples: 97 clean/3 outliers/100 measured/300 total)
 | average: 8243.309278350516 μs
 | outlier average: 9033.333333333334 μs
 | outlier/clean ratio: 1.0958382159768851x
 | noise: 2.60%

Follow-up

A few benchmarks regressed: recordPaintCommands, draw_rect (with static Paint), and clipped_out_pictures. The first two were expected, since I'm literally moving the work of creating SkPaint to the draw* invocations. clipped_out_pictures uses a static Paint object, so again the work to create SkPaint moved into the measured section of the benchmark.

Of course, there's a real trade-off in that if the same Paint object is heavily reused we repeatedly pay the cost of reallocating SkPaint. This does not seem to be noticeable in real-world situations though. None of our macrobenchmarks that use real widgets (e.g. Material 3 benchmarks) felt the change.

Overall, the trade-off seems to be worth it for two reasons:

• It does help keep memory usage lower.
• It is consistent with the mobile engine. Developers expect memory usage characteristics to be consistent no matter which platform they run the app on.

That said, perhaps there's room for improvement. For example, we could use stack-based allocation for the temporary objects, which is something Skwasm can do (I'll test it when I make the same change in Skwasm). We could also introduce an arena for all frame-scoped native objects. This would improve over stack-based allocation in that the same object can be reused in multiple places in the same frame, and cleaned up after the frame is done, perhaps even asynchronously to let the frame present to screen without waiting for the clean-up.

[yjbanov]

I removed this comment because all the setters are cheap now.

[eyebrowsoffire]

Had some driveby comments.

[eyebrowsoffire]

Perhaps it would be more logical to do this at the point of converstion, rather than in the setter. This happening in the setter always felt kind of weird to me anyway...

[yjbanov]

Good catch! The actual conversion to SkImageFilter happens in CkManagedSkImageFilterConvertible.imageFilter used in toSkPaint() above. But now that I read the various implementations of CkManagedSkImageFilterConvertible.imageFilter, they all use UniqueRef, so they are subject to GC. I'm going to have to fix that.

[yjbanov]

Done.

[eyebrowsoffire]

Since we will have to do something similar for skwasm, do you think it might be worthwhile to pull this out of the canvaskit-specific code and instead just make it a renderer-agnostic class that I could reuse in skwasm? And perhaps just making an extension method for toSkPaint or whatever?

[yjbanov]

Not a bad idea! I'm not going to do it in this PR, since CkPaint has direct dependencies on canvaskit_api.dart and many other Ck* classes. But it can be done as a follow-up when SkwasmPaint moves to this model. I filed flutter/flutter#154281 and will leave a TODO.

[flutter-dashboard]

Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change).

If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review.

Changes reported for pull request #54818 at sha 780c6fc

[eyebrowsoffire]

LGTM!

[flutter-dashboard]

Golden file changes are available for triage from new commit, Click here to view.

Changes reported for pull request #54818 at sha ae5bddf