perf(vrl): adapt Vector to VRL ObjectMap#25697
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: abdc43a27f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| BENCH="/Users/luke.steensen/code/vector/bench.sh" | ||
| RESULTS_DIR="/Users/luke.steensen/code/vector/bench-results/full" |
There was a problem hiding this comment.
Replace hard-coded benchmark paths
On any checkout that is not exactly /Users/luke.steensen/code/vector, running the checked-in bench-all.sh invokes a non-existent benchmark script and writes results under that developer-specific home directory, so the batch benchmark workflow fails before exercising the cases. Derive these paths from the script location, as bench.sh does, or make them configurable.
Useful? React with 👍 / 👎.
| protobuf-compiler \ | ||
| && rm -rf /var/lib/apt/lists/* | ||
| WORKDIR /vector | ||
| COPY --from=vrl . /vrl |
There was a problem hiding this comment.
Use the copied VRL checkout for bench builds
When ./bench.sh build is used to benchmark local VRL variants, this copies ../vrl into the image, but the subsequent cargo build has no path override or [patch] pointing Cargo at /vrl (and the repo config has no such override), so the image is built from the pinned git dependency in Cargo.lock instead. This makes local changes in ../vrl invisible to the benchmark images the script is meant to compare.
Useful? React with 👍 / 👎.
| vector-vrl-category = { path = "lib/vector-vrl/category" } | ||
| vector-vrl-functions = { path = "lib/vector-vrl/functions", default-features = false } | ||
| vrl = { git = "https://github.com/vectordotdev/vrl.git", branch = "main", default-features = false, features = ["arbitrary", "cli", "test", "test_framework", "stdlib-base"] } | ||
| vrl = { git = "https://github.com/lukesteensen/vrl.git", branch = "experiment/objectmap-backends", default-features = false, features = ["arbitrary", "cli", "test", "test_framework", "stdlib-base"] } |
There was a problem hiding this comment.
Avoid depending on a personal VRL fork
For release/CI builds from this commit, the vrl dependency now resolves from a personal fork and experiment branch instead of the official vectordotdev/vrl repository. Even with Cargo.lock pinning a commit, fresh builders still have to fetch from that fork, so deleting or force-pushing the branch, changing access, or losing the fork breaks reproducible builds and puts production dependency resolution outside the project-controlled remote.
Useful? React with 👍 / 👎.
| DOCKER_BUILDKIT=1 docker buildx build \ | ||
| --build-context "vrl=${SCRIPT_DIR}/../vrl" \ | ||
| -f "${SCRIPT_DIR}/Dockerfile.bench" \ | ||
| -t "vector:${tag}" \ | ||
| "${SCRIPT_DIR}" |
There was a problem hiding this comment.
Load buildx images before running benchmarks
When using a non-docker Buildx builder such as the docker-container driver commonly used with buildx, this tagged build is not loaded into the local Docker image store unless --load or an explicit exporter is set; Docker's docs state that otherwise Buildx defaults to cacheonly and the result remains only in the build cache. In that environment ./bench.sh build <tag> succeeds but smp local run cannot find vector:<tag>, so add --load here and to the derive build.
Useful? React with 👍 / 👎.
Supersedes fork-based draft PR #25649.
Summary
Adapts Vector to the experimental VRL
ObjectMapwork by replacing directBTreeMapassumptions withObjectMap-compatible construction and iterationpatterns.
VRL dependency used for this draft:
90933efb3b4e9e27f95697ef9adc129fba508e44Commit structure
The main ObjectMap work is isolated in:
refactor: adapt Vector to VRL ObjectMapchore: use git VRL ObjectMap dependencyThis branch also includes two small test fixes that were discovered while
testing the ObjectMap changes locally, but are otherwise unrelated to ObjectMap:
test(codecs): avoid mutating global log schema in syslog testslog_schemastate, which can polluteother tests depending on execution order.
test(vector-core): keep empty DDSketch fixtures canonicalround-trip expectations.
Benchmark support is included separately in:
chore(bench): add ObjectMap benchmark helpersValidation
Passed:
cargo fmt --all -- --checkcargo check --workspace --all-targets --all-features --lockedmake check-clippycargo test -p vector-core --all-features --lib --lockedcargo test -p codecs --all-features --lib --lockedobject_maptestsBenchmark sanity check:
datadog_agent_remap_blackholebtree-> currentflat:+24.43%--replicates 1 --total-samples 60 --warmup-seconds 15