Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exclude empty starters jars and annotation processors when running or packaging with Maven and Gradle #22036

Closed
philwebb opened this issue Jun 19, 2020 · 18 comments
Assignees
Labels
type: enhancement A general enhancement
Milestone

Comments

@philwebb
Copy link
Member

@odrotbohm brought up the interesting idea that we could simply exclude empty starter JARs from our fat jars. This might help performance when searching for missing classes.

@philwebb philwebb added for: team-attention An issue we'd like other members of the team to review status: waiting-for-triage An issue we've not yet triaged labels Jun 19, 2020
@dreis2211
Copy link
Contributor

Hi,

I tried testing the potential impact on startup times today on a simple app which includes spring-boot-starter-web and spring-boot-starter-security and only one (hello world) controller.

I did so by using the following command to delete the starters from the fat jar:

christoph.dreis@MB-20000266 sessiondemo % zip -d target/sessiondemo-0.0.1-SNAPSHOT.jar 'BOOT-INF/lib/spring-boot-starter-*' 
deleting: BOOT-INF/lib/spring-boot-starter-web-2.3.1.RELEASE.jar
deleting: BOOT-INF/lib/spring-boot-starter-2.3.1.RELEASE.jar
deleting: BOOT-INF/lib/spring-boot-starter-logging-2.3.1.RELEASE.jar
deleting: BOOT-INF/lib/spring-boot-starter-json-2.3.1.RELEASE.jar
deleting: BOOT-INF/lib/spring-boot-starter-tomcat-2.3.1.RELEASE.jar
deleting: BOOT-INF/lib/spring-boot-starter-security-2.3.1.RELEASE.jar

So overall we can get rid of 6 starters here.

Benchmarking this shows the following results compared to the normal baseline fat jar:

Baseline New
2.292 2.177
2.288 2.153
2.220 2.274
2.229 2.098
2.298 2.226
2.233 2.276
2.270 2.124
2.247 2.246
2.251 2.137
2.239 2.196
Mean 2.257 2.191
Range 0.078 0.178

A relatively small, but yet visible impact. On my local machine the benchmark results were ranging between 50-100ms faster startup times for the excluded starter variant.

Hope this helps.
Cheers,
Christoph

@philwebb
Copy link
Member Author

Another thought, we could leave them in the jar but exclude them from the classpath.

@odrotbohm
Copy link
Member

odrotbohm commented Jun 22, 2020

Any particular reason why they should be in the JAR in the first place?

#22030 is probably related. It would allow users to manually exclude them from being packaged easily.

@wilkinsona
Copy link
Member

we could leave them in the jar but exclude them from the classpath

I'd prefer that we exclude them entirely. I think having jars in BOOT-INF/lib that don't end up on the classpath is likely to be confusing. It's simpler, and probably more extensible, if we just exclude them at build time.

@philwebb philwebb added type: enhancement A general enhancement and removed for: team-attention An issue we'd like other members of the team to review status: waiting-for-triage An issue we've not yet triaged labels Jun 22, 2020
@philwebb philwebb added this to the 2.4.x milestone Jun 22, 2020
@dreis2211
Copy link
Contributor

dreis2211 commented Jun 22, 2020

I'm fiddling around with that one and did the following for Gradle:

--- a/spring-boot-project/spring-boot-tools/spring-boot-gradle-plugin/src/main/java/org/springframework/boot/gradle/tasks/bundling/BootArchiveSupport.java
+++ b/spring-boot-project/spring-boot-tools/spring-boot-gradle-plugin/src/main/java/org/springframework/boot/gradle/tasks/bundling/BootArchiveSupport.java
@@ -195,6 +195,7 @@ class BootArchiveSupport {
                if (this.excludeDevtools) {
                        excludes.add("**/spring-boot-devtools-*.jar");
                }
+               excludes.add("**/spring-boot-starter-*.jar");
                this.exclusions.setExcludes(excludes);
        }

The problem that I'm seeing here is that we might exclude starters that are not from Spring-Boot itself, but rather external starters that are prefixed with spring-boot-starter. I would say this is a rather common naming pattern. Unfortunately, we don't have the group information available here to the best of my knowledge in order to narrow that down. Possibly we have to get a list of all spring-boot owned starters here to only exclude those. Thoughts? We probably need this for the maven part anyhow, which I would personally configure in AbstractPackagerMojo.getAdditionalFilters...

In case you generally agree with my approach, I'm happy to work on a PR for that.

@philwebb
Copy link
Member Author

My gut instinct is that we won't be able to use the jar name alone to determine if we can exclude it or not. We probably also need to inspect that jar during the build to find out if it's empty (or nearly empty). We could perhaps add something to the META-INF/MANIFEST.MF file of the starters to do indicate there's nothing of value in it.

@dreis2211
Copy link
Contributor

I'm playing around on the Gradle side of things so far in this branch. Probably tomorrow I'll play around with the Maven part by filtering the JARs in a similar way inside the Repackager. Let me know if this is what you had in mind.

@dreis2211
Copy link
Contributor

dreis2211 commented Jun 23, 2020

I've done the same for Maven now in the mentioned branch. In case you think this is a valid solution I would give this a cleanup and introduce a Jar(File)Utils in spring-boot-loader-tools to reduce some of the copy paste of getting manifest attributes in both Maven & Gradle and open a PR. And give this an actual testing round if it works with mvn package etc.. Just let me know if I should open a PR.

EDIT: Update the branch in place with the JarFileUtils class to reduce at least a bit of copy-paste.

dreis2211 added a commit to dreis2211/spring-boot that referenced this issue Jul 2, 2020
@wilkinsona wilkinsona changed the title Don't include empty starter jars Exclude empty starters jars and annotation processors when running or packaging with Maven and Gradle Sep 10, 2020
@wilkinsona wilkinsona self-assigned this Sep 10, 2020
wilkinsona added a commit that referenced this issue Sep 10, 2020
This commit updates the Maven Plugin to filter dependencies based on
the Spring-Boot-Jar-Type entry in their manifest. Jars with a
Spring-Boot-Jar-Type of dependencies-starter or annotation-processor
are excluded.

See gh-22036
wilkinsona added a commit that referenced this issue Sep 10, 2020
This commit updates the Gradle Plugin to filter dependencies based on
the Spring-Boot-Jar-Type entry in their manifest. Jars with a
Spring-Boot-Jar-Type of dependencies-starter are excluded. Unlike the
Maven plugin, jars with a type of annotation-processor are not
excluded. It is not necessary with Gradle as use of the
annotationProcessor configuration for such dependencies already ensures
that they are not included.

See gh-22036
@wilkinsona
Copy link
Member

Jars with a Spring-Boot-Jar-Type manifest entry with a value of dependencies-starter are now automatically filtered out when the Maven and Gradle plugins are running or packaging an application. The Maven plugin will also filter out jars where the entry's value is annotation-processor. This isn't necessary with Gradle as use of the annotationProcessor configuration already ensures that such dependencies are excluded.

@OrangeDog
Copy link
Contributor

I've just tested this in M4 and it doesn't seem to work. For example both spring-boot-starter-2.4.0-M4.jar and spring-boot-configuration-processor-2.4.0-M4.jar are included.

@dreis2211
Copy link
Contributor

dreis2211 commented Oct 21, 2020

@OrangeDog Are you maybe using Maven and have you switched the maven plugin to M4 as well? I can't reproduce the behaviour with the mentioned version.

@OrangeDog
Copy link
Contributor

As far as I can tell

[INFO] --- spring-boot-maven-plugin:2.4.0-M4:repackage (repackage) @ data-platform ---
[INFO] Replacing main artifact with repackaged archive

@dreis2211
Copy link
Contributor

If I generate a fresh example via https://start.spring.io/ including spring-boot-starter-web and build the jar it doesn't contain any starter. I wonder if you can provide a sample project?

@OrangeDog
Copy link
Contributor

OrangeDog commented Oct 21, 2020

I can't share my current project, but I've removed everything non-standard-looking from the pom and it still includes everything.
I'll try and build an example from scratch.

I've not had much luck working out why, but it also doesn't work on the final 2.4.0 release.

@edwardsre
Copy link
Contributor

edwardsre commented Dec 21, 2020

I can't share my current project, but I've removed everything non-standard-looking from the pom and it still includes everything.
I'll try and build an example from scratch.

I've not had much luck working out why, but it also doesn't work on the final 2.4.0 release.

@OrangeDog @dreis2211 I am having similar issues using 2.4.1. I have two projects which use the same starters. In one project, all starters are included in the runnable jar, in the other project, none are included (as expected). I haven't had much luck figuring out the cause either, and they appear to be configured the same way. It would help if the spring-boot-maven plugin would output some debug information when using mvn -X, but it doesn't show any additional output.

@snicoll
Copy link
Member

snicoll commented Dec 22, 2020

@edwardsre make sure that both projects use version 2.4.x of the Maven Plugin. Unfortunately, it's hard for us to help you without sharing something we can run ourselves. If you manage to build a sample, please create a separate issue.

@edwardsre
Copy link
Contributor

@edwardsre make sure that both projects use version 2.4.x of the Maven Plugin. Unfortunately, it's hard for us to help you without sharing something we can run ourselves. If you manage to build a sample, please create a separate issue.

@snicoll I have verified both projects are using the same version of the plugin 2.4.1. I will try to come up with a sample project to demonstrate it, but it is proving difficult to find the cause.

@edwardsre
Copy link
Contributor

@snicoll I found the problem and created a new issue. #24594

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants