[Impeller] Reland: one descriptor pool per frame. #55960
Conversation
jonahwilliams
added
the
test: all
jonahwilliams
removed
the
test: all
|
This pull request has been changed to a draft. The currently pending flutter-gold status will not be able to resolve until a new commit is pushed or the change is marked ready for review again. |
| std::weak_ptr<const Context> context, | ||
| std::weak_ptr<const DeviceHolderVK> device_holder, | ||
| std::shared_ptr<TrackedObjectsVK> tracked_objects, | ||
| std::shared_ptr<FenceWaiterVK> fence_waiter) |
There was a problem hiding this comment.
fence waiter is unused
chinmaygarde
left a comment
chinmaygarde
left a comment
There was a problem hiding this comment.
Some suggestions but this should work well.
Don't forget to back out the CI stuff.
.ci.yaml
Outdated
| - shell/platform/android/** | ||
| - testing/scenario_app/** | ||
| - testing/skia_gold_client/** | ||
| # runIf: |
There was a problem hiding this comment.
Presumably, this is unrelated and will be backed out?
| return nullptr; | ||
| } | ||
|
|
||
| // look up a cached descriptor pool for the current frame and reuse it |
There was a problem hiding this comment.
In cases where we operate on a thread pool where threads are being created and collected, thread ID reuse may cause us issues. This is just a hypothetical though. Our existing pools don't work that way. Not sure we should care.
There was a problem hiding this comment.
Thinking about this some more, perhaps we should steal NSNotificationCenter from Foundation. The thread locals can add listeners (by object) to certain lifecycle events where they can collect resources when the context shuts down in addition to collection those same resources when the thread itself shuts down. In any case, what you have here should work well for our use.
There was a problem hiding this comment.
Can you file a bug for this and describe how it would work?
jonahwilliams
added
the
autosubmit
…157295) flutter/engine@de569ba...5eb21d2 2024-10-21 [email protected] [Impeller] Reland: one descriptor pool per frame. (flutter/engine#55960) 2024-10-21 [email protected] Scribe (Android stylus handwriting text input) (flutter/engine#52943) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
|
And also: |
reverted flutter/engine#55939 due to postsubmit failures. It turns out the lifecycle is not exactly the same as the command pool. Instead, use a thread id map on the context to keep the right thread running and simplify shutdown. Leaving CI yaml edit for now to prove it works :) Fixes flutter#157115
2 participants
reverted #55939 due to postsubmit failures.
It turns out the lifecycle is not exactly the same as the command pool. Instead, use a thread id map on the context to keep the right thread running and simplify shutdown. Leaving CI yaml edit for now to prove it works :)
Fixes flutter/flutter#157115