comphead opened a new pull request, #7356:
URL: https://github.com/apache/datafusion-comet/pull/7356
## Which issue does this PR close?
Related to #7344 and #7108. Both stay open. This PR only makes a jemalloc
build of the native library available as an opt-in. #7344 is still collecting
the data on whether it should become the default.
## Rationale for this change
glibc keeps much of the native memory Comet frees in its per-thread arenas,
so a long-running executor's resident memory can stay far above what Comet's
memory pools report, and the container gets killed (#7108). The `jemalloc`
Cargo feature already exists, but trying it needs a custom native build. That
is why the jemalloc arm is still missing from the comparisons in #7108 and
#7344. Shipping both builds in the jar lets a user switch with one config, and
lets that evaluation run on a stock jar.
The cost is jar size. In the 1.2.0 snapshot jar (99 MB), the Linux libraries
are 44 MB (amd64) and 40.5 MB (aarch64) compressed, so a second pair brings
each published jar to about 185 MB. Only the selected library is extracted at
runtime.
## What changes are included in this PR?
- **Config.** `spark.comet.memory.jemalloc.enabled`, off by default. The
`ENABLE_COMET_JEMALLOC` environment variable sets its default, like
`ENABLE_COMET`. `NativeBase` reads it from the application's `SparkConf`
(`SparkEnv.get.conf`), not through `SQLConf`, because `SQLConf.get` depends on
the thread that loads the library first. On an executor outside a task, that is
an empty conf.
- **Loader.** With the config set, `NativeBase` loads
`org/apache/comet/<os>/<arch>/jemalloc/libcomet.*`. It falls back to the
default library with a warning when the jar has no jemalloc build for the
platform, or when `System.load` rejects it, which leaves nothing loaded. It
logs which allocator the loaded library uses. A library on `java.library.path`
still takes precedence, as before.
- **Packaging.**
- `make core-amd64-libs` and `core-arm64-libs` build `-p datafusion-comet
--features jemalloc` first and copy it to `native/target/release/jemalloc/`.
Then they build the default library, so every existing script finds it at the
usual path.
- `build-release-comet.sh` and `publish_snapshot.yml` copy both. The
snapshot workflow checks both against the glibc 2.31 baseline and requires both
in every jar.
- `spark/pom.xml` bundles `${jni.dir}/jemalloc` when it exists.
- **CI.** A `jemalloc_test` label sets `build_linux_jemalloc` in
`compute-changes.py`, which `ci.yml` passes as the `jemalloc` input of
`pr_build_linux.yml`.
- That input builds the jemalloc library with `--profile ci` and sets
`ENABLE_COMET_JEMALLOC=true` for the JVM tests.
- It only applies to pull requests into `main`. A manual run sets every
output, and a release branch has no merge queue to test the default library
afterwards.
- Unlabeled runs, the merge queue and the nightly are unchanged and test
the default library.
- `check-ci-config.py` has cases for the label, and `ci.md` lists it.
- **Docs.** The config doc, and a paragraph in the memory tuning guide on
enabling it and on jemalloc reading `_RJEM_MALLOC_CONF` rather than
`MALLOC_CONF`.
Not included: making jemalloc the default (#7344), macOS jemalloc builds,
and the `core-amd64`/`core-arm64` targets that build `comet-native-*.jar`.
## How are these changes tested?
No new tests. The default path is unchanged, and the existing suites keep
running on the default library.
Checked locally:
- `./mvnw -o -Pspark-4.1 -DskipTests compile spotless:check` passes. It
includes scalastyle, which runs in `compile`.
- `python3 dev/ci/check-ci-config.py` passes, including the new label cases.
- `actionlint` reports nothing new on the three changed workflows.
- `make -n core-amd64-libs` and `core-arm64-libs` print the expected build
order, with and without `COMET_FEATURES`.
- With a placeholder `native/target/debug/jemalloc/libcomet.dylib`,
`process-resources` puts it at
`org/apache/comet/darwin/aarch64/jemalloc/libcomet.dylib`.
Not run yet: loading the jemalloc library in a JVM, and the packaging
workflows. The `jemalloc_test` label (it has to be created) runs the Linux test
matrix with that library, and a `publish_snapshot.yml` dry run would check the
packaging.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
