Restore optimization to remove unused .so's from dist/, and add libvulkan.so etc explicitly to package. |
||||||||||
Issue descriptionWe need to be able to load the vulkan driver on fuchsia targets for accelerated graphics. Currently our SDK does not include libvulkan.so, so there's no way to load drivers.
,
Aug 3
I tried it and got the following error: [02946.317] dlsvc: could not open 'libvulkan_intel.so' [02946.340] dlsvc: could not open 'libtrace-engine.so' ERROR: Error loading shared library libtrace-engine.so: ZX_ERR_NOT_FOUND (needed by libvulkan_intel.so) [02946.342] dlsvc: could not open 'libvulkan_intel.so' [02946.365] dlsvc: could not open 'libtrace-engine.so' ERROR: Error loading shared library libtrace-engine.so: ZX_ERR_NOT_FOUND (needed by libvulkan_intel.so) [02946.366] dlsvc: could not open 'libvulkan_intel.so' [02946.389] dlsvc: could not open 'libtrace-engine.so' ERROR: Error loading shared library libtrace-engine.so: ZX_ERR_NOT_FOUND (needed by libvulkan_intel.so) ERROR: loader_validate_instance_extensions: Instance extension VK_KHR_surface not supported by available ICDs or enabled layers. [0803/222617.015171:ERROR:software_renderer.cc(37)] Failed to create software surface
,
Aug 3
The vulkan loader has a hack for loading libvulkan_intel.so, but it doesn't help for transitive dependencies of that object.
,
Aug 7
As I tried to explain in #1 (but probably not very clearly) your package will need to include libtrace-engine.so in its lib/ directory, along with the other shared libraries listed in that comment. wez@ - can you help spang@ get his package's libs stored out? I'm not sure exactly how to express this with Chrome's fuchsia packaging rules.
,
Aug 7
Assigning-to-self to follow up on tomorrow, and CC'ing kmarshall@, who is likely to be the ultimate owner. ;)
,
Aug 7
I tried this:
diff --git a/build/config/fuchsia/build_manifest.py b/build/config/fuchsia/build_manifest.py
index cb776faa0f79..b4ece91a1b71 100644
--- a/build/config/fuchsia/build_manifest.py
+++ b/build/config/fuchsia/build_manifest.py
@@ -44,7 +44,7 @@ def ReadDynamicLibDeps(paths):
if lib != 'libzircon.so':
libs.append(lib)
- return libs + ['libvulkan.so']
+ return libs + ['libvulkan.so', 'libtrace-engine.so', 'libc++.so.2', 'libc++abi.so.1', 'libunwind.so.1']
def ComputeTransitiveLibDeps(executable_path, available_libs):
and it's loading now.
,
Aug 7
To make this available in Chromium you'll want to wrap it up like e.g. the "svc" package (see https://cs.chromium.org/chromium/src/third_party/fuchsia-sdk/BUILD.gn?l=278) - that defines a package that Chromium targets can depend upon, and which build_manifest.py etc will automagically include the relevant .so's from.
,
Aug 7
,
Aug 7
Are you sure that will help? These aren't dependencies of chrome, they are dependencies of the vulkan internals. I think we'd have to link them with --no-as-needed. Is that possible with the SDK template?
,
Aug 7
Re #6: Creating a fuchsia_sdk_pkg() that adds the libraries will cause them to be specified on the link line of the executable, which should cause them to be NEEDED, which will cause build_manifest.py to include them. That should presumably work for libvulkan.so, since presumably the binary, or one of the Chromium component SOs, will depend on symbols from it - is the issue here that the libvulkan_intel.so is dynamically-loaded by libvulkan.so, such that there is no NEEDED relationship? We may be able to address that via |data_deps| on the fuchsia_sdk_pkg() rule, but right now build_manifest.py is limited to computing the dependencies of the executable identified by |app_filename| - if we need to explicitly pull in dynamic libraries then we'll need to extend it to also pull in the transitive dependencies of those. That said, are we really expecting every package to bundle Vulkan drivers for all the platforms it wants to support..?
,
Aug 7
,
Aug 7
Chrome does not directly link with either GL or vulkan. These graphics APIs are designed to be called using function pointers. Libraries needed by the driver especially won't end up in the list - Chrome loads libvulkan.so using dlopen, which in turn loads libvulkan_intel.so using dlopen. This breaks the dependency chain in two places. We can't find all the necessary libraries by closing over DT_NEEDED entries; these libraries need to be added to that list at the end because they are an indirect, unknowable dependencies (except by inspecting all of the drivers in the fuchsia tree, or asking James).
,
Aug 7
Here's another idea that works:
diff --git a/build/config/fuchsia/build_manifest.py b/build/config/fuchsia/build_manifest.py
index b4ece91a1b71..828fb3c466b1 100644
--- a/build/config/fuchsia/build_manifest.py
+++ b/build/config/fuchsia/build_manifest.py
@@ -44,7 +44,11 @@ def ReadDynamicLibDeps(paths):
if lib != 'libzircon.so':
libs.append(lib)
- return libs + ['libvulkan.so', 'libtrace-engine.so', 'libc++.so.2', 'libc++abi.so.1', 'libunwind.so.1']
+ # Add vulkan indirect dependencies
+ if lib == 'libvulkan.so':
+ libs += ['libtrace-engine.so', 'libc++.so.2', 'libc++abi.so.1', 'libunwind.so.1']
+
+ return libs
def ComputeTransitiveLibDeps(executable_path, available_libs):
diff --git a/third_party/fuchsia-sdk/BUILD.gn b/third_party/fuchsia-sdk/BUILD.gn
index 0cf8012a8e96..df0079bd5aa8 100644
--- a/third_party/fuchsia-sdk/BUILD.gn
+++ b/third_party/fuchsia-sdk/BUILD.gn
@@ -399,3 +399,15 @@ fuchsia_sdk_pkg("zx") {
"vmo.cpp",
]
}
+
+config("vulkan_libs") {
+ ldflags = [
+ "-Wl,--no-as-needed",
+ "-lvulkan",
+ "-Wl,--as-needed",
+ ]
+}
+
+source_set("vulkan_loader") {
+ all_dependent_configs = [":vulkan_libs"]
+}
,
Aug 7
Re #12: OK, understood. In that case we'll want to enhance the build_manifest.py to calculate dependencies from one or more executables, so that in this case we would have [ <app>, 'libvulkan.so', 'libvulkan_intel.so' ], for example, as required to be included, and build_manifest.py would add the other stuff. That may require some new parameter, e.g. |dynamic_libs| to the fuchsia_sdk_pkg() rule to express that a .so is a (possible) dlopen() dependency.
,
Aug 7
Re #1 & #4: It doesn't seem correct for a package to need to bundle a dependency required by a sysroot-bundled driver, as appears to be the case for libvulkan_intel.so's dependency on libtrace-engine.so. In general how can a package be expected to know the dependencies of things bundled with the sysroot?
,
Aug 7
I assume James was planning to remove them as part of cutting down the deps, per #1. I would like to understand the long term strategy, though. Are we going to remove all dependencies of the driver? If the driver uses even libc.so, then we've already caused a diamond dependency problem by putting libc.so in the package, as well as undermined the "fidl is the ABI" concept. What's the plan here?
,
Aug 7
The long term strategy is to remove all of the shared library dependencies from libvulkan_{vendor}.so, so application developers only need to package up libvulkan.so. Some of these are tricky to remove so we haven't been able to achieve this yet. We're working actively on removing the remaining shared library dependencies from libvulkan_{vendor}.so.
,
Aug 8
Taking this back for one of the Chrome-Fuchsia team to add support for calculating the transitive dependencies of >1 executable, so we can add e.g. a dynamic_libs to fuchsia_sdk_pkg().
,
Aug 9
,
Aug 10
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/30029f7b62764903416a8648bec3a1c6ff509347 commit 30029f7b62764903416a8648bec3a1c6ff509347 Author: Wez <wez@chromium.org> Date: Fri Aug 10 01:16:48 2018 Temporarily bundle all SOs from the SDK "dist" directory into packages. We previously attempted to include only the SOs listed in the primary executable's ELF headers as NEEDED. This causes SOs needed by other executables in the package to be missed, as well as any libraries used by the executable, but loaded via dlopen() rather than at load-time. Bug: 861931 Change-Id: If72d8007ca7d18674dd28eb310a0a5aa76519516 Reviewed-on: https://chromium-review.googlesource.com/1170179 Reviewed-by: Kevin Marshall <kmarshall@chromium.org> Commit-Queue: Wez <wez@chromium.org> Cr-Commit-Position: refs/heads/master@{#581998} [modify] https://crrev.com/30029f7b62764903416a8648bec3a1c6ff509347/build/config/fuchsia/build_manifest.py
,
Aug 14
image_pipe_swapchain will be available alongside the other Vulkan layers once https://chromium-review.googlesource.com/c/chromium/src/+/1174011 clears the commit queue. It has similar dynamic library requirements to the other layers, but also requires libasync-default.so. Later today, the shared library dependencies for all vulkan layers will shrink a good bit in that they will no longer require any of the c++ standard libraries. libvulkan_intel.so will follow shortly after that.
,
Sep 11
Remaining work is to re-implement culling of unused dist .so's, while taking into account the full set of runtime_deps, but this is not urgent.
,
Oct 2
,
Oct 2
,
Jan 17
(6 days ago)
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/12cc899e71aae6032d6eec4172a4e755c71cb5f3 commit 12cc899e71aae6032d6eec4172a4e755c71cb5f3 Author: Sergey Ulanov <sergeyu@chromium.org> Date: Thu Jan 17 05:23:09 2019 [Fuchsia] Exclude vulkan validation layers from release builds. Previously build_package.py was including all .so libs from the sdk/arch/dist directory. This includes vulkan validation layer libraries that are used only in debug builds. This CL restores the original logic in build_package.py that ensures that we only include libraries from sdk/arch/dist when they are used by the main application binary, or a transitive dependency. The vulkan libraries now are pulled as data_deps in //gpu/vulkan. The validation layer is included only in debug builds, which makes release packages smaller. Bug: 861931, 922139 Change-Id: I9d7fdbc451194da6013cdde9a32500cd6767d28b Reviewed-on: https://chromium-review.googlesource.com/c/1416834 Commit-Queue: Sergey Ulanov <sergeyu@chromium.org> Reviewed-by: Antoine Labour <piman@chromium.org> Reviewed-by: Wez <wez@chromium.org> Cr-Commit-Position: refs/heads/master@{#623592} [modify] https://crrev.com/12cc899e71aae6032d6eec4172a4e755c71cb5f3/build/config/fuchsia/build_manifest.py [modify] https://crrev.com/12cc899e71aae6032d6eec4172a4e755c71cb5f3/gpu/vulkan/BUILD.gn [modify] https://crrev.com/12cc899e71aae6032d6eec4172a4e755c71cb5f3/third_party/fuchsia-sdk/BUILD.gn
,
Jan 17
(5 days ago)
We have vulkan libs in SDK now and they are pulled to the packages generated in chromium. I'm keeping this bug open to address this TODO: https://chromium.googlesource.com/chromium/src/+/12cc899e71aae6032d6eec4172a4e755c71cb5f3/build/config/fuchsia/build_manifest.py#183 |
||||||||||
►
Sign in to add a comment |
||||||||||
Comment 1 by jamesr@google.com
, Jul 27The vulkan loader library (libvulkan.so) is now exported to the SDK used by Chromium at the path //third_party/fuchsia-sdk/sdk/arch/{x64,arm64}/lib/libvulkan.so (for linking) and //third_party/fuchsia-sdk/sdk/arch/{x64,arm64}/dist/libvulkan.so (for packaging/distribution). libvulkan.so currently requires that the following shared libraries from the SDK be included in the package using the loader: libc.so libfdio.so This is currently true in practice for every binary build in the Chromium tree, but is a bit awkward for future expansion so we're working on cutting that down. The vulkan client driver for Intel HD 5/600 parts is also included in the system image and can be loaded through the loader. It's called libvulkan_intel.so. It has a broader set of shared library requirements currently: libc++.so.2 libc++abi.so.1 libunwind.so libasync-default.so libtrace-engine.so I'm working on cutting this down as well. The vulkan client layers are not yet exposed in the SDK, but will be soon.