Skip to content

Commit c4e325d

Browse files
authored
Android code quality improvements (#17636)
- Fix check-after-use bug in JNI scalar type lookup: move `.count()` check before `.at()` so unsupported scalar types throw a Java exception instead of C++ `std::out_of_range` - Fix `initHybrid` → `numThreads` parameter name in `Module.java` native declaration - Fix missing space in `getMethodMetadata()` error message - Modernize build config: Java 11 source/target, remove dead root ext properties (minSdkVersion, targetSdkVersion, compileSdkVersion, buildToolsVersion), update test dependencies (junit 4.13.2, androidx.test.ext:junit 1.2.1, androidx.test:rules 1.6.1, commons-io 2.18.0) ## Test plan - CI (existing unit + instrumentation tests cover the JNI and Module paths) - Build config changes are compile-time only; verified locally that Gradle sync succeeds
1 parent bf0cd1c commit c4e325d

4 files changed

Lines changed: 16 additions & 17 deletions

File tree

extension/android/build.gradle

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,6 @@
11
allprojects {
22
buildscript {
33
ext {
4-
minSdkVersion = 21
5-
targetSdkVersion = 34
6-
compileSdkVersion = 34
7-
buildToolsVersion = '33.0.1'
8-
94
fbjniJavaOnlyVersion = "0.7.0"
105
soLoaderNativeLoaderVersion = "0.10.5"
116
}

extension/android/executorch_android/build.gradle

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,8 @@ android {
3636
}
3737

3838
compileOptions {
39-
sourceCompatibility = JavaVersion.VERSION_1_8
40-
targetCompatibility = JavaVersion.VERSION_1_8
39+
sourceCompatibility = JavaVersion.VERSION_11
40+
targetCompatibility = JavaVersion.VERSION_11
4141
}
4242

4343
sourceSets {
@@ -49,7 +49,7 @@ android {
4949
}
5050
}
5151
kotlinOptions {
52-
jvmTarget = "1.8"
52+
jvmTarget = "11"
5353
}
5454
}
5555

@@ -61,12 +61,12 @@ dependencies {
6161
implementation 'com.facebook.fbjni:fbjni:0.7.0'
6262
implementation 'com.facebook.soloader:nativeloader:0.10.5'
6363
implementation libs.core.ktx
64-
testImplementation 'junit:junit:4.12'
64+
testImplementation 'junit:junit:4.13.2'
6565
testImplementation 'org.assertj:assertj-core:3.27.2'
6666
testImplementation 'org.jetbrains.kotlin:kotlin-test:1.9.23'
67-
androidTestImplementation 'androidx.test.ext:junit:1.1.5'
68-
androidTestImplementation 'androidx.test:rules:1.2.0'
69-
androidTestImplementation 'commons-io:commons-io:2.4'
67+
androidTestImplementation 'androidx.test.ext:junit:1.2.1'
68+
androidTestImplementation 'androidx.test:rules:1.6.1'
69+
androidTestImplementation 'commons-io:commons-io:2.18.0'
7070
androidTestImplementation 'org.json:json:20250107'
7171
androidTestImplementation 'org.jetbrains.kotlin:kotlin-test:1.9.23'
7272
if (qnnVersion) {

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ public class Module {
5454

5555
@DoNotStrip
5656
private static native HybridData initHybrid(
57-
String moduleAbsolutePath, int loadMode, int initHybrid);
57+
String moduleAbsolutePath, int loadMode, int numThreads);
5858

5959
private Module(String moduleAbsolutePath, int loadMode, int numThreads) {
6060
ExecuTorchRuntime runtime = ExecuTorchRuntime.getRuntime();
@@ -201,7 +201,7 @@ public int loadMethod(String methodName) {
201201
*/
202202
public MethodMetadata getMethodMetadata(String name) {
203203
if (!mMethodMetadata.containsKey(name)) {
204-
throw new RuntimeException("method " + name + "does not exist for this module");
204+
throw new RuntimeException("method " + name + " does not exist for this module");
205205
}
206206

207207
MethodMetadata methodMetadata = mMethodMetadata.get(name);

extension/android/jni/jni_layer.cpp

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,14 +58,15 @@ class TensorHybrid : public facebook::jni::HybridClass<TensorHybrid> {
5858
// Java wrapper currently only supports contiguous tensors.
5959

6060
const auto scalarType = tensor.scalar_type();
61-
int jdtype = scalar_type_to_java_dtype.at(scalarType);
6261
if (scalar_type_to_java_dtype.count(scalarType) == 0) {
6362
std::stringstream ss;
64-
ss << "executorch::aten::Tensor scalar [java] type: " << jdtype
65-
<< " is not supported on java side";
63+
ss << "executorch::aten::Tensor scalar type "
64+
<< static_cast<int>(scalarType) << " is not supported on java side";
6665
jni_helper::throwExecutorchException(
6766
static_cast<uint32_t>(Error::InvalidArgument), ss.str().c_str());
67+
return nullptr;
6868
}
69+
int jdtype = scalar_type_to_java_dtype.at(scalarType);
6970

7071
const auto& tensor_shape = tensor.sizes();
7172
std::vector<jlong> tensor_shape_vec;
@@ -131,6 +132,7 @@ class TensorHybrid : public facebook::jni::HybridClass<TensorHybrid> {
131132
ss << "Unknown Tensor jdtype: [" << jdtype << "]";
132133
jni_helper::throwExecutorchException(
133134
static_cast<uint32_t>(Error::InvalidArgument), ss.str().c_str());
135+
return nullptr;
134136
}
135137
ScalarType scalar_type = java_dtype_to_scalar_type.at(jdtype);
136138
const jlong dataCapacity = jni->GetDirectBufferCapacity(jbuffer.get());
@@ -139,6 +141,7 @@ class TensorHybrid : public facebook::jni::HybridClass<TensorHybrid> {
139141
ss << "Tensor buffer is not direct or has invalid capacity";
140142
jni_helper::throwExecutorchException(
141143
static_cast<uint32_t>(Error::InvalidArgument), ss.str().c_str());
144+
return nullptr;
142145
}
143146
const size_t elementSize = executorch::runtime::elementSize(scalar_type);
144147
const jlong expectedElements = static_cast<jlong>(numel);
@@ -153,6 +156,7 @@ class TensorHybrid : public facebook::jni::HybridClass<TensorHybrid> {
153156
<< " (element size bytes: " << elementSize << ")";
154157
jni_helper::throwExecutorchException(
155158
static_cast<uint32_t>(Error::InvalidArgument), ss.str().c_str());
159+
return nullptr;
156160
}
157161
return from_blob(
158162
jni->GetDirectBufferAddress(jbuffer.get()), shape_vec, scalar_type);

0 commit comments

Comments
 (0)