System Properties from gradle.properties are not passed to R8 Gradle Workers in separate processes [380110863]

Fixed

Bug

Status Update

No update yet.

Description

ly...@gmail.com

created issue #1

Nov 21, 2024 10:39AM

Steps to reproduce:

Configure execution profile for R8 and run it in separate process:

android {
    execution {
        profiles {
            create("separate") {
                r8 {
                    jvmOptions += "-Xmx3G -Xms3G".split(" ")
                    runInSeparateProcess = true
                }
            }
            create("inside") {
                r8.runInSeparateProcess = false
            }
        }
        defaultProfile = "separate"
    }
}

Specify whyareyounotinlining option in proguard config
Enable this option by specifying system property in gradle.properties:

systemProp.com.android.tools.r8.experimental.enablewhyareyounotinlining=1

Sample project is attached.

Actual behavior

When R8 runs inside the main Gradle Daemon process, the system property works correctly and build finishes successfully. When R8 runs outside the process, the build failes with:

Caused by: com.android.tools.r8.internal.g: Unknown option "-whyareyounotinlining"

As I understand, the option systemProp.com.android.tools.r8.experimental.enablewhyareyounotinlining=1 wasn't passed to forked process.

Environment:

Gradle version: 8.10.2 AGP version: 8.7.2

MyApplication.zip

98 KB

Download

Comments

je...@google.com <je...@google.com> Nov 23, 2024 04:31AM

Reassigned to hu...@google.com.

hu...@google.com <hu...@google.com> #2Dec 16, 2024 02:12PM

Marked as fixed.

Thank you for the report and reproduction. I can reproduce on a Android TV emulator.

an...@google.com <an...@google.com> #3Dec 23, 2024 09:19PM

Status of current findings.

This is related to the SplitReturnRewriter added in https://r8.googlesource.com/r8/+/0c68ba1dc945da74c2f72085b401fe46635b3fc9.

Not running this code pass on java.lang.Object androidx.collection.LruCache.remove(java.lang.Object) works around the issue. I used the following patch:

diff --git a/src/main/java/com/android/tools/r8/ir/conversion/passes/SplitReturnRewriter.java b/src/main/java/com/android/tools/r8/ir/conversion/passes/SplitReturnRewriter.java
index 157d26c66a..63e0f76064 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/passes/SplitReturnRewriter.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/passes/SplitReturnRewriter.java
@@ -43,6 +43,10 @@ public class SplitReturnRewriter extends CodeRewriterPass<AppInfo> {
 
   @Override
   protected CodeRewriterResult rewriteCode(IRCode code) {
+    if (code.context().toSourceString().equals(
+        "java.lang.Object androidx.collection.LruCache.remove(java.lang.Object)")) {
+      return CodeRewriterResult.NO_CHANGE;
+    }
     int color = colorExceptionHandlers(code);
     AffectedValues affectedValues = new AffectedValues();
     boolean changed = false;

Comparing the DEX with and without this patch shows one instruction diff in void androidx.leanback.widget.GridLayoutManager$2.addItem(java.lang.Object, int, int, int, int) where java.lang.Object androidx.collection.LruCache.remove(java.lang.Object) is inlined into.

Tracing the IR for void androidx.leanback.widget.GridLayoutManager$2.addItem(java.lang.Object, int, int, int, int) for the two versions shows that they start to deviate in second pass of IR after inserting assume instructions (SSA) with

block 29, pred-counts: 1, succ-count: 2, filled: true, sealed: false
predecessors: 28 
successors: 31 30  (no try/catch successors)
no phis
 #143:loadView;1762:addItem: -1: InstanceGet          v67 <- v136; field: androidx.collection.LruCache androidx.leanback.widget.ViewsStateBundle.mChildStates
                           : -1: Assume               v140 <- v67; lower bound: @Nullable androidx.collection.LruCache {}
                           : -1: SafeCheckCast        v131 <- v65; java.lang.String
                           : -1: Invoke-Virtual       v132 <- v140, v131; method: java.lang.Object androidx.collection.LruCache.remove(java.lang.String)
                           : -1: CheckCast            v69 <- v132; android.util.SparseArray
 #144:loadView;1762:addItem: -1: If                   v69 EQZ block 31 (fallthrough 30)

vs.

block 29, pred-counts: 1, succ-count: 2, filled: true, sealed: false
predecessors: 28 
successors: 31 30  (no try/catch successors)
no phis
 #143:loadView;1762:addItem: -1: InstanceGet          v67 <- v136; field: androidx.collection.LruCache androidx.leanback.widget.ViewsStateBundle.mChildStates
                           : -1: Assume               v140 <- v67; lower bound: @Nullable androidx.collection.LruCache {}
                           : -1: SafeCheckCast        v131 <- v65; java.lang.String
                           : -1: Invoke-Virtual       v132 <- v140, v131; method: java.lang.Object androidx.collection.LruCache.remove(java.lang.String)
                           : -1: Assume               v141 <- v132; not null
                           : -1: CheckCast            v69 <- v141; android.util.SparseArray
 #144:loadView;1762:addItem: -1: If                   v69 EQZ block 31 (fallthrough 30)

This is from computeDynamicReturnType concluding a dynamic return type of DynamicTypeWithUpperBound(@NotNull java.lang.Object {}).

Issue 380110863