comphead commented on PR #7356:
URL:
https://github.com/apache/datafusion-comet/pull/7356#issuecomment-6615579626
Thanks @andygrove and @sunchao. The changes are in 023d32b6b3.
**Label run passing on glibc** (both reviews). The check now runs inside the
test JVM. `the run-jemalloc-tests CI run loads the jemalloc library` asserts
`NativeBase.isFeatureEnabled("jemalloc")` whenever `ENABLE_COMET_JEMALLOC` is
true, and cancels otherwise. It is in `CometSparkSessionExtensionsSuite`, which
runs in the `[exec]` group. Every group gets the same native artifact and the
same environment, so a jemalloc library that is missing or fails to load fails
that group, and with it the run. If you'd rather have every group check before
its suites, I can move the assertion into `CometTestBase`. The fallback moved
into `NativeBase.loadBundled`. `a missing or unloadable jemalloc library falls
back to the default one` covers five cases: not requested, requested and
bundled, missing from the jar, rejected by `System.load`, and no default
library.
**aarch64 page size.** You're right, and it is worse than an abort at the
first allocation. On Linux, jemalloc's `configure` detects the build host's
page size, and `pages_boot` fails jemalloc's init on a kernel with larger
pages, so every allocation returns NULL. opendal registers its services in a
load-time constructor that allocates, so the JVM would abort inside
`System.load`, where the fallback cannot help. `core-arm64-libs` now builds
jemalloc with `JEMALLOC_SYS_WITH_LG_PAGE=16`. tikv-jemalloc-sys documents that
setting for this case: the allocator page must be at least as large as the
system page. That works on both 4 KiB and 64 KiB kernels. On a 4 KiB kernel it
means 64 KiB allocator pages, whose memory cost I have not measured. I don't
have a 64 KiB kernel to test on.
**Jar size.** The loader finds the jemalloc library with
`NativeBase.class.getResourceAsStream`, which searches the whole classpath. So
the jemalloc libraries could ship as a separate native-only jar with no loader
change, one per Comet version, since the native library does not depend on the
Spark profile. That needs a new artifact in `build-release-comet.sh`,
`publish-to-maven.sh` and the snapshot workflow, plus a check that it comes
from the same build as the plugin jar. For now the installation guide states
the size: about 85 MB, which every executor downloads when the jar comes from
`--jars` or `spark.jars`.
**Label name.** Renamed to `run-jemalloc-tests` in 6a46039190.
**Build time.** `cargo tree -e features` resolves the same features for
every shared dependency in the `-p datafusion-comet --features jemalloc` build
as in the workspace build. The only differences are the features of
`datafusion-comet` itself, the three jemalloc crates, and `comet-test-udfs`,
which the first build skips. So no dependency builds twice. In the labeled CI
run at 6a46039190, the jemalloc build took 3m34s and the default build after it
took 1m35s, with dependencies from main's cache. That is the CI profile,
though. The release profile uses thin LTO and one codegen unit, so it costs
more there, and I haven't measured that. A `publish_snapshot.yml` dry run would
give the real number.
**Config read timing.** Spark sets `SparkEnv` before it starts plugins or
runs tasks.
- On the driver, `SparkContext` calls `SparkEnv.set(_env)` before it creates
the `PluginContainer`. I checked 3.4.3, 3.5.9, 4.0.1 and master.
- On executors, `createExecutorEnv` sets it before `Executor` creates its
plugins and runs tasks. I checked 3.4.3, 3.5.9 and master.
- Comet first loads the library later than both points: on the driver from
`isCometLoaded` during planning, and on executors from the first native call in
a task.
The only gap is code that uses Comet before a SparkContext exists. That case
now logs a warning and reads the JVM's `spark.*` system properties as well as
`ENABLE_COMET_JEMALLOC`.
--
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]
