sargue / java-time-jsptags

JSP tag support for Java 8 java.time (JSR-310)
Apache License 2.0
44 stars 9 forks source link

Do not include jsp-api as it slows down your server dramatically #14

Closed kicktipp closed 7 months ago

kicktipp commented 7 months ago

In this project you can test the performance bug:

https://github.com/kicktipp/jspel/

In your gradle.build you reference jsp-api and jstl-api as dependencies:

api(group: "jakarta.servlet.jsp", name: "jakarta.servlet.jsp-api", version: "3.0.0")
api(group: "jakarta.servlet.jsp.jstl", name: "jakarta.servlet.jsp.jstl-api", version: "2.0.0")

This triggers the follwoing dependency graph in our project:

+--- net.sargue:java-time-jsptags:2.0.0
|    +--- jakarta.servlet.jsp:jakarta.servlet.jsp-api:3.0.0
|    \--- jakarta.servlet.jsp.jstl:jakarta.servlet.jsp.jstl-api:2.0.0 -> 3.0.0
|         +--- jakarta.servlet:jakarta.servlet-api:6.0.0
|         \--- jakarta.el:jakarta.el-api:5.0.0

As we use Spring Boot and embedded tomcat we end up with two versions of jsp-api on the classpath. The package tomcat-embed-jasper-10.1.19.jar doesn't have it as a dependency but has it packaged inside its jar

This way we get two different ScopedAttributeELResolver on our classpath which leads to huge performance issues. This is described here in every detail:

For our large project it is a difference between managing 1.000 req/s or 10.000 req/s. A single request is often 3ms slower if the jakarta.servlet-jsp-api is on the classpath.

A simple workaround for us is to exclude this dependency:

    implementation("net.sargue:java-time-jsptags:2.0.0") {
        exclude(group = "jakarta.servlet.jsp", module = "jakarta.servlet.jsp-api")
    }

I think you should change the dependency form api to implementation as described here: https://docs.gradle.org/current/userguide/java_library_plugin.html

sargue commented 7 months ago

Thanks for your report.

According to the same link you point out (https://docs.gradle.org/current/userguide/java_library_plugin.html#sec:java_library_recognizing_dependencies) this should use the api dependency type:

So when should you use the api configuration? An API dependency is one that contains at least one type that is exposed in the library binary interface, often referred to as its ABI (Application Binary Interface). This includes, but is not limited to:

types used in super classes or interfaces

types used in public method parameters, including generic parameter types (where public is something that is visible to compilers. I.e. , public, protected and package private members in the Java world)

types used in public fields

public annotation types

The JSP and JSTL types are indeed used in public classes and methods of the library.

Still, I understand this is messing with dependencies. So I agree this is justified. I will publish a new version as soon as possible changing the dependencies type.

sargue commented 7 months ago

I've published version 2.0.1 with the dependency type change.

Could you please test on your side to check if the issue is solved?

kicktipp commented 7 months ago

This didn't help. Sorry. I am not very firm with these gradle dependencies. The compileClasspath and runtimeClasspath are the same with 2.0.1 and 2.0.0

So I guess you have to declare it with compileOnly. The JSP Api must be on the classpath already if you are using a tag lib. Its similar to mavens "provided".

More about compileOnly here https://blog.gradle.org/introducing-compile-only-dependencies

We have the third use case, I guess.

sargue commented 7 months ago

Ok, let's try that.

I pushed version 2.0.2 with the change. It will take several hours for Sonatype to publish to Maven Central.

kicktipp commented 7 months ago

That's it. The transitive dependency to jsp is no gone. Thank you very much.

sargue commented 7 months ago

You're welcome. I'm glad it's solved.