IssueTracker

Instead of `-assumenosideeffects interface org.slf4j.Logger { ... }`, could you try the following rules?

-assumenosideeffects class * implements org.slf4j.Logger {
  public void info(...);
  ...
}

Any updates on this? Having similar issue. Some of the traces aren't removed with R8.
To be more specific, the log methods are removed, but not the log strings. Decompiling the APK is easy to find orphan log strings.  
In our case we aren't using any third-party library for logging.

Re: comment #3,

That sounds like a different issue. Maybe related to issue 130561746 (which is about -assumevalues, though). Would you mind filing a separate issue for us? In particular, tell us more about log string itself. I guess it's `final static` string whose uses (perhaps logging methods) are removed by -assumenosideeffects but the string itself still remains?

I've tried to reproduce the issue: https://r8-review.googlesource.com/c/r8/+/37283

If a log string is an one-time constant, like "Tag" or the enclosing class's simple name (e.g., via TestClass.class.getSimpleName()), it's removed after its usages, e.g., logger calls, are all removed due to -assumenosideeffects. One case I found is, if that log string is stored to a static field, `static-put` is not removed even though the corresponding field is not read at all (again, due to -assumenosideeffects). Technically, its underlying issue is same as issue 130561746.

Again, it'd be really appreciated if you can share with us how your log strings are set and used.

this is my repo. https://github.com/sollyu/131b2800285b95d5

Thanks for the reproduction.

The problem is that R8 only applies -assumenosideeffects and -assumevalues rules when it can determine that a given invoke is guaranteed to target a single method. In your case, the logging statements lead to invoke-interface instructions, where R8 is unable to determine which method is going to be invoked, and therefore, R8 fails to apply the rules.

Essentially, the question is if the rule `-assumenosideeffects class Logger { public void log(); }` should implicitly also mark all methods that override Logger.log() as having no side effects. In that case, R8 could simply apply the rule to Logger.log()-invocations even when it can't determine which virtual method will be invoked at runtime.

Thank you for taking a look at this on behalf of me. I updated the repro test too.

What can I do now?
现在我能做什么？

While we're working on the fix, as a workaround, you can try rules I mentioned at comment #2.

I tried, but I still didn't work.
我尝试了，但是依然不工作。

Sorry, it was my mistake; yes, we need to revisit -assumenosideeffects evaluation. We're working on it now, and will keep you posted here.

OK, thanks.
好的，辛苦你们了。

Project: r8
Branch: master

commit 88f57ee620305d546ab5efe8ce2a03d2e2028762
Author: Jinseong Jeon <jsjeon@google.com>
Date:   Thu Apr 25 09:26:32 2019

    Add tests for assumenosideeffects with multiple targets.
   
    Bug: 130804193, 70550443
    Change-Id: I62110c3b67047f3e9974aa20735d8bb669ce4ca6

M       src/main/java/com/android/tools/r8/ir/optimize/MemberValuePropagation.java
A       src/test/java/com/android/tools/r8/shaking/assumenosideeffects/AssumenosideeffectsWithMultipleTargetsTest.java

https://r8-review.googlesource.com/37283

How can I use?
我该怎么样使用？

comment #14 adds tests, not a fix. Will work on this, based on that guide line.

Project: r8
Branch: master

commit d45c3e91e8895c73960e7e18475915baa9709d97
Author: Jinseong Jeon <jsjeon@google.com>
Date:   Fri Apr 26 14:20:18 2019

    Use refined receiver type when searching for -assumenosideeffects.
   
    Bug: 130804193
    Change-Id: I4bf42b9968c1cb0d226e68c9665278404b706480

M       src/main/java/com/android/tools/r8/ir/optimize/MemberValuePropagation.java
M       src/test/java/com/android/tools/r8/shaking/assumenosideeffects/AssumenosideeffectsWithMultipleTargetsTest.java

https://r8-review.googlesource.com/37701

Project: r8
Branch: master

commit f97e04c8757d02da5967351d3dfbea8f5b62b5cf
Author: Jinseong Jeon <jsjeon@google.com>
Date:   Mon Apr 29 07:55:41 2019

    Prioritize noSideEffects in side-effect analysis of invocations.
   
    Bug: 130804193, 130561746
    Change-Id: I2469b4f0988d86f54a2e11042ee6af57f67a023d

M       src/main/java/com/android/tools/r8/ir/code/InvokeDirect.java
M       src/main/java/com/android/tools/r8/ir/code/InvokeStatic.java
M       src/main/java/com/android/tools/r8/ir/code/InvokeVirtual.java
M       src/main/java/com/android/tools/r8/ir/optimize/MemberValuePropagation.java
M       src/test/examples/annotationremoval/keep-rules-keep-innerannotation.txt
M       src/test/examples/annotationremoval/keep-rules.txt
M       src/test/examples/naming001/keep-rules-105.txt
M       src/test/java/com/android/tools/r8/accessrelaxation/InvokeTypeConversionTest.java
M       src/test/java/com/android/tools/r8/checkdiscarded/CheckDiscardedTest.java
M       src/test/java/com/android/tools/r8/checkdiscarded/testclasses/UsedClass.java
M       src/test/java/com/android/tools/r8/ir/optimize/string/NameThenLengthTest.java
M       src/test/java/com/android/tools/r8/ir/optimize/unusedarguments/CollisionWithLibraryMethodsTest.java
M       src/test/java/com/android/tools/r8/ir/optimize/unusedarguments/UnusedArgumentRemovalWithOverridingTest.java
M       src/test/java/com/android/tools/r8/naming/applymapping/sourcelibrary/ApplyMappingTest.java
M       src/test/java/com/android/tools/r8/regress/b69825683/Regress69825683Test.java
M       src/test/java/com/android/tools/r8/rewrite/staticvalues/StaticValuesTest.java
M       src/test/java/com/android/tools/r8/shaking/assumevalues/DeadFieldAfterAssumevaluesTest.java

https://r8-review.googlesource.com/37680

Not sure why the submit message for https://r8-review.googlesource.com/c/r8/+/38000 is not shown yet, but the fix is available now. You can update your top-level `build.gradle` file as follows to use that:

buildscript {

    repositories {
        maven {
            url "http://storage.googleapis.com/r8-releases/raw/master"
        }
    }

    dependencies {
        classpath 'com.android.tools:r8:13b0c8484ea241c70d98eb6f55b8f6d65ed2c102'  // Must be before the Gradle Plugin for Android.
        classpath 'com.android.tools.build:gradle:X.Y.Z'     // Your current AGP version.
     }
}

I'm working on cherry-picking it to R8 1.5 and hopefully it's available as part of AS 3.5 beta2.

Note that, instead of rules I mentioned (e.g., comment #2), please stick to the rules you used (comment #1).

It's cherry-picked to R8 1.5 branch, and you can use it as follows:

buildscript {

    repositories {
        maven {
            url 'http://storage.googleapis.com/r8-releases/raw'
        }
    }

    dependencies {
        classpath 'com.android.tools:r8:1.5.25'          // Must be before the Gradle Plugin for Android.
        classpath 'com.android.tools.build:gradle:X.Y.Z' // Your current AGP version.
     }
}

Yes, it worked successfully.
是的，它工作成功。

thank you very much.
非常感谢。

Glad to hear that it works for you! It would be available via AS 3.5 beta2, and around that time, you don't need to specify R8 version, and just use what comes with AGP.