[comphead]

Which issue does this PR close?

Related to #7344 and #7108. Both stay open.

Rationale for this change

glibc keeps freed native memory resident in long-running executors (#7108). The jemalloc Cargo feature exists, but trying it needs a custom build, so it cannot be compared with glibc on a stock jar (#7344). This PR bundles a jemalloc build next to the default library and lets a config pick it. The default stays the system allocator.

What changes are included in this PR?

flowchart TB
    subgraph build["Build: make core-amd64-libs / core-arm64-libs"]
        JB["cargo build -p datafusion-comet --features jemalloc"] --> JL["native/target/release/jemalloc/libcomet.so"]
        DB["cargo build"] --> DL["native/target/release/libcomet.so"]
    end

    subgraph jar["Comet jar"]
        JR["org/apache/comet/linux/ARCH/jemalloc/libcomet.so"]
        DR["org/apache/comet/linux/ARCH/libcomet.so"]
    end

    JL --> JR
    DL --> DR

    subgraph load["NativeBase.load(), once per JVM"]
        P{"libcomet on java.library.path?"}
        C{"spark.comet.memory.jemalloc.enabled<br/>read from SparkEnv.get.conf"}
        T{"jemalloc build in the jar<br/>and System.load succeeds?"}
        P -->|yes| U["use that library"]
        P -->|no| C
        C -->|false| D["load the default library"]
        C -->|true| T
        T -->|yes| J["load the jemalloc library"]
        T -->|"no, log a warning"| D
    end

    E["ENABLE_COMET_JEMALLOC=true<br/>sets the default, as CI does<br/>for the run-jemalloc-tests label"] -.-> C
    JR -.-> J
    DR -.-> D

Loading

• spark.comet.memory.jemalloc.enabled defaults to false, and ENABLE_COMET_JEMALLOC can set that default. NativeBase reads it from SparkEnv.get.conf, because SQLConf.get depends on the thread that loads the library.
• make core-*-libs, build-release-comet.sh and publish_snapshot.yml build and bundle both Linux libraries, and check both against glibc 2.31. Each jar grows from about 99 MB to about 185 MB. Only one library is extracted at runtime.
• CI: the run-jemalloc-tests label runs a pull request's Linux tests on the jemalloc library, for pull requests into main only. Every other run still tests the default library.

How are these changes tested?

No new tests. Checked locally: offline compile with spotless and scalastyle, dev/ci/check-ci-config.py, actionlint, make -n, and the jar layout with a placeholder library. Not run yet: loading the jemalloc library in a JVM (the run-jemalloc-tests label, once created) and a publish_snapshot.yml dry run.

[andygrove]

Thanks for this, @comphead. Getting a jemalloc arm into the #7108 and #7344 comparisons on a stock jar is a good goal. The loader is careful. A missing resource and a failed System.load both fall back to the default library, the glibc 2.31 check now covers both libraries, and reading the SparkConf instead of the SQLConf is the right call. I compared against branch-1.2. With the config off, the same library loads from the same path, so I don't see a behavior change for existing users other than the jar size.

I have a few questions I'd like answered before this leaves draft.

Jar size for users who never opt in. The jemalloc libraries add about 85 MB to each published jar, and there are seven Spark and Scala combinations per release. Spark ships the jar to every executor, so everyone pays for that in localization time and disk, including users who never set the config. The description says #7344 has not yet shown that jemalloc should be the default. Did you consider publishing the jemalloc libraries as a separate artifact or classifier, so only users who opt in download them? If the bundled jar stays, could the user guide state the size?

aarch64 page size. jemalloc is compiled for the page size of the build host. I believe tikv-jemalloc-sys can fail at init on a kernel with larger pages, such as 64K on some Arm server distributions, but I haven't checked that against the crate. If it does fail there, jemalloc aborts the process at the first allocation, and the fallback in bundleLoadLibrary can't catch that because it isn't an UnsatisfiedLinkError. How does the aarch64 build behave on such a kernel? Should the build set JEMALLOC_SYS_WITH_LG_PAGE, or can you test it on one?

The jemalloc_test run can pass without testing jemalloc. If the jemalloc library is missing or fails to load, NativeBase logs a warning and loads the default one, and the Linux matrix goes green having tested glibc. I checked the plumbing and the artifact does keep jemalloc/libcomet.so, it lands in native/target/release/jemalloc, and CI doesn't set java.library.path, so the bundled path is used. Could we add a check that fails the jemalloc run when NativeBase.isFeatureEnabled("jemalloc") doesn't match ENABLE_COMET_JEMALLOC? I'd also suggest a unit test for the two fallback paths in bundleLoadLibrary, since the description says there are no new tests and the loader now has several branches.

Label name. The other opt-in labels are all run-*, for example run-macos-tests. Should this one be run-jemalloc-tests? That would also keep ci.md and the compute-changes.py rules uniform.

Release and snapshot build time. core-amd64-libs and core-arm64-libs now run two native release builds per architecture. The -p datafusion-comet --features jemalloc build and the following workspace build may unify dependency features differently, so dependencies could rebuild. Do you know how much this adds to the release and the snapshot workflow?

Config read timing. jemallocRequested() returns the default when SparkEnv.get() is null. If someone set the config and the env isn't there yet, they silently get glibc, and the only signal is the logAllocator() line. Can you say when SparkEnv is guaranteed to be set before NativeBase.load() on the driver and on executors? If there is a window where it isn't, a warning in that case would help.

[sunchao]

The Linux packaging includes both libraries, but a green jemalloc test run can still be using the system allocator. If the jemalloc resource is absent or fails to load, the JVM falls back and the same suites can pass. I agree with the existing review. Could the opt-in job assert NativeBase.isFeatureEnabled("jemalloc") inside the test JVM before running the suites?