-
Notifications
You must be signed in to change notification settings - Fork 6k
[Impeller] Perform integrity checks for Vulkan pipeline caches. #54654
Conversation
Fixes flutter/flutter#128126 I scared myself looking into the recent Vulkan driver issues and decided to fix this to follow best-practices. In addition to the comments, see the linked issue for the article on how this works. I didn't perform the data hashing because we use ::rename in fml::WriteAtomically and I am not as concerned about that. But we can add it later if needed. We also don't have a good utility to has data. This also gets rid of one intermediate allocation. We could also write directly into the file mapping but FML has no utilities to msync. Something to fix later if needed.
This comment was marked as outdated.
This comment was marked as outdated.
// https://medium.com/@zeuxcg/creating-a-robust-pipeline-cache-with-vulkan-961d09416cda. | ||
struct PipelineCacheHeader { | ||
// This can be used by Impeller to manually invalidate all old caches. | ||
uint32_t magic = 0xC0DEF00D; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jtmcdole this is almost you
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lol yeah! The other one I had in mind was 0xFA51F00D
(fast-food for our pipelines) but the T
didn't read right.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
General approach seems good, but we should have unit tests that sanity check the logic such that we can be sure we don't continually thrash the cash on app restart, among other things.
/// directory. This function performs integrity checks the Vulkan | ||
/// driver may have missed. | ||
/// | ||
/// @warning The pipeline cache must be externally synchronized for most |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left this in warning in there because it is theoretically possible if we are still in the middle of constructing pipelines. The failure mode is not fatal though and we only persist after a set number of frames. So I'm not too worried about this.
This patch changes what we persist, not when. We discussed the latter a bit in I'll add some tests for the header checks though. |
Right, its the header checks I'd be concerned about. |
okie dokie. Tests are done. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for adding the tests!
impeller/renderer/backend/vulkan/pipeline_cache_data_vk_unittests.cc
Outdated
Show resolved
Hide resolved
} | ||
|
||
auto allocation_mapping = CreateMappingFromAllocation(allocation); | ||
if (!allocation_mapping) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be nice if we had a unique_ptr variant for when we don't need shared ownership like here. Not blocking.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once we get concepts, I'd like to apply one for any pointer like thing that can be moved. So it would work for both.
…153884) flutter/engine@b94e009...f493c21 2024-08-21 [email protected] [iOS] Tweak note about OpenGL support on mac in a user facing log. (flutter/engine#54690) 2024-08-21 [email protected] [Impeller] use blit pass to resize decoded images. (flutter/engine#54606) 2024-08-21 [email protected] Remove spammy warning message on `FlutterView` (flutter/engine#54686) 2024-08-21 [email protected] [Impeller] Perform integrity checks for Vulkan pipeline caches. (flutter/engine#54654) 2024-08-21 [email protected] docs: use test: all rather than editing .ci.yaml (flutter/engine#54667) 2024-08-21 [email protected] Reland "[DisplayList] Allow random access to ops through indexing" (flutter/engine#54676) 2024-08-21 [email protected] Roll Skia from 51ac9d93850c to 249d3f07c4d5 (2 revisions) (flutter/engine#54684) 2024-08-21 [email protected] iOS,macOS: Don't archive extra framework metadata (flutter/engine#54674) 2024-08-21 [email protected] Roll Dart SDK from 48f9b96d71e7 to 060e40916514 (1 revision) (flutter/engine#54682) 2024-08-21 [email protected] [web] annotate obscured text fields as passwords (flutter/engine#54664) 2024-08-21 [email protected] Roll Skia from c31e2ca59bd9 to 51ac9d93850c (2 revisions) (flutter/engine#54681) 2024-08-21 [email protected] [engine] reland weaken affinity of raster/ui to non-e core instead of only fast core (flutter/engine#54616) 2024-08-21 [email protected] Roll Skia from c00866df101a to c31e2ca59bd9 (2 revisions) (flutter/engine#54680) 2024-08-21 [email protected] Roll Skia from 39e5118034f4 to c00866df101a (1 revision) (flutter/engine#54678) 2024-08-21 [email protected] Roll Skia from 221ada80b174 to 39e5118034f4 (1 revision) (flutter/engine#54677) 2024-08-21 [email protected] Roll Skia from d576296091e0 to 221ada80b174 (2 revisions) (flutter/engine#54675) 2024-08-21 [email protected] Roll Dart SDK from 49f655b526c7 to 48f9b96d71e7 (1 revision) (flutter/engine#54672) 2024-08-21 [email protected] Roll Fuchsia Linux SDK from 3a16kOsyFmJh3lo7e... to XGzE3idakwfQZ68pb... (flutter/engine#54671) Also rolling transitive DEPS: fuchsia/sdk/core/linux-amd64 from 3a16kOsyFmJh to XGzE3idakwfQ 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],[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
…visions) (#153884)" (#153899) Reverts: #153884 Initiated by: jason-simmons Reason for reverting: flutter/engine@f8d553a introduced a log message that is not expected by the `run_debug_test_android` devicelab test Reverting until an engine build is available with the log removed (see flutter/engine#54699) Original PR Author: engine-flutter-autoroll Reviewed By: {fluttergithubbot} This change reverts the following previous change: flutter/engine@b94e009...f493c21 2024-08-21 [email protected] [iOS] Tweak note about OpenGL support on mac in a user facing log. (flutter/engine#54690) 2024-08-21 [email protected] [Impeller] use blit pass to resize decoded images. (flutter/engine#54606) 2024-08-21 [email protected] Remove spammy warning message on `FlutterView` (flutter/engine#54686) 2024-08-21 [email protected] [Impeller] Perform integrity checks for Vulkan pipeline caches. (flutter/engine#54654) 2024-08-21 [email protected] docs: use test: all rather than editing .ci.yaml (flutter/engine#54667) 2024-08-21 [email protected] Reland "[DisplayList] Allow random access to ops through indexing" (flutter/engine#54676) 2024-08-21 [email protected] Roll Skia from 51ac9d93850c to 249d3f07c4d5 (2 revisions) (flutter/engine#54684) 2024-08-21 [email protected] iOS,macOS: Don't archive extra framework metadata (flutter/engine#54674) 2024-08-21 [email protected] Roll Dart SDK from 48f9b96d71e7 to 060e40916514 (1 revision) (flutter/engine#54682) 2024-08-21 [email protected] [web] annotate obscured text fields as passwords (flutter/engine#54664) 2024-08-21 [email protected] Roll Skia from c31e2ca59bd9 to 51ac9d93850c (2 revisions) (flutter/engine#54681) 2024-08-21 [email protected] [engine] reland weaken affinity of raster/ui to non-e core instead of only fast core (flutter/engine#54616) 2024-08-21 [email protected] Roll Skia from c00866df101a to c31e2ca59bd9 (2 revisions) (flutter/engine#54680) 2024-08-21 [email protected] Roll Skia from 39e5118034f4 to c00866df101a (1 revision) (flutter/engine#54678) 2024-08-21 [email protected] Roll Skia from 221ada80b174 to 39e5118034f4 (1 revision) (flutter/engine#54677) 2024-08-21 [email protected] Roll Skia from d576296091e0 to 221ada80b174 (2 revisions) (flutter/engine#54675) 2024-08-21 [email protected] Roll Dart SDK from 49f655b526c7 to 48f9b96d71e7 (1 revision) (flutter/engine#54672) 2024-08-21 [email protected] Roll Fuchsia Linux SDK from 3a16kOsyFmJh3lo7e... to XGzE3idakwfQZ68pb... (flutter/engine#54671) Also rolling transitive DEPS: fuchsia/sdk/core/linux-amd64 from 3a16kOsyFmJh to XGzE3idakwfQ 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],[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
…153902) flutter/engine@b94e009...85d4be0 2024-08-22 [email protected] Export GPU symbols for embedder (flutter/engine#54662) 2024-08-22 [email protected] [Impeller] Remove a log message in the Vulkan back end that is visible during engine startup (flutter/engine#54699) 2024-08-22 [email protected] Roll Skia from 3cd00377cefc to 34aa8ce13af6 (3 revisions) (flutter/engine#54698) 2024-08-21 [email protected] macOS: Make framework creation consistent with iOS (flutter/engine#54685) 2024-08-21 [email protected] Roll Skia from 69f4bd859025 to 3cd00377cefc (8 revisions) (flutter/engine#54693) 2024-08-21 [email protected] Roll Dart SDK from 060e40916514 to 025bf8d376d3 (1 revision) (flutter/engine#54692) 2024-08-21 [email protected] Split tests out of Linux Android artifact creation builds (flutter/engine#54683) 2024-08-21 [email protected] Roll Skia from 249d3f07c4d5 to 69f4bd859025 (5 revisions) (flutter/engine#54691) 2024-08-21 [email protected] [iOS] Tweak note about OpenGL support on mac in a user facing log. (flutter/engine#54690) 2024-08-21 [email protected] [Impeller] use blit pass to resize decoded images. (flutter/engine#54606) 2024-08-21 [email protected] Remove spammy warning message on `FlutterView` (flutter/engine#54686) 2024-08-21 [email protected] [Impeller] Perform integrity checks for Vulkan pipeline caches. (flutter/engine#54654) 2024-08-21 [email protected] docs: use test: all rather than editing .ci.yaml (flutter/engine#54667) 2024-08-21 [email protected] Reland "[DisplayList] Allow random access to ops through indexing" (flutter/engine#54676) 2024-08-21 [email protected] Roll Skia from 51ac9d93850c to 249d3f07c4d5 (2 revisions) (flutter/engine#54684) 2024-08-21 [email protected] iOS,macOS: Don't archive extra framework metadata (flutter/engine#54674) 2024-08-21 [email protected] Roll Dart SDK from 48f9b96d71e7 to 060e40916514 (1 revision) (flutter/engine#54682) 2024-08-21 [email protected] [web] annotate obscured text fields as passwords (flutter/engine#54664) 2024-08-21 [email protected] Roll Skia from c31e2ca59bd9 to 51ac9d93850c (2 revisions) (flutter/engine#54681) 2024-08-21 [email protected] [engine] reland weaken affinity of raster/ui to non-e core instead of only fast core (flutter/engine#54616) 2024-08-21 [email protected] Roll Skia from c00866df101a to c31e2ca59bd9 (2 revisions) (flutter/engine#54680) 2024-08-21 [email protected] Roll Skia from 39e5118034f4 to c00866df101a (1 revision) (flutter/engine#54678) 2024-08-21 [email protected] Roll Skia from 221ada80b174 to 39e5118034f4 (1 revision) (flutter/engine#54677) 2024-08-21 [email protected] Roll Skia from d576296091e0 to 221ada80b174 (2 revisions) (flutter/engine#54675) 2024-08-21 [email protected] Roll Dart SDK from 49f655b526c7 to 48f9b96d71e7 (1 revision) (flutter/engine#54672) 2024-08-21 [email protected] Roll Fuchsia Linux SDK from 3a16kOsyFmJh3lo7e... to XGzE3idakwfQZ68pb... (flutter/engine#54671) Also rolling transitive DEPS: fuchsia/sdk/core/linux-amd64 from 3a16kOsyFmJh to XGzE3idakwfQ 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],[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
…lutter#153884) flutter/engine@b94e009...f493c21 2024-08-21 [email protected] [iOS] Tweak note about OpenGL support on mac in a user facing log. (flutter/engine#54690) 2024-08-21 [email protected] [Impeller] use blit pass to resize decoded images. (flutter/engine#54606) 2024-08-21 [email protected] Remove spammy warning message on `FlutterView` (flutter/engine#54686) 2024-08-21 [email protected] [Impeller] Perform integrity checks for Vulkan pipeline caches. (flutter/engine#54654) 2024-08-21 [email protected] docs: use test: all rather than editing .ci.yaml (flutter/engine#54667) 2024-08-21 [email protected] Reland "[DisplayList] Allow random access to ops through indexing" (flutter/engine#54676) 2024-08-21 [email protected] Roll Skia from 51ac9d93850c to 249d3f07c4d5 (2 revisions) (flutter/engine#54684) 2024-08-21 [email protected] iOS,macOS: Don't archive extra framework metadata (flutter/engine#54674) 2024-08-21 [email protected] Roll Dart SDK from 48f9b96d71e7 to 060e40916514 (1 revision) (flutter/engine#54682) 2024-08-21 [email protected] [web] annotate obscured text fields as passwords (flutter/engine#54664) 2024-08-21 [email protected] Roll Skia from c31e2ca59bd9 to 51ac9d93850c (2 revisions) (flutter/engine#54681) 2024-08-21 [email protected] [engine] reland weaken affinity of raster/ui to non-e core instead of only fast core (flutter/engine#54616) 2024-08-21 [email protected] Roll Skia from c00866df101a to c31e2ca59bd9 (2 revisions) (flutter/engine#54680) 2024-08-21 [email protected] Roll Skia from 39e5118034f4 to c00866df101a (1 revision) (flutter/engine#54678) 2024-08-21 [email protected] Roll Skia from 221ada80b174 to 39e5118034f4 (1 revision) (flutter/engine#54677) 2024-08-21 [email protected] Roll Skia from d576296091e0 to 221ada80b174 (2 revisions) (flutter/engine#54675) 2024-08-21 [email protected] Roll Dart SDK from 49f655b526c7 to 48f9b96d71e7 (1 revision) (flutter/engine#54672) 2024-08-21 [email protected] Roll Fuchsia Linux SDK from 3a16kOsyFmJh3lo7e... to XGzE3idakwfQZ68pb... (flutter/engine#54671) Also rolling transitive DEPS: fuchsia/sdk/core/linux-amd64 from 3a16kOsyFmJh to XGzE3idakwfQ 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],[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
…visions) (flutter#153884)" (flutter#153899) Reverts: flutter#153884 Initiated by: jason-simmons Reason for reverting: flutter/engine@f8d553a introduced a log message that is not expected by the `run_debug_test_android` devicelab test Reverting until an engine build is available with the log removed (see flutter/engine#54699) Original PR Author: engine-flutter-autoroll Reviewed By: {fluttergithubbot} This change reverts the following previous change: flutter/engine@b94e009...f493c21 2024-08-21 [email protected] [iOS] Tweak note about OpenGL support on mac in a user facing log. (flutter/engine#54690) 2024-08-21 [email protected] [Impeller] use blit pass to resize decoded images. (flutter/engine#54606) 2024-08-21 [email protected] Remove spammy warning message on `FlutterView` (flutter/engine#54686) 2024-08-21 [email protected] [Impeller] Perform integrity checks for Vulkan pipeline caches. (flutter/engine#54654) 2024-08-21 [email protected] docs: use test: all rather than editing .ci.yaml (flutter/engine#54667) 2024-08-21 [email protected] Reland "[DisplayList] Allow random access to ops through indexing" (flutter/engine#54676) 2024-08-21 [email protected] Roll Skia from 51ac9d93850c to 249d3f07c4d5 (2 revisions) (flutter/engine#54684) 2024-08-21 [email protected] iOS,macOS: Don't archive extra framework metadata (flutter/engine#54674) 2024-08-21 [email protected] Roll Dart SDK from 48f9b96d71e7 to 060e40916514 (1 revision) (flutter/engine#54682) 2024-08-21 [email protected] [web] annotate obscured text fields as passwords (flutter/engine#54664) 2024-08-21 [email protected] Roll Skia from c31e2ca59bd9 to 51ac9d93850c (2 revisions) (flutter/engine#54681) 2024-08-21 [email protected] [engine] reland weaken affinity of raster/ui to non-e core instead of only fast core (flutter/engine#54616) 2024-08-21 [email protected] Roll Skia from c00866df101a to c31e2ca59bd9 (2 revisions) (flutter/engine#54680) 2024-08-21 [email protected] Roll Skia from 39e5118034f4 to c00866df101a (1 revision) (flutter/engine#54678) 2024-08-21 [email protected] Roll Skia from 221ada80b174 to 39e5118034f4 (1 revision) (flutter/engine#54677) 2024-08-21 [email protected] Roll Skia from d576296091e0 to 221ada80b174 (2 revisions) (flutter/engine#54675) 2024-08-21 [email protected] Roll Dart SDK from 49f655b526c7 to 48f9b96d71e7 (1 revision) (flutter/engine#54672) 2024-08-21 [email protected] Roll Fuchsia Linux SDK from 3a16kOsyFmJh3lo7e... to XGzE3idakwfQZ68pb... (flutter/engine#54671) Also rolling transitive DEPS: fuchsia/sdk/core/linux-amd64 from 3a16kOsyFmJh to XGzE3idakwfQ 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],[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
…lutter#153902) flutter/engine@b94e009...85d4be0 2024-08-22 [email protected] Export GPU symbols for embedder (flutter/engine#54662) 2024-08-22 [email protected] [Impeller] Remove a log message in the Vulkan back end that is visible during engine startup (flutter/engine#54699) 2024-08-22 [email protected] Roll Skia from 3cd00377cefc to 34aa8ce13af6 (3 revisions) (flutter/engine#54698) 2024-08-21 [email protected] macOS: Make framework creation consistent with iOS (flutter/engine#54685) 2024-08-21 [email protected] Roll Skia from 69f4bd859025 to 3cd00377cefc (8 revisions) (flutter/engine#54693) 2024-08-21 [email protected] Roll Dart SDK from 060e40916514 to 025bf8d376d3 (1 revision) (flutter/engine#54692) 2024-08-21 [email protected] Split tests out of Linux Android artifact creation builds (flutter/engine#54683) 2024-08-21 [email protected] Roll Skia from 249d3f07c4d5 to 69f4bd859025 (5 revisions) (flutter/engine#54691) 2024-08-21 [email protected] [iOS] Tweak note about OpenGL support on mac in a user facing log. (flutter/engine#54690) 2024-08-21 [email protected] [Impeller] use blit pass to resize decoded images. (flutter/engine#54606) 2024-08-21 [email protected] Remove spammy warning message on `FlutterView` (flutter/engine#54686) 2024-08-21 [email protected] [Impeller] Perform integrity checks for Vulkan pipeline caches. (flutter/engine#54654) 2024-08-21 [email protected] docs: use test: all rather than editing .ci.yaml (flutter/engine#54667) 2024-08-21 [email protected] Reland "[DisplayList] Allow random access to ops through indexing" (flutter/engine#54676) 2024-08-21 [email protected] Roll Skia from 51ac9d93850c to 249d3f07c4d5 (2 revisions) (flutter/engine#54684) 2024-08-21 [email protected] iOS,macOS: Don't archive extra framework metadata (flutter/engine#54674) 2024-08-21 [email protected] Roll Dart SDK from 48f9b96d71e7 to 060e40916514 (1 revision) (flutter/engine#54682) 2024-08-21 [email protected] [web] annotate obscured text fields as passwords (flutter/engine#54664) 2024-08-21 [email protected] Roll Skia from c31e2ca59bd9 to 51ac9d93850c (2 revisions) (flutter/engine#54681) 2024-08-21 [email protected] [engine] reland weaken affinity of raster/ui to non-e core instead of only fast core (flutter/engine#54616) 2024-08-21 [email protected] Roll Skia from c00866df101a to c31e2ca59bd9 (2 revisions) (flutter/engine#54680) 2024-08-21 [email protected] Roll Skia from 39e5118034f4 to c00866df101a (1 revision) (flutter/engine#54678) 2024-08-21 [email protected] Roll Skia from 221ada80b174 to 39e5118034f4 (1 revision) (flutter/engine#54677) 2024-08-21 [email protected] Roll Skia from d576296091e0 to 221ada80b174 (2 revisions) (flutter/engine#54675) 2024-08-21 [email protected] Roll Dart SDK from 49f655b526c7 to 48f9b96d71e7 (1 revision) (flutter/engine#54672) 2024-08-21 [email protected] Roll Fuchsia Linux SDK from 3a16kOsyFmJh3lo7e... to XGzE3idakwfQZ68pb... (flutter/engine#54671) Also rolling transitive DEPS: fuchsia/sdk/core/linux-amd64 from 3a16kOsyFmJh to XGzE3idakwfQ 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],[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
Fixes flutter/flutter#128126
I scared myself looking into the recent Vulkan driver issues and decided to fix this to follow best-practices.
In addition to the comments, see the linked issue for the article on how this works. I didn't perform the data hashing because we use ::rename in fml::WriteAtomically and I am not as concerned about that. But we can add it later if needed. We also don't have a good utility to hash data.
This also gets rid of one intermediate allocation. We could also write directly into the file mapping but FML has no utilities to msync. Something to fix later if needed.