Add Vulkan patches and update Docker build process#671
Add Vulkan patches and update Docker build process#671ericcurtin wants to merge 1 commit intomainfrom
Conversation
Summary of ChangesHello @ericcurtin, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances Vulkan compatibility and performance for specific deployment environments, particularly for Linux virtual machines on macOS. By integrating targeted driver patches and refining the Docker build process, it resolves critical allocation and compatibility issues, ensuring a smoother and more functional Vulkan experience. The changes are focused on enabling robust graphics capabilities in previously challenging configurations. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- On Debian/Ubuntu images
uname -mtypically returnsarm64rather thanaarch64, so the aarch64 check inapt-install.shmay never trigger; consider usingdpkg --print-architectureor checking for both values. - The
enable_source_reposfunction rewrites all.sourcesfiles and uncomments alldeb-srclines globally, which may be broader than needed; you might want to scope this to the specific Ubuntu repo entries you rely on to avoid unexpected side effects. - The temporary build directory for rebuilding Mesa is only cleaned up on success; using a
trapto remove it on exit or error would avoid leaving build artifacts around when the script fails midway.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- On Debian/Ubuntu images `uname -m` typically returns `arm64` rather than `aarch64`, so the aarch64 check in `apt-install.sh` may never trigger; consider using `dpkg --print-architecture` or checking for both values.
- The `enable_source_repos` function rewrites all `.sources` files and uncomments all `deb-src` lines globally, which may be broader than needed; you might want to scope this to the specific Ubuntu repo entries you rely on to avoid unexpected side effects.
- The temporary build directory for rebuilding Mesa is only cleaned up on success; using a `trap` to remove it on exit or error would avoid leaving build artifacts around when the script fails midway.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Code Review
This pull request introduces patches for the Vulkan driver to enhance compatibility and performance, particularly for running on macOS with a 16k kernel via a Linux VM. The Docker build process is updated to apply these patches and build a custom Mesa driver for ARM64 systems. The changes are logical and well-structured. I have a couple of suggestions on the patch files to improve maintainability and reduce risks, particularly regarding a magic number and a commented-out log message.
a5d9739 to
75510c8
Compare
Add three Vulkan driver patches to fix venus compatibility, force 16k alignment for allocations, and silence stuck wait messages. Update Dockerfile to copy all scripts and install patched Mesa drivers on ARM64 systems. This ensure Linux VMs with 4k kernels can use Vulkan on macOS which has a 16k kernel. Signed-off-by: Eric Curtin <eric.curtin@docker.com>
75510c8 to
91c8d20
Compare
Add three Vulkan driver patches to fix venus compatibility, force 16k alignment for allocations, and silence stuck wait messages. Update Dockerfile to copy all scripts and install patched Mesa drivers on ARM64 systems. This ensure Linux VMs with 4k kernels can use Vulkan on macOS which has a 16k kernel.