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

dockerfile: enable SIMD optimization for x86_64, amd64, arm64 and aarch64 #9575

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

edsiper
Copy link
Member

@edsiper edsiper commented Nov 10, 2024


Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.

@edsiper
Copy link
Member Author

edsiper commented Nov 12, 2024

FYI: we will put this PR on hold until next minor release.

# SIMD support: x86_64, amd64 and arm64
ARG SIMD_CMAKE_FLAG=""
RUN SIMD_CMAKE_FLAG="" && \
case "$(uname -m)" in \
Copy link
Contributor

@patrick-stephens patrick-stephens Nov 12, 2024

Choose a reason for hiding this comment

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

This won't work for containers on emulated targets - it takes the value from the host. If you try to build the container on a different host then it'll just take whatever value the host is.
Personally I think we should make this backwards compatible and just pass it in as required. People can then adopt it as required and configure it appropriately for their host. I know this attempts to discover it if not set but it will be auto-applied for those who may not want it.

Copy link
Contributor

@patrick-stephens patrick-stephens Nov 12, 2024

Choose a reason for hiding this comment

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

I think we will have to update container builds to be split per target to support this as well (i.e. build and push by digest for a specific arch/target and then make a multi-arch/target manifest separately), rather than the simple current approach we have of building all on the same target:

- name: Build the production images
id: build_push
uses: docker/build-push-action@v6
with:
file: ./dockerfiles/Dockerfile
context: .
tags: ${{ steps.meta.outputs.tags }}
labels: ${{ steps.meta.outputs.labels }}
platforms: linux/amd64, linux/arm64, linux/arm/v7, linux/s390x
target: production
# Must be disabled to provide legacy format images from the registry
provenance: false
push: true
load: false
build-args: |
FLB_NIGHTLY_BUILD=${{ inputs.unstable }}
RELEASE_VERSION=${{ inputs.version }}

Currently the image you make with this change would just be based on the host.

This was requested anyway to support other things like a common image for both Linux and Windows: #9509

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants