New issue
Advanced search Search tips

Issue 861931 link

Starred by 3 users

Issue metadata

Status: Started
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Fuchsia
Pri: 3
Type: Bug



Sign in to add a comment

Restore optimization to remove unused .so's from dist/, and add libvulkan.so etc explicitly to package.

Project Member Reported by spang@chromium.org, Jul 9

Issue description

We 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.
 
The 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.
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

The vulkan loader has a hack for loading libvulkan_intel.so, but it doesn't help for transitive dependencies of that object.
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.
Cc: jam...@chromium.org kmarshall@chromium.org
Labels: M-70
Owner: w...@chromium.org
Status: Assigned (was: Untriaged)
Assigning-to-self to follow up on tomorrow, and CC'ing kmarshall@, who is likely to be the ultimate owner. ;)
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.
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.
Cc: w...@chromium.org
Owner: spang@chromium.org
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?
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..?
Cc: sergeyu@chromium.org
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).
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"]
+}

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.
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?
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?
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.
Cc: -jam...@chromium.org fdegans@chromium.org
Labels: -Pri-2 Pri-1
Owner: w...@chromium.org
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().
Status: Started (was: Assigned)
Project Member

Comment 20 by bugdroid1@chromium.org, 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

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.
Labels: -Pri-1 -M-70 Pri-3
Owner: ----
Status: Available (was: Started)
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.
Components: Internals>PlatformIntegration
Summary: Restore optimization to remove unused .so's from dist/, and add libvulkan.so etc explicitly to package. (was: add libvulkan.so & shared deps to fuchsia SDK)
Project Member

Comment 25 by bugdroid1@chromium.org, 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

Comment 26 by sergeyu@chromium.org, Jan 17 (5 days ago)

Status: Started (was: Available)
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