Skip to content

Conversation

@crepererum
Copy link
Collaborator

@crepererum crepererum commented Dec 8, 2025

Extends the pre-build setup (introduced in #225) by also compiling the WASM payload to different target architectures. This avoids having to do this "live" in InfluxDB and waste CI time there. Note that -- if you trust the binaries that we release -- this has the same security model as just-in-time (JIT) compiling the WASM bytecode.

You can find a release that was produced by this workflow here:
https://github.com/influxdata/datafusion-udf-wasm/releases/tag/wasm-binaries%2F2025-12-09T16-51-51%2F76c0d72017b0908c59a8af5ad786797c874de34f

@crepererum crepererum force-pushed the crepererum/prebuild-plus branch 5 times, most recently from 76c0d72 to 7ec7f58 Compare December 10, 2025 10:12
@crepererum crepererum force-pushed the crepererum/prebuild-plus branch from 7ec7f58 to a25565c Compare December 10, 2025 14:18
@crepererum crepererum marked this pull request as ready for review December 10, 2025 14:18
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR extends the pre-build workflow to compile WASM binaries to native machine code for multiple target architectures using Wasmtime. This allows InfluxDB and other consumers to use pre-compiled binaries instead of JIT-compiling WASM at runtime, saving CI time and improving startup performance.

Key Changes:

  • Added a new compile binary that pre-compiles WASM components to target-specific machine code
  • Extended the GitHub Actions prebuild workflow to compile WASM to 5 different target architectures (ARM64/x86_64 for Linux/Windows/macOS)
  • Enhanced the release artifacts with checksums, wasmtime version metadata, and improved documentation

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated 4 comments.

File Description
host/src/bin/compile.rs New CLI binary that compiles WASM to native code for specified target architectures
host/Cargo.toml Added binary configuration with all-arch feature requirement
.github/workflows/prebuild.yml Extended workflow to compile WASM to multiple targets, calculate checksums, extract wasmtime version, and enhance release documentation

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 80 to 87
targets=(
"aarch64-apple-darwin"
"aarch64-pc-windows-msvc"
"aarch64-unknown-linux-gnu"
"x86_64-pc-windows-msvc"
"x86_64-unknown-linux-gnu"
)
Copy link

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

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

[nitpick] Missing common target: The target list includes ARM64 and x86_64 for Linux and Windows, and ARM64 for macOS, but is missing x86_64-apple-darwin (Intel Macs). While Intel Macs can run ARM64 binaries via Rosetta 2, native x86_64 binaries would provide better performance. Consider adding this target for completeness.

Copilot uses AI. Check for mistakes.
for wasm_file in out/*.wasm; do
wasm_file="$(basename "$wasm_file" .wasm)"
for target in "${targets[@]}"; do
out_file="${wasm_file}.${target}.so"
Copy link

Choose a reason for hiding this comment

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

Should these always be so files? I'd expect dynlib on Mac and dll on windows, but I might be expecting the wrong thing.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

They are ALL ELF object files. Yes, even on Windows these will be elf files, which is not what a dll usually is. Same for mac Mac, where dynlib files are also different format). This might be a bit weird, but ELF can actually contain Windows and Mac code (this is done via the ABI metadata), and hence is a nice container for all sorts of machine code.

Also having to handle a file ending based on the target triplet (instead of only the triplet) makes both this build job and the code that uses these artifacts complicated for no good reason beyond -- what I would call -- a cosmetic beauty.

I however agree that .so is confusing. I've checked the ELF wikipedia article and it suggests that .elf is also a common suffix, so I'll use that.

@crepererum crepererum force-pushed the crepererum/prebuild-plus branch from 0bc9311 to 27d20de Compare December 11, 2025 16:44
@crepererum crepererum force-pushed the crepererum/prebuild-plus branch from 27d20de to 6f18055 Compare December 12, 2025 11:11
@crepererum crepererum added this pull request to the merge queue Dec 12, 2025
Merged via the queue into main with commit 28c2a47 Dec 12, 2025
1 check passed
@crepererum crepererum deleted the crepererum/prebuild-plus branch December 12, 2025 12:49
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.

3 participants