-
Notifications
You must be signed in to change notification settings - Fork 35
use erofs for headful image #39
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
syncing up with the approach used in the headless image
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.
What Changed
This PR updates the chromium-headful image to use an EROFS-based initrd for its root filesystem, aligning its build process with the chromium-headless image. This is expected to reduce memory usage. Key changes include:
- Modifying
images/chromium-headful/build-unikernel.shto create the EROFS filesystem. - Updating
images/chromium-headful/Kraftfileto use the generatedinitrd. - Introducing a new shared script,
shared/erofs-utils.sh, to manage theerofs-utilsdependency across different environments. - Refactoring the
chromium-headlessbuild script to leverage this new shared utility, which reduces code duplication.
Risks / Concerns
This is a great change. It not only brings a performance improvement to the headful image but also improves the overall codebase by refactoring shared logic into a common utility script. The changes are clear and follow an established pattern within the project. Well done!
4 files reviewed | 0 comments | Review on Mesa | Edit Reviewer Settings
Sayan-
left a comment
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.
code changes lgtm. I'd like to understand how we want to verify the mem footprint savings but maybe it's a ship it and see situation
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.
Bug: Sudo Usage Inconsistency Across Build Scripts
Inconsistent sudo usage for the mkfs.erofs command between the chromium-headful (uses sudo) and chromium-headless (does not use sudo) build scripts. Both scripts utilize the --all-root flag, which typically requires elevated privileges. This inconsistency can lead to build failures or differing file permission behaviors depending on the environment's sudo configuration.
images/chromium-headful/build-unikernel.sh#L24-L25
Was this report helpful? Give feedback by reacting with 👍 or 👎
syncing up with the approach used in the headless image
this should reduce memory usage
TL;DR
Switched the Chromium headful unikernel to use EROFS for its root filesystem, aligning it with the headless image and reducing memory usage.
Why we made these changes
To reduce memory consumption for the Chromium headful image and standardize its root filesystem approach with the existing headless image, improving overall efficiency.
What changed?
Kraftfileto useidlescale-to-zero policy, increased cooldown, and pointed rootfs to./initrd.build-unikernel.shto generate theinitrdusingmkfs.erofsfrom Docker-extracted contents.shared/erofs-utils.shwith functions to check for and installmkfs.erofs.chromium-headless/build-unikernel.shto utilize these new shared utility scripts, removing duplicated logic.Validation
Kraftfilebehave correctly.