Deduplicate file validation and UTF-8 checking utilities#17648
Deduplicate file validation and UTF-8 checking utilities#17648kirklandsign merged 3 commits intomainfrom
Conversation
Java: Extract validateFilePath() into ExecuTorchRuntime and replace inline checks in Module, LlmModule, and TrainingModule. This also removes the "!!" typo in TrainingModule error messages. C++: Move utf8_check_validity() from anonymous namespaces in jni_layer_llama.cpp and jni_layer_asr.cpp into the shared jni_helper.h/.cpp.
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/17648
Note: Links to docs will display an error until the docs builds have been completed. ❌ 3 New Failures, 35 PendingAs of commit 16a2501 with merge base c4e325d ( NEW FAILURES - The following jobs have failed:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
This PR needs a
|
There was a problem hiding this comment.
Pull request overview
This PR deduplicates common validation utilities across the Android Java API surface and Android JNI C++ layer by centralizing file-path validation in ExecuTorchRuntime and UTF-8 validity checking in jni_helper.
Changes:
- Java: Added
ExecuTorchRuntime.validateFilePath()and replaced duplicatedjava.io.Filereadability checks inModule,LlmModule, andTrainingModule(also removing the “!!” typo inTrainingModuleerrors). - C++: Moved
utf8_check_validity()from per-file anonymous namespaces into sharedjni_helper.h/.cppand updated JNI layers to reuse it. - JNI: Added
jni_helper.hinclude where needed and imported the shared UTF-8 validator.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| extension/android/jni/jni_layer_llama.cpp | Replaces local UTF-8 validator with shared jni_helper implementation. |
| extension/android/jni/jni_layer_asr.cpp | Includes jni_helper.h and switches to shared UTF-8 validator. |
| extension/android/jni/jni_helper.h | Declares shared utf8_check_validity() (adds required includes). |
| extension/android/jni/jni_helper.cpp | Defines shared utf8_check_validity() implementation. |
| extension/android/executorch_android/src/main/java/org/pytorch/executorch/training/TrainingModule.java | Uses centralized file validation and removes duplicated checks/typo. |
| extension/android/executorch_android/src/main/java/org/pytorch/executorch/extension/llm/LlmModule.java | Uses centralized file validation instead of inline File checks. |
| extension/android/executorch_android/src/main/java/org/pytorch/executorch/Module.java | Uses centralized file validation instead of inline File checks. |
| extension/android/executorch_android/src/main/java/org/pytorch/executorch/ExecuTorchRuntime.java | Adds validateFilePath() utility for reuse across modules. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| String moduleAbsolutePath, int loadMode, int numThreads); | ||
|
|
||
| private Module(String moduleAbsolutePath, int loadMode, int numThreads) { | ||
| ExecuTorchRuntime runtime = ExecuTorchRuntime.getRuntime(); |
There was a problem hiding this comment.
ExecuTorchRuntime runtime = ExecuTorchRuntime.getRuntime(); is assigned but never used. Consider removing the local variable (and the call if it’s no longer needed) to avoid unused-variable warnings and reduce redundant initialization work.
…/executorch/extension/llm/LlmModule.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
Java: Extract validateFilePath() into ExecuTorchRuntime and replace inline checks in Module, LlmModule, and TrainingModule. This also removes the "!!" typo in TrainingModule error messages.
C++: Move utf8_check_validity() from anonymous namespaces in jni_layer_llama.cpp and jni_layer_asr.cpp into the shared jni_helper.h/.cpp.
Test plan
CI