Add version of UnusedTracker that supports full shrinking using R8. #566

netomi · 2020-04-15T12:54:48Z

Pull request for #565 .

netomi · 2020-04-15T13:08:16Z

There are some test failures wrt to exclude settings. I have to look into them, so far I did not take these settings into account.

netomi · 2020-04-15T14:26:43Z

I have fixed the test failures. There is one remaining but this one can not be easily solved when using a full shrinker. The test setup is as follows:

an unused class in an api dependency is implementing an interface from a transitive dependency. This interface is never used or accessed, thus the interface is removed, leaving no trace of it. The test wants to ensure that the interface is not removed (LibEntity). Cases like that are normally not handled by a shrinker, it usually can safely remove such interfaces. If the test would be slightly adapted to a more real world example (LibEntity would be used somehow in the otherwise UnusedEntity), the interface would not be removed and the test would succeed.

Edit: I have updated the last failing test slightly to take the shrinking capabilities of R8 into account.

src/main/groovy/com/github/jengelman/gradle/plugins/shadow/internal/UnusedTrackerWithR8.groovy

build.gradle

gradle/dependencies.gradle

covers1624 · 2020-08-17T07:38:56Z

Is there any update on this PR? I am interested in using this functionality.

bowsersenior · 2021-06-07T21:08:51Z

It would be nice if we could have some feedback on this PR. It seems very useful from my team's needs.

johnrengelman · 2021-08-08T22:12:48Z

src/main/groovy/com/github/jengelman/gradle/plugins/shadow/tasks/ShadowJar.java

@@ -98,7 +98,7 @@ public InheritManifest getManifest() {
    @Override
    protected CopyAction createCopyAction() {
        DocumentationRegistry documentationRegistry = getServices().get(DocumentationRegistry.class);
-        final UnusedTracker unusedTracker = minimizeJar ? UnusedTracker.forProject(getProject(), configurations, dependencyFilterForMinimize) : null;
+        final UnusedTracker unusedTracker = minimizeJar ? UnusedTrackerWithR8.forProject(getProject(), configurations, dependencyFilterForMinimize) : null;


That choice of using the standard tracker vs the r8 tracker needs to be configurable by the user.
A couple of options here:

Add an useR8 = true option that works in conjunction with minimizeJar to configure the implementation.

Detect the presence of the R8 classes on the classpath and then use the R8 implementation if present.

johnrengelman · 2021-08-08T22:14:33Z

gradle/dependencies.gradle

@@ -15,6 +15,8 @@ dependencies {
        exclude group: 'org.ow2.asm'
    }

+    implementation 'com.android.tools:r8:2.0.61'


I think we should make this compileOnly and force the user to add their only r8 dependency so we don't have to maintain the compatibility. This should happened regardless of the extension implementation in the comment below.

johnrengelman · 2021-08-08T22:15:50Z

src/main/groovy/com/github/jengelman/gradle/plugins/shadow/internal/UnusedTracker.groovy

        this.toMinimize = toMinimize
        projectUnits = classDirs.collect { cp.addClazzpathUnit(it) }
        projectUnits.addAll(classJars.collect { cp.addClazzpathUnit(it) })
    }

+    boolean performsFullShrinking() {


None of these methods are necessary in this class since they aren't used by it. If they are only needed by the R8 implementation, then they should reside in that class.

johnrengelman · 2021-08-08T22:16:34Z

src/main/groovy/com/github/jengelman/gradle/plugins/shadow/internal/UnusedTrackerWithR8.groovy

+ * It is an extension of the existing UnusedTracker to make it easy to exchange them,
+ * if the pull request is accepted, a common base interface should be extracted.
+ */
+class UnusedTrackerWithR8 extends UnusedTracker {


We could just write this in pure Java in the src/main/java directory if this is a concern.

johnrengelman · 2021-08-08T22:18:20Z

src/main/groovy/com/github/jengelman/gradle/plugins/shadow/internal/UnusedTrackerWithR8.groovy

+    @CompileStatic
+    static void addJDKLibrary(BaseCommand.Builder builder) {
+        String JAVA_HOME = System.getenv("JAVA_HOME")
+        builder.addLibraryResourceProvider(JdkClassFileProvider.fromJdkHome(Paths.get(JAVA_HOME)))


I'm not sure we can/should rely on this. Perhaps this needs to be exposed to the user directly? We could maybe figure out a default setting based on the configuration of the compileMain task or somethign.

johnrengelman · 2021-08-08T22:19:51Z

src/test/groovy/com/github/jengelman/gradle/plugins/shadow/ShadowPluginSpec.groovy

@@ -495,12 +495,12 @@ class ShadowPluginSpec extends PluginSpecification {

        file('lib/src/main/java/lib/LibEntity.java') << """
            package lib;
-            public interface LibEntity {}
+            public interface LibEntity { void simple(); }


we should follow appropriate Java styling and put this on a new line for ease of reading. Same for all the changes in this file.

nicolabeghin · 2022-05-09T09:42:05Z

Any update on this PR? seems really promising! thanks

netomi · 2022-10-11T05:26:26Z

Somehow I missed that changes have been requested to this PR which make total sense and I would be willing to work on them.
In the meantime I worked on my own bytecode manipulation library including a shrinker: https://github.com/netomi/bat and I created a fork of the shadow plugin to use it for relocation and shrinking (still to be done): https://github.com/netomi/gradle-shadow

It takes the current shadow plugin, converts it to kotlin only and uses my bat library for the byte code stuff. Its still very early work, but I will come back to this PR and make it clean to be included. @johnrengelman

netomi · 2022-10-11T10:32:26Z

When I sync'd my PR branch with latest master and reworked the PR the PR was closed as a consequence. I am not sure how to fix that, but I created a new PR at #796 , its a pity that the comments and suggestions are lost somehow, but I hope that suffices.

sgjesse reviewed Apr 15, 2020

View reviewed changes

src/main/groovy/com/github/jengelman/gradle/plugins/shadow/internal/UnusedTrackerWithR8.groovy Outdated Show resolved Hide resolved

sgjesse reviewed Apr 15, 2020

View reviewed changes

src/main/groovy/com/github/jengelman/gradle/plugins/shadow/internal/UnusedTrackerWithR8.groovy Outdated Show resolved Hide resolved

sgjesse reviewed Apr 15, 2020

View reviewed changes

src/main/groovy/com/github/jengelman/gradle/plugins/shadow/internal/UnusedTrackerWithR8.groovy Outdated Show resolved Hide resolved

ZacSweers reviewed Apr 15, 2020

View reviewed changes

build.gradle Outdated Show resolved Hide resolved

gradle/dependencies.gradle Outdated Show resolved Hide resolved

netomi mentioned this pull request May 31, 2020

JAR minimization #198

Closed

gildor mentioned this pull request Aug 24, 2020

Support a full shrinking mode in for UnusedTracker using R8 #565

Closed

johnrengelman requested changes Aug 8, 2021

View reviewed changes

johnrengelman added this to the 8.0 milestone Aug 8, 2021

netomi closed this Oct 11, 2022

netomi force-pushed the minimize-with-r8 branch from 0f32aea to 3844331 Compare October 11, 2022 06:38

netomi mentioned this pull request Oct 11, 2022

Add version of UnusedTracker that supports full shrinking using R8. #796

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add version of UnusedTracker that supports full shrinking using R8. #566

Add version of UnusedTracker that supports full shrinking using R8. #566

netomi commented Apr 15, 2020

netomi commented Apr 15, 2020

netomi commented Apr 15, 2020 •

edited

Loading

covers1624 commented Aug 17, 2020

bowsersenior commented Jun 7, 2021

johnrengelman Aug 8, 2021

johnrengelman Aug 8, 2021

johnrengelman Aug 8, 2021

johnrengelman Aug 8, 2021

johnrengelman Aug 8, 2021

johnrengelman Aug 8, 2021

nicolabeghin commented May 9, 2022

netomi commented Oct 11, 2022

netomi commented Oct 11, 2022

Add version of UnusedTracker that supports full shrinking using R8. #566

Add version of UnusedTracker that supports full shrinking using R8. #566

Conversation

netomi commented Apr 15, 2020

netomi commented Apr 15, 2020

netomi commented Apr 15, 2020 • edited Loading

covers1624 commented Aug 17, 2020

bowsersenior commented Jun 7, 2021

johnrengelman Aug 8, 2021

Choose a reason for hiding this comment

johnrengelman Aug 8, 2021

Choose a reason for hiding this comment

johnrengelman Aug 8, 2021

Choose a reason for hiding this comment

johnrengelman Aug 8, 2021

Choose a reason for hiding this comment

johnrengelman Aug 8, 2021

Choose a reason for hiding this comment

johnrengelman Aug 8, 2021

Choose a reason for hiding this comment

nicolabeghin commented May 9, 2022

netomi commented Oct 11, 2022

netomi commented Oct 11, 2022

netomi commented Apr 15, 2020 •

edited

Loading