Skip to content

Commit e25369b

Browse files
authored
Android: build/runtime fixes for modern Clang + ETDump path override (pytorch#19293)
## Summary Five small fixes needed to build and run ExecuTorch Android on hosts with Apple Clang 21, on devices where `/data/local/tmp` isn't app-writable, and to make the Android build script behave correctly on hosts with multiple Python interpreters: 1. **flatcc `-Werror` on modern Clang.** flatcc's vendored `CMakeLists.txt` hits `-Wimplicit-int-conversion-on-negation` and `-Wunterminated-string-initialization` on new Clang. Pass `FLATCC_ALLOW_WERROR=OFF` into the `flatcc_ep` ExternalProject so the third-party build doesn't fail. 2. **`EXECUTORCH_ANDROID_PROFILING` was never forwarded.** `scripts/build_android_library.sh` set `EXECUTORCH_ENABLE_EVENT_TRACER` from `$EXECUTORCH_ANDROID_PROFILING` but never forwarded the separate `-DEXECUTORCH_ANDROID_PROFILING` define that gates `ETDumpGen` allocation in `extension/android/jni/jni_layer.cpp`. As a result the event tracer was compiled in but the JNI never attached it, and `Module.etdump()` always returned `false`. 3. **`Module.etdump(String outputPath)` overload.** Add a JNI method (`etdumpToNative`) and Java wrapper so apps can write the ETDump into their own `filesDir`. The existing `etdump()` hardcodes `/data/local/tmp/result.etdump`, which is not app-writable on modern Android (Pixel 9 / API 36 returns `EACCES`). The new overload factors the write path through a shared `etdump_to_path()` helper; `etdump()` now delegates to it with the legacy path. 4. **`define_overridable_option` was expanding `${DESCRIPTION}` unquoted.** The `set(... CACHE TYPE DOCSTRING [FORCE])` form expects the docstring as a single argument, but `${DESCRIPTION}` was unquoted, so any `STRING` option with a multi-word description that the user overrides on the cmake command line had its description text spill into subsequent `set()` arguments. One-character fix: quote `"${DESCRIPTION}"` in both `set()` calls. 5. **`scripts/build_android_library.sh` never forwarded `PYTHON_EXECUTABLE` to cmake.** The script defaults `PYTHON_EXECUTABLE=python3` and honors a caller override, but only used it for `which`. cmake's `find_package(Python3)` then picked up whatever interpreter came first on PATH (e.g. Homebrew's `python3` on macOS) instead of the project's torch-equipped venv, failing at `find_package_torch_headers` with `'NoneType' object has no attribute 'submodule_search_locations'`. Pass `-DPYTHON_EXECUTABLE="${PYTHON_EXECUTABLE}"` so the script's documented behavior actually flows through. ## Test plan - [x] Apple Clang 21 host build: `scripts/build_android_library.sh` now succeeds (flatcc no longer fails `-Werror`). - [x] With `EXECUTORCH_ANDROID_PROFILING=1`: `-DEXECUTORCH_ANDROID_PROFILING` reaches the JNI compile, `ETDumpGen` is attached, `Module.etdump()` returns `true` and writes a valid `.etdump`. - [x] On Pixel 9 / API 36: `Module.etdump(context.getFilesDir() + "/result.etdump")` succeeds; legacy `Module.etdump()` still works on devices where `/data/local/tmp` is writable. - [x] Default build (`EXECUTORCH_ANDROID_PROFILING` unset): unchanged — `etdump()` returns `false` as before. - [x] `cmake -DEXECUTORCH_VULKAN_FP16_PRECISION=mediump …` (multi-word description STRING option) now configures cleanly. - [x] Hosts with both Homebrew `python3` and a torch-equipped venv: `PYTHON_EXECUTABLE=$VENV/bin/python scripts/build_android_library.sh` configures correctly. 🤖 Authored with [Claude Code](https://claude.com/claude-code) cc @kirklandsign @cbilgin
1 parent 8020fe0 commit e25369b

5 files changed

Lines changed: 76 additions & 28 deletions

File tree

extension/android/executorch_android/src/main/java/org/pytorch/executorch/Module.java

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,28 @@ public String[] readLogBuffer() {
227227
@DoNotStrip
228228
public native boolean etdump();
229229

230+
/**
231+
* Dump the ExecuTorch ETDump file to {@code outputPath}.
232+
*
233+
* @param outputPath absolute path to write the etdump file to.
234+
* @return true if the etdump was successfully written, false otherwise.
235+
*/
236+
@Experimental
237+
public boolean etdump(String outputPath) {
238+
mLock.lock();
239+
try {
240+
if (!mHybridData.isValid()) {
241+
throw new IllegalStateException("Module has been destroyed");
242+
}
243+
return etdumpToNative(outputPath);
244+
} finally {
245+
mLock.unlock();
246+
}
247+
}
248+
249+
@DoNotStrip
250+
private native boolean etdumpToNative(String outputPath);
251+
230252
/**
231253
* Explicitly destroys the native Module object. Calling this method is not required, as the
232254
* native object will be destroyed when this object is garbage-collected. However, the timing of

extension/android/jni/jni_layer.cpp

Lines changed: 48 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -466,34 +466,15 @@ class ExecuTorchJni : public facebook::jni::HybridClass<ExecuTorchJni> {
466466
}
467467

468468
jboolean etdump() {
469-
#ifdef EXECUTORCH_ANDROID_PROFILING
470-
executorch::etdump::ETDumpGen* etdumpgen =
471-
(executorch::etdump::ETDumpGen*)module_->event_tracer();
472-
auto etdump_data = etdumpgen->get_etdump_data();
469+
return etdump_to_path("/data/local/tmp/result.etdump");
470+
}
473471

474-
if (etdump_data.buf != nullptr && etdump_data.size > 0) {
475-
int etdump_file =
476-
open("/data/local/tmp/result.etdump", O_WRONLY | O_CREAT, 0644);
477-
if (etdump_file == -1) {
478-
ET_LOG(Error, "Cannot create result.etdump error: %d", errno);
479-
return false;
480-
}
481-
ssize_t bytes_written =
482-
write(etdump_file, (uint8_t*)etdump_data.buf, etdump_data.size);
483-
if (bytes_written == -1) {
484-
ET_LOG(Error, "Cannot write result.etdump error: %d", errno);
485-
return false;
486-
} else {
487-
ET_LOG(Info, "ETDump written %d bytes to file.", bytes_written);
488-
}
489-
close(etdump_file);
490-
free(etdump_data.buf);
491-
return true;
492-
} else {
493-
ET_LOG(Error, "No ETDump data available!");
472+
jboolean etdumpTo(facebook::jni::alias_ref<jstring> outputPath) {
473+
if (!outputPath) {
474+
ET_LOG(Error, "etdumpTo called with null outputPath");
475+
return false;
494476
}
495-
#endif
496-
return false;
477+
return etdump_to_path(outputPath->toStdString().c_str());
497478
}
498479

499480
facebook::jni::local_ref<facebook::jni::JArrayClass<jstring>> getMethods() {
@@ -565,10 +546,51 @@ class ExecuTorchJni : public facebook::jni::HybridClass<ExecuTorchJni> {
565546
makeNativeMethod(
566547
"readLogBufferStaticNative", ExecuTorchJni::readLogBufferStatic),
567548
makeNativeMethod("etdump", ExecuTorchJni::etdump),
549+
makeNativeMethod("etdumpToNative", ExecuTorchJni::etdumpTo),
568550
makeNativeMethod("getMethods", ExecuTorchJni::getMethods),
569551
makeNativeMethod("getUsedBackends", ExecuTorchJni::getUsedBackends),
570552
});
571553
}
554+
555+
private:
556+
jboolean etdump_to_path(const char* path) {
557+
#ifdef EXECUTORCH_ANDROID_PROFILING
558+
auto* tracer = module_->event_tracer();
559+
if (!tracer) {
560+
ET_LOG(Error, "ETDump not available: no event tracer attached");
561+
return false;
562+
}
563+
auto* etdumpgen = static_cast<executorch::etdump::ETDumpGen*>(tracer);
564+
auto etdump_data = etdumpgen->get_etdump_data();
565+
566+
if (etdump_data.buf != nullptr && etdump_data.size > 0) {
567+
int etdump_file = open(path, O_WRONLY | O_CREAT | O_TRUNC, 0644);
568+
if (etdump_file == -1) {
569+
ET_LOG(Error, "Cannot create %s error: %d", path, errno);
570+
free(etdump_data.buf);
571+
return false;
572+
}
573+
ssize_t bytes_written =
574+
write(etdump_file, (uint8_t*)etdump_data.buf, etdump_data.size);
575+
if (bytes_written == -1) {
576+
ET_LOG(Error, "Cannot write %s error: %d", path, errno);
577+
close(etdump_file);
578+
free(etdump_data.buf);
579+
return false;
580+
} else {
581+
ET_LOG(Info, "ETDump written %zd bytes to %s.", bytes_written, path);
582+
}
583+
close(etdump_file);
584+
free(etdump_data.buf);
585+
return true;
586+
} else {
587+
ET_LOG(Error, "No ETDump data available!");
588+
}
589+
#else
590+
(void)path;
591+
#endif
592+
return false;
593+
}
572594
};
573595
} // namespace executorch::extension
574596

scripts/build_android_library.sh

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,11 @@ build_android_native_library() {
3737

3838
cmake . -DCMAKE_INSTALL_PREFIX="${CMAKE_OUT}" \
3939
-DCMAKE_TOOLCHAIN_FILE="${ANDROID_NDK}/build/cmake/android.toolchain.cmake" \
40+
-DPYTHON_EXECUTABLE="${PYTHON_EXECUTABLE}" \
4041
--preset "android-${ANDROID_ABI}" \
4142
-DANDROID_PLATFORM=android-26 \
4243
-DEXECUTORCH_ENABLE_EVENT_TRACER="${EXECUTORCH_ANDROID_PROFILING:-OFF}" \
44+
-DEXECUTORCH_ANDROID_PROFILING="${EXECUTORCH_ANDROID_PROFILING:-OFF}" \
4345
-DEXECUTORCH_BUILD_EXTENSION_LLM="${EXECUTORCH_BUILD_EXTENSION_LLM:-ON}" \
4446
-DEXECUTORCH_BUILD_EXTENSION_LLM_RUNNER="${EXECUTORCH_BUILD_EXTENSION_LLM:-ON}" \
4547
-DEXECUTORCH_BUILD_EXTENSION_ASR_RUNNER="${EXECUTORCH_BUILD_EXTENSION_LLM:-ON}" \
@@ -51,6 +53,7 @@ build_android_native_library() {
5153
-DQNN_SDK_ROOT="${QNN_SDK_ROOT}" \
5254
-DEXECUTORCH_BUILD_VULKAN="${EXECUTORCH_BUILD_VULKAN}" \
5355
-DXNNPACK_ENABLE_ARM_SME2="${XNNPACK_ENABLE_ARM_SME2}" \
56+
-DFLATCC_ALLOW_WERROR=OFF \
5457
-DSUPPORT_REGEX_LOOKAHEAD=ON \
5558
-DCMAKE_BUILD_TYPE="${EXECUTORCH_CMAKE_BUILD_TYPE}" \
5659
-B"${CMAKE_OUT}"

third-party/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,7 @@ ExternalProject_Add(
100100
-DFLATCC_TEST=OFF
101101
-DFLATCC_REFLECTION=OFF
102102
-DFLATCC_DEBUG_CLANG_SANITIZE=OFF
103+
-DFLATCC_ALLOW_WERROR=OFF
103104
-DFLATCC_INSTALL=ON
104105
-DCMAKE_POLICY_VERSION_MINIMUM=3.5
105106
-DCMAKE_INSTALL_PREFIX:PATH=<INSTALL_DIR>

tools/cmake/common/preset.cmake

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,12 +82,12 @@ macro(define_overridable_option NAME DESCRIPTION VALUE_TYPE DEFAULT_VALUE)
8282
if(DEFINED ${NAME} AND NOT DEFINED CACHE{${NAME}})
8383
set(${NAME}
8484
${${NAME}}
85-
CACHE ${VALUE_TYPE} ${DESCRIPTION} FORCE
85+
CACHE ${VALUE_TYPE} "${DESCRIPTION}" FORCE
8686
)
8787
else()
8888
set(${NAME}
8989
${DEFAULT_VALUE}
90-
CACHE ${VALUE_TYPE} ${DESCRIPTION}
90+
CACHE ${VALUE_TYPE} "${DESCRIPTION}"
9191
)
9292
endif()
9393

0 commit comments

Comments
 (0)