Closed Bug 1540820 Opened 2 years ago Closed 2 years ago

|mach build| rebuilds native code when changing java

Categories

(Firefox Build System :: Android Studio and Gradle Integration, defect, P2)

Unspecified
Android
defect

Tracking

(firefox68 fixed)

RESOLVED FIXED
mozilla68
Tracking Status
firefox68 --- fixed

People

(Reporter: agi, Assigned: nalexander)

References

(Blocks 1 open bug, Regression)

Details

Attachments

(2 files)

Attached file build.txt

STR

  • build geckoview from m-c: ./mach build
  • change a trivial java file, e.g. I added a dummy method:
diff --git a/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/RuntimeTelemetry.java b/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/RuntimeTelemetry.java
index 53f3510ae6ec..338fa8e7fc1b 100644
--- a/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/RuntimeTelemetry.java
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/RuntimeTelemetry.java
@@ -57,4 +57,6 @@ public final class RuntimeTelemetry {
 
         return result;
     }
+
+    public void testChange() {}
 }
  • rebuild
  • observe some native stuff is being rebuilt (on my machine it takes about 5min vs 30sec when rebuilding without changes)

See also attached build log.

Attachment #9054954 - Attachment description: build.log → build.txt
Attachment #9054954 - Attachment filename: build.log → build.txt

My guess from the build log is that we're accidentally touching the generated JNI wrapper files, causing many things to rebuild. N.b., it's not enough to just witness the JNI wrapper task being invoked: we try not to write files if they haven't changed so the task can be invoked and not spark downstream rebuilds. But that's still what I think is happening, and where I would start investigating.

Agi says mach package doesn't trigger a native code rebuild, so we have a workaround.

Or should this bug move to the "Firefox for Android::Build Config & IDE Support" Bugzilla component?

OS: All → Android
Product: GeckoView → Firefox Build System

(In reply to Chris Peterson [:cpeterson] from comment #2)

Agi says mach package doesn't trigger a native code rebuild, so we have a workaround.

It's kind of an accident that mach package triggers Java code rebuild at all, but... shrug.

Or should this bug move to the "Firefox for Android::Build Config & IDE Support" Bugzilla component?

Yeah. I just dug into this a little; it's definitely writing JNI wrapper files when it shouldn't. I'm going to go find out why :)

Assignee: nobody → nalexander
Status: NEW → ASSIGNED
Component: General → Build Config & IDE Support
Product: Firefox Build System → Firefox for Android

This was fallout from Bug 1509572, which moved the "invalidation smarts" to Gradle. Unfortunately, it's not smart enough: there are many situations where the annotations might change (a new method) but where they don't actually change (a new method that isn't annotated with @JNITarget).

Since we don't want to spend the time to make the "invalidation smarts" truly smart, we need to bring back a little bit of Bug 1509572. Patch inbound.

Regressed by: 1509572

This was fallout from Bug 1509572, which moved the "invalidation
smarts" to Gradle. Unfortunately, those smarts are not smart enough:
there are many situations where the annotations might change (a new
method) but where they don't actually change (a new method that isn't
annotated with @JNITarget).

Since we don't want to spend the time to make the "invalidation
smarts" truly smart, we need to bring back this little bit of Bug
1509572.

While we're here, we ensure that there is only one JNI wrapper
generation task for GeckoView and Fennec, regardless of variant.
Right now, those are named like:

  • geckoview:generateJNIWrappersForGeneratedWithGeckoBinariesDebug
  • app:generateJNIWrappersForFennecWithoutGeckoBinariesDebug

See https://bugzilla.mozilla.org/show_bug.cgi?id=1509539#c1 for some
discussion of these JNI wrapper generation tasks.

agi: some local testing would be appreciated. Works fine for me, but could you exercise things for a day as you review? Thanks!

Flags: needinfo?(agi)

From what I can see it works :) Thanks nalexander! I'll keep using it for a day and then approve the code review.

Flags: needinfo?(agi)

Marking as P2 per triage.

Priority: -- → P2
Pushed by nalexander@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/744aec2e16db
Don't write generated JNI wrappers for every Java-level change. r=agi
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 68
Product: Firefox for Android → Firefox Build System
Target Milestone: Firefox 68 → mozilla68
You need to log in before you can comment on or make changes to this bug.