Skip to content
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

Use newer npm version in Docker UI build #17064

Merged
merged 5 commits into from
Feb 9, 2025
Merged

Conversation

desertaxle
Copy link
Member

@desertaxle desertaxle commented Feb 9, 2025

Docker builds have been running offensively long when running npm ci. This wasn't shown in our Docker build benchmark because we were only building for amd64 and arm64 was causing the issue. I belive the issue is because some of the npm dependeices (namely esbuilt) rely on C binaries that need to be recompiles for arm64.

This PR updates the Docker build benchmark workflow to perform a multi-platform build to get a more accurate read of build performance. It also updates the Dockerfile to only run the UI build step for the current build platform. Since the UI build step produces static HTML, JS, and CSS files, it is not platform-dependent. The result of this change is a ~90% reduction in build time.

Screenshot 2025-02-09 at 3 04 12 PM

Copy link

codspeed-hq bot commented Feb 9, 2025

CodSpeed Performance Report

Merging #17064 will not alter performance

Comparing tweak-docker-ui-build (c910269) with main (c8986ed)

Summary

✅ 2 untouched benchmarks

@@ -14,7 +14,7 @@ ARG NODE_VERSION=18.18.0
ARG EXTRA_PIP_PACKAGES=""

# Build the UI distributable.
FROM node:${NODE_VERSION}-bullseye-slim AS ui-builder
FROM --platform=$BUILDPLATFORM node:${NODE_VERSION}-bullseye-slim AS ui-builder
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the key change right here

@desertaxle desertaxle marked this pull request as ready for review February 9, 2025 21:04
Copy link
Collaborator

@zzstoatzz zzstoatzz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙌

@desertaxle desertaxle merged commit 332413a into main Feb 9, 2025
47 checks passed
@desertaxle desertaxle deleted the tweak-docker-ui-build branch February 9, 2025 21:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants