Skip to content

Android: compile shared platform-neutral Java sources in place (step 1 of desktop JVM support) - #22338

Open
RanjithRagavan wants to merge 4 commits into
pytorch:mainfrom
RanjithRagavan:shared-java-sources
Open

Android: compile shared platform-neutral Java sources in place (step 1 of desktop JVM support)#22338
RanjithRagavan wants to merge 4 commits into
pytorch:mainfrom
RanjithRagavan:shared-java-sources

Conversation

@RanjithRagavan

Copy link
Copy Markdown
Contributor

Summary

Step 1 of the desktop JVM plan discussed in #20761, implementing @psiddh's shared-source design: the platform-neutral Java/Kotlin API sources move to a neutral location and are compiled in place by each platform artifact, instead of shipping a shared runtime jar.

flowchart TD
    S["extension/java/src/main/java — 18 platform-neutral sources<br/>no android.* imports · Log facade by simple name · no Log definition here"]
    A["Android AAR (self-contained)<br/>shared classes compiled in · Log.kt delegates to android.util.Log<br/>Java 11 · one fbjni dep · no manifest provider"]
    J["Desktop JVM jar — step 2, not this PR<br/>same classes · console Log · per-OS native jars"]
    S -->|"compiled via java.srcDirs"| A
    S -.->|"compiled via kotlin.srcDirs"| J
Loading

What changed

  • 18 platform-neutral sources (Module, Tensor, EValue, DType, ExecuTorchRuntime, LlmModule, AsrModule, training, annotations) moved extension/android/executorch_android/src/main/javaextension/java/src/main/java via git mv (blame/history preserved).
  • executorch_android/build.gradle: main source set adds java.srcDirs += ['../../java/src/main/java']; spotless target extended so ktfmt still covers the moved files.
  • Tensor.kt drops its android.util.Log import and calls the org.pytorch.executorch.Log facade by simple name (same package). The Android module compiles the one-file implementation Log.kt, delegating to android.util.Log — logcat tags/priorities/messages unchanged. The shared directory contains no Log definition, so there is exactly one implementation per artifact (no duplicate-class collision). ImageProcessor stays in the Android module (android.graphics.Bitmap).
  • extension/android/BUCK: targets keep their historical names, ownership, and attributes (required_for_source_only_abi, pure_kotlin, deps); srcs now reference the shared location.
  • extension/java/README.md documents the sharing contract; docs link updated.

Android shipping contract — preserved

  • Self-contained AAR · Java 11 bytecode · exactly one fbjni dependency · existing logging and native-loading behavior · no new manifest provider · no mutable cross-platform loader configuration.

Verification (run locally, macOS arm64, NDK 26.1, API 34 emulator)

Gate Result
AAR before/after diff identical entries; only delta: one internal Log.class; manifest/POM/natives unchanged; bytecode major 55 (Java 11) both
Unit tests (debug + release) 61/61 pass
Platform-neutrality smoke: shared sources compiled on pure desktop classpath (JDK 17 + fbjni-java-only + soloader, no android.jar) + same unit tests 61/61 pass
Instrumentation on emulator (native lib rebuilt from this branch; real model artifacts) 136 tests across 12 classes; 131 pass; 5 failures are error-message-text assertions reproduced byte-identically on unmodified main (pre-existing upstream test/native skew)
spotless/ktfmt clean

Note: extension/android/BUCK srcs referencing ../java/src/main/java/... needs internal Buck/FoA validation — @psiddh kindly offered to own that side.

Next steps after this lands (per the agreed sequencing in #20761): desktop JVM jar implementation, then native classifier publication.

…1 of desktop JVM support)

Move the platform-neutral Java/Kotlin API sources (Module, Tensor, EValue,
DType, LlmModule, AsrModule, training, ...) from
extension/android/executorch_android/src/main/java to a new shared source
directory extension/java/src/main/java, preserving git history via renames.

The Android AAR stays self-contained and behavior-identical: it compiles
the shared sources directly via java.srcDirs, so there is no new published
artifact, no new transitive dependency, no manifest change, and bytecode
remains at Java 11. Verified locally by building release AARs before and
after the move: the only classes.jar delta is one new internal class,
org.pytorch.executorch.Log.

Logging: Tensor.kt previously imported android.util.Log directly. Shared
sources now reference an org.pytorch.executorch.Log facade by simple name;
each platform artifact compiles exactly one implementation. The Android
implementation (new Log.kt in the Android module) delegates to
android.util.Log, so logcat tags, priorities, and messages are unchanged.
The shared directory deliberately contains no Log definition, avoiding
duplicate-class collisions.

Buck targets in extension/android/BUCK keep their historical names,
ownership, and attributes; their srcs now reference the shared location.

This is step 1 of the plan discussed in pytorch#20761: land the behavior-
preserving shared-source extraction first, then add the desktop JVM jar
and native classifier publication on top.
@pytorch-bot

pytorch-bot Bot commented Aug 31, 2026

Copy link
Copy Markdown

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/22338

Note: Links to docs will display an error until the docs builds have been completed.

⚠️ 15 Awaiting Approval

As of commit 3411d2c with merge base 80a07e6 (image):

AWAITING APPROVAL - The following workflows need approval before CI can run:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Aug 31, 2026
@RanjithRagavan

Copy link
Copy Markdown
Contributor Author

@pytorchbot label "release notes: none"

@kirklandsign

Copy link
Copy Markdown
Contributor

@claude review

@claude

claude Bot commented Sep 1, 2026

Copy link
Copy Markdown

Claude finished @kirklandsign's task in 7m 0s —— View job


Code Review — Android shared Java sources (step 1)

  • Read PR diff and changed files
  • Analyze build.gradle / BUCK / source moves
  • Verify no dangling references to old path
  • Post review

Overall this is a clean, well-scoped refactor. The git mv preserves history, the shared sources are genuinely platform-neutral (verified: no android.* / java.awt.* imports remain under extension/java/src/main/java), and the Log facade split is a tidy way to keep the AAR self-contained while removing the android.util.Log hard dependency from shared code. The single Log.class delta in the AAR diff is consistent with what the code does. Nice work.

A few observations, mostly minor:

1. BUCK explicit list vs. Gradle glob — maintainability hazard (worth noting)

The two build systems now consume the shared directory differently:

  • Gradle (build.gradle:49) globs the whole tree: java.srcDirs += ['../../java/src/main/java'] — any new .kt file is picked up automatically.
  • BUCK (BUCK:19-28) lists each shared source explicitly by path.

Consequence: when someone adds a new shared source, the OSS Gradle AAR compiles it (CI stays green) but the internal Buck build silently omits it until the srcs list is hand-updated. The PR already flags that the Buck side needs internal validation — this divergence is the concrete reason. Not a blocker, but worth a comment in the BUCK header (the existing NOTE is a good spot) so future contributors know both places must be touched.

2. Gradle globs ASR/LLM/training that BUCK's executorch target doesn't

The Gradle java.srcDirs glob pulls in all shared subpackages (asr, llm, training) into the single AAR, whereas BUCK splits them across executorch / executorch_training / executorch_llama targets. This matches pre-PR behavior (those files were already under the module's default src/main/java), so no regression — just confirming it's intentional and the AAR contents are unchanged aside from Log.class.

3. Log facade surface is minimal

Log.kt exposes v/d/i/w/e (+ throwable overloads for w/e). Shared code today only calls Log.e(tag, msg) (single use in Tensor.kt:439), so this is sufficient. Just flagging that when step 2 lands the desktop implementation, the two facades must keep identical signatures or shared code that adds a new log call will compile on one platform and not the other. Since Log is internal, that mismatch is compile-time only — acceptable.

4. Verified: no other references to the old path

Grepped the tree — the only reference to the old executorch_android/.../org/pytorch/executorch API path outside of code was the docs link, which this PR updates (using-executorch-android.md:23). No build scripts, CI YAML, or CMake reference the moved files. ImageProcessor* correctly stays in the Android module (depends on android.graphics.Bitmap). Good.

Nit

  • Consider adding a one-line pointer in extension/java/README.md that the BUCK srcs list must be updated in lockstep (ties into Add support for quantized LeakyReLU #1) — the README currently documents the source contract but not the dual-build maintenance rule.

No correctness issues found. The internal object Log visibility works because Tensor.kt and Log.kt compile in the same module (both Gradle and the executorch Buck target). LGTM pending the internal Buck validation you already called out.
· shared-java-sources

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. release notes: none Do not include this in the release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants