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.

Sign up for GitHub

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
Open

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

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion dockerfiles/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,15 @@ COPY . ./
# We split the builder setup out so people can target it or use as a base image without doing a full build.
FROM builder-base AS builder
WORKDIR /src/fluent-bit/build/
RUN cmake -DFLB_RELEASE=On \

# 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 •
edited
Loading

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 •
edited
Loading

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:

fluent-bit/.github/workflows/call-build-images.yaml

Lines 115 to 131 in 58cc2d5

- 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

"x86_64"|"amd64"|"aarch64"|"arm64") \
SIMD_CMAKE_FLAG="-DFLB_SIMD=On";; \
esac && \
cmake -DFLB_RELEASE=On \
-DFLB_JEMALLOC=On \
-DFLB_TLS=On \
-DFLB_SHARED_LIB=Off \
Expand All @@ -78,6 +86,7 @@ RUN cmake -DFLB_RELEASE=On \
-DFLB_NIGHTLY_BUILD="$FLB_NIGHTLY_BUILD" \
-DFLB_LOG_NO_CONTROL_CHARS=On \
-DFLB_CHUNK_TRACE="$FLB_CHUNK_TRACE" \
$SIMD_CMAKE_FLAG \
..

RUN make -j "$(getconf _NPROCESSORS_ONLN)"
Expand Down
Loading