Introduces several caching of objects across graphics Skiko code#3117
Introduces several caching of objects across graphics Skiko code#3117ApoloApps wants to merge 9 commits into
Conversation
… This aligns to Android's behaviour
…for indexed loop instead of generic Collection which allocates the iterator)
…ct on each frame (it reuses the same via CanvasHolder.drawInto). This aligns the behaviour with Android's. Uses the exact same pattern
…lections to avoid boxing, iterators and improve performance. (Similar change to PointerToPositionMap)
|
|
||
| private typealias PointerToPositionMap = LongLongMap | ||
|
|
||
| private typealias PointerIdSet = MutableLongSet |
There was a problem hiding this comment.
Non-boxing collection is way faster that HashSet especially in terms of contains operator, which is widely used across the file
WasmJs
| Operation | Size | HashSet (ops/μs) | LongSet (ops/μs) | Improvement (%) |
|---|---|---|---|---|
| Add | 16 | 4.803 | 5.266 | +9.64% |
| 256 | 0.381 | 0.401 | +5.25% | |
| 4096 | 0.023 | 0.025 | +8.70% | |
| Contains | 16 | 7.199 | 12.788 | +77.64% |
| 256 | 0.385 | 0.861 | +123.64% | |
| 4096 | 0.026 | 0.048 | +84.62% | |
| Iterate | 16 | 17.634 | 39.695 | +125.11% |
| 256 | 2.001 | 2.835 | +41.68% | |
| 4096 | 0.128 | 0.172 | +34.38% | |
| Remove | 16 | 2.806 | 3.177 | +13.22% |
| 256 | 0.174 | 0.251 | +44.25% | |
| 4096 | 0.013 | 0.016 | +23.08% |
Ivan Matkov (MatkovIvan)
left a comment
Ivan Matkov (MatkovIvan)
left a comment
There was a problem hiding this comment.
Thanks!
My main concern here is the shape of the newly introduced public API - it doesn't seem that it should be exposed like that for wide audience with full compatibility guarantees
| * Holder class that is used to issue scoped calls to a [Canvas] from the framework equivalent | ||
| * canvas without having to allocate an object on each draw call | ||
| */ | ||
| class CanvasHolder { |
There was a problem hiding this comment.
Note: a new public-stable API that we'll have to maintain forever
| internal val internalSkiaCanvas: SkCanvas, | ||
| ) : Canvas { | ||
| @PublishedApi | ||
| internal class SkiaBackedCanvas : Canvas { |
There was a problem hiding this comment.
It wasn't supposed to be exposed. Again, requirement for binary compatibility in a place where we're not ready to do it
|
I'll wait for the #3126 changes |
|
Separated non-graphics related changes to this PR #3164 to unblock merging some parts of this PR. After other PR merged I will be rebasing (as the other PR contains commits of this one) |
2 participants
Fixes https://youtrack.jetbrains.com/issue/CMP-9463/Web-asComposeCanvas-method-returns-object-allocated-on-each-rendered-frame (for all targets, not only web)
Release Notes
N/A