search

jhipster / generator-jhipster

JHipster is a development platform to quickly generate, develop, & deploy modern web applications & microservice architectures.
https://www.jhipster.tech
Apache License 2.0
21.54k stars 4.02k forks source link

Migrate away from Jackson Afterburner #9934

Closed jdubois closed 4 years ago

jdubois commented 5 years ago

We've been using Jackson Afterburner for quite a long time: it has a small performance cost at startup, but can make JHipster applications about 10% more performant overall than vanilla Spring Boot applications (One of the many tricks we use!!).

Unfortunately it gives an ugly warning message at startup since Java 9, see https://github.com/FasterXML/jackson-modules-base/issues/37

Given the way Afterburner works, I'm not sure this can be solved, so we need to find alternative solutions. It looks like https://github.com/stevenschlansker/jackson-blackbird (ping @stevenschlansker ) is promising, we could offer them our help, WDYT?

stevenschlansker commented 5 years ago

Hi @jdubois , I'm very excited you've found my project! It's a proof of concept right now, although we do run it in production on our own systems. I believe the approach is flexible and should be much easier to extend looking forward (as the LambdaMetafactory hopefully gets more powerful e.g. generated field accessors). Bytecode manipulation was always the most powerful approach but it's a PITA to be the maintainer!

I would be excited to collaborate on taking the blackbird project forward :)

The biggest caveat right now is it basically requires JDK 11 (9 works but is EOL), which is not a problem for us, as we already run 12...

The other big TODO is to get a broader picture of performance versus Afterburner versus vanilla, across a sampling of different use cases.

jdubois commented 5 years ago

Thanks @stevenschlansker ! I didn't understand that Blackbird required JDK 11... Unfortunately we need to support JDK 8 for some time still, as we have many users who are locked on it, and that would be a stopper issue for us... Can you check the JDK version and only activate it when JDK 11 is found? Otherwise that's something we could on our side, and switch between Afterburner and Blackbird depending on the JDK.

jdubois commented 5 years ago

I'm adding a bug bounty here, as this is an important added value for JHipster users.

SudharakaP commented 5 years ago

@jdubois @pascalgrimaud : I've tested jackson-blackbird on my local with Java 11 and it works as expected. I can do a pull request for this but would like to discuss my approach before doing it. 😄

  1. We can prompt the user to choose between Java 8 and 11 in the generator and based on that set the JAVA_VERSION variable to the appropriate java version. This will enable us to choose between jackson-afterburner and jackson-blackbird when generating the initial pom. Does this sound good or any suggessions?

  2. Jackson Blackbird doesn't have releases yet so we have to build it ourselves. I am not sure how you guys do custom package distribution; do we use the Github package registry or some other approach to do this?

gmarziou commented 5 years ago

I'm not a big fan of using SNAPSHOT versions that could change without notice. Maybe @stevenschlansker should release a 0.1 version.

Also, Steven states "Blackbird is experimental and needs brave pilots to take her for test flights". I suspect that JHipster users don't expect their code generator to turn their applications into pilots for promising technologies without requesting their consent.

The safest would be to propose it as an option/question and by default just offer plain Jackson without Blackbird/AfterBurner in JDK 11+.

stevenschlansker commented 5 years ago

Also, Steven states "Blackbird is experimental and needs brave pilots to take her for test flights". I suspect that JHipster users don't expect their code generator to turn their applications into pilots for promising technologies without requesting their consent.

Since I wrote that, we have run blackbird in production on our own apps for months with no problems.

I still think it's reasonable to have it opt-in to start with, of course! And it would be straightforward to detect the necessary JDK features and disable itself entirely if it cannot run, or even helpfully delegate to Afterburner in that case.

I am happy to make a release but had not set up a Maven Central org to host it. Perhaps I can "borrow" my Jdbi maintainer status and release it there, until we can contribute it to Jackson.

SudharakaP commented 5 years ago

@jdubois, @gmarziou : If there are no objections, I will wait until @stevenschlansker releases the first version of BlackBird to work on this one.

stevenschlansker commented 5 years ago

I am building a release now.

SudharakaP commented 5 years ago

@stevenschlansker : Thanks much. Feel free to ping me or post here once you've released it to Maven Central.

stevenschlansker commented 5 years ago

@SudharakaP, I completed a release as:

    <dependency>
      <groupId>org.jdbi.jackson-contrib</groupId>
      <artifactId>jackson-module-blackbird</artifactId>
      <version>0.0.1</version>
    </dependency>

It should be on Maven Central shortly. Please let me know if you have any issues!

stevenschlansker commented 5 years ago

I started discussing contributing my code to Jackson here: https://github.com/FasterXML/jackson-modules-base/issues/59

SudharakaP commented 5 years ago

@stevenschlansker : Thank you 😄

@jdubois @pascalgrimaud : I've added jackson-blackbird support for Java 11 as suggested by @gmarziou (https://github.com/jhipster/generator-jhipster/issues/9934#issuecomment-521961712). The user is given a prompt to choose between java 11 and 8 and if java 11 is chosen, another prompt for using BlackBird with default option being "no".

Please feel free to let me know if you see any issues or if there's anything to be changed. 😄

jdubois commented 5 years ago

I just got back from holidays: this looks awesome!!! Yes we'll need to be careful when we release this first, maybe mark the "Java 11" option with "experimental" for some time

SudharakaP commented 5 years ago

I just got back from holidays: this looks awesome!!! Yes we'll need to be careful when we release this first, maybe mark the "Java 11" option with "experimental" for some time

@jdubois : There's two questions; users are first prompted for Java 8 or Java 11 and if they choose Java 11 we give an option to include BlackBird. In this second prompt I've mentioned that this is an experimental library. If you have any other way of presenting the info; please feel free to let me know 😄

jdubois commented 5 years ago

@SudharakaP I was just thinking of one option with everything, but two options work too -> we'll just keep the "experimental" option for some time, and if we have no negative feedback, then we'll remove it. It's simple, I like it!

SudharakaP commented 5 years ago

@jdubois : Wonderful, thanks. I'll keep the options as they are for now. There's one suggestion in the reviews section of the pull request which I don't quite understand, which we need to discuss. Apart from that, I guess this is complete and good to go.

stevenschlansker commented 5 years ago

I have a PR open to contribute my work as a Jackson addon: https://github.com/FasterXML/jackson-modules-base/pull/85

It also includes basic Java 8 compatibility since Jackson's build is only on Java 8. However, I don't expect performance to be great, and would not recommend running in that configuration.

SudharakaP commented 4 years ago

Just to keep everyone in the loop, as per the discussion >>here<<, I've replaced Afterburner by Blackbird; removing the logic we put earlier giving the user the ability to choose between Afterburner and Blackbird. Thus the plan is to merge this at v7. :smile:

SudharakaP commented 4 years ago

As per discussion in the following thread; https://github.com/jhipster/generator-jhipster/pull/10283 we have decided not to use Blackbird or Afterburner and we have merged #11606 which removes Afterburner. Therefore I am closing this issue. :smile: