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

Deprecate Java 8 support and move to Java 11 #626

Closed
artursouza opened this issue Sep 10, 2021 · 13 comments
Closed

Deprecate Java 8 support and move to Java 11 #626

artursouza opened this issue Sep 10, 2021 · 13 comments
Assignees

Comments

@artursouza
Copy link
Member

Describe the proposal

Since we already have multiple releases of this SDK supporting Java 8, can we migrate to Java 11 as the minimum JDK version?

This might allow us to migrate to the native HTTP client and move out of OKHTTP, for example.
Also, we might consider adopting newer features available in the newer Java language version.

Please, vote with 👍 or 👎

@artursouza artursouza pinned this issue Sep 10, 2021
@mthmulders
Copy link
Contributor

In a few days from now, Java 8 will officially be two LTS releases behind. I think the case of replacing OkHttp with the native HTTP client is a good example of the "price" we would pay if we decide to keep on supporting Java 8.

I would be in favour of investigating if the native HTTP client is a suitable replacement for OkHttp. If so, I would vote to drop Java 8 support and set 11 as our new baseline. It's a big move, but we're not alone: Spring 6 will even have Java 17 as its baseline.

A nice additional benefit we can get from this move is that it would also reduce our dependency footprint. This is what we currently add to any project that wants to use the Dapr SDK for Java, because of our dependency on OkHttp:

  \- com.squareup.okhttp3:okhttp:jar:4.9.0:compile
     +- com.squareup.okio:okio:jar:2.8.0:compile
     |  \- org.jetbrains.kotlin:kotlin-stdlib-common:jar:1.4.0:compile
     \- org.jetbrains.kotlin:kotlin-stdlib:jar:1.4.10:compile
        \- org.jetbrains:annotations:jar:13.0:compile

@johnou
Copy link

johnou commented Sep 11, 2021

Have you run any relevant benchmarks against native HTTP client and OKHTTP?

@artursouza
Copy link
Member Author

Have you run any relevant benchmarks against native HTTP client and OKHTTP?

No. This replacement would be a follow up on this issue. This would be good data to have to decide on replacing it. If there is a performance penalty, might be worth keeping the OkHTTP dependency.

@mthmulders
Copy link
Contributor

Have you run any relevant benchmarks against native HTTP client and OKHTTP?
What kind of setup would we need to gather those benchmarks?

How about this?

  1. a default nginx setup that serves a static response (can even be straight from memory) so we have as little delay on the "server" as possible
  2. run ab to have an idea of how fast that webserver can go
  3. run an OkHttp-based Java program to measure how fast that client is
  4. run an JDK 11 HTTP Client based Java program to measure how fast that client is
  5. compare 2, 3 and 4; assess how much 3 and 4 differ, and how much they each differ from 2

@johnou
Copy link

johnou commented Sep 22, 2021

How about turning DaprHttp into an interface, creating two implementations, okhttp and "native" jdk 11 http client then writing / running a performance test using actors with jmh?

@artursouza
Copy link
Member Author

I like the proposals above. Just be mindful that we cannot have client breaking changes in the SDK.

@Juliandb1708
Copy link

To investigate if the Java HTTP client is a suitable replacement for OkHttp in terms of performance, I created a benchmark setup. For the setup I followed the suggestions of @mthmulders and @johnou. The setup is as follows:

  1. default nginx webserver that serves a static JSON response
  2. run ApacheBench to have an idea of how fast that webserver can go (2000 requests)
  3. run Java program with JMH to performance test two implementations (OkHttp and Java HTTP client), both implementing a simplified interface version of the DaprHttp class (2000 requests each)
  4. compare the results of the benchmarks

I ran all the tests on my own machine (with a pretty good processor). So all performance tests ran very fast. I also tried it on a slower machine, which had a visible impact on the speed of the HTTP clients. But relatively this gave the same results. On my own machine this resulted in the following measurements:

  • Apachebench: 0.244 ms/op
  • JMH OkHttp: 0.497 ms/op
  • JMH Java HTTP Client: 0.594 ms/op

I put the setup in a Git repository with clear instructions in the readme, so you can run the benchmark on your own machine too.

@artursouza
Copy link
Member Author

artursouza commented Dec 9, 2021

This is excellent work @Juliandb1708. I think the perf difference does not justify adding a (potential) dependency conflict to the applications consuming this SDK. Thoughts?

Vote 🎉 here to use Java HTTP Client.
Vote 🚀 here to keep OkHTTP client.

@tanvigour
Copy link
Contributor

/assign

@artursouza
Copy link
Member Author

Please, look at the partial work I have on this. It is moving to Java 11 and removing OKHTTP dependency:
https://github.com/artursouza/java-sdk/tree/native_http_client

Pending work:

  • Rewrite the unit test for DaprHttp so it does not use OKHttp mocking anymore. Needs to find another mocking library that can work with the generic HTTP Client from Java 11.
  • Rewrite the URL building logic so it does not use the OKHttp object anymore - we might need to recreate those objects in our code, keeping those package visible only.

@lburgazzoli
Copy link

lburgazzoli commented Oct 19, 2023

I can help with this issue if no one else is looking at it.

I would propose to:

  1. deprecate Java 8 and move to Java 11 (it is already required to build the project)
  2. switch to Java HTTP Client from OkHTTP not needed anymore

@skyao
Copy link
Member

skyao commented Dec 18, 2023

I just submitted a proposal to add jdk17 and springboot 3.0 to the dapr java sdk. One of the pending issues is whether continue to support java8.

#971

@artursouza
Copy link
Member Author

We now require JDK 11 as the minimum JDK version.

@artursouza artursouza unpinned this issue Apr 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants