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

Build an über jar using the standard jar plugin. #154

Merged
merged 3 commits into from
Aug 2, 2019

Conversation

jake-at-work
Copy link
Contributor

  • Corrects the use of class paths to get all dependencies into the jar.
  • Explode all dependency jars into the über jar.
  • Updates minimum Gradle in 5.5.0 (released)
  • Cleanup some deprecation warnings.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.01%) to 51.737% when pulling 6482f72 on pivotal-jbarrett:feature/uber-jar into 3f5003d on melix:master.

@melix
Copy link
Owner

melix commented Jul 30, 2019

Thanks for the PR. However I think it contradicts the intent of this task, which is to avoid the creation of an uber jar unless you use the shadow plugin. In particular not all projects need an uber jar, they can keep their dependencies separate.

Actually I would prefer if we had an option to use either the shadow jar or your code, but keep the option to use separate jars. Does it make sense?

@jake-at-work
Copy link
Contributor Author

Thanks for the feedback. The reason I changed this jar is because what was produced didn’t make sense to me. The jar contained the main and jmh classes and then jars from runtimeOnly (as jars embedded in this jar). I couldn’t find any real use for this jar. I assumed it was mean to produce something like the shadow jar but had been limited by older version of Gradle.

So what is the intended use case for this jar?

@melix
Copy link
Owner

melix commented Jul 31, 2019

I agree with you that the current jar doesn't make sense. I'm not quite sure why it was implemented this way, possibly as a workaround for some JMH classloading issue.

I think ideally the normal jar should remain the normal jar, nothing embedded.
Then there should be something that generates a runnable JMH "app", and last there should be an option to create a fatjar.

@jake-at-work
Copy link
Contributor Author

I agree with you that the current jar doesn't make sense. I'm not quite sure why it was implemented this way, possibly as a workaround for some JMH classloading issue.

Well if there isn't value in the current implementation we should probably change it. Is there a strong need to keep an unknown thing like this around?

I think ideally the normal jar should remain the normal jar, nothing embedded.
Then there should be something that generates a runnable JMH "app", and last there should be an option to create a fatjar.

What do you see as a difference between a runnable JMH jar and a fat jar?

What I see is that we have two things, either you are running your JMH via Gradle or not.

If you run them from Gradle then you don't need the jar at all. You set the class path for the forked worker and it would execute the JHM runner, just the same as it would from the JMH app jar, but without a need to generate and include that jar.

If you run it outside of Gradle then you likely want to do it the JMH way, which is the app/fat/uber jar that is documented in JMH. In this case I don't think we really need to be using shadow anymore since Gradle's jar task can do all that now too.

I don't see much value in producing any other jar artifacts since it isn't common, in the JMH way of things, to exec java with a long line of jars in the class path including one that has the generated benchmark byte codes. (let Gradle do that ugliness for you)

Long message short, I see really only a need for the uber/fat/app jar that contains everything necessary to execute the benchmark, call this jmhJar, and a jmh task to execute JMH but from Gradle class paths and not dependent at all on the jmhJar. In another PR I will be opening is a POC of using Gradle task options to enhance the command line for jmh task, like ./gradlew jmh --includes=FooBenchmark --threads=8. Doing so makes needing the 'jmhJar` task pretty useless unless you want to script out some more advanced benchmarking or have existing tooling expecting the benchmark uber jar.

@melix
Copy link
Owner

melix commented Aug 1, 2019

Well if there isn't value in the current implementation we should probably change it. Is there a strong need to keep an unknown thing like this around?

No. To be honest I'm very unsatisfied by the implementation of this plugin. It should be simpler and easier to maintain. Only I don't have the bandwidth to do it.

What do you see as a difference between a runnable JMH jar and a fat jar?

I'm not particularly talking about a runnable jar. There should be different things:

  • the benchmarks
  • the dependencies of the benchmarks
  • the JMH runtime

Then we can provide something that can be executed from the CLI: java -cp bench.jar:jmh.jar Main

And what I'm saying is that their could be a convenience, just like the application plugin, to have something that runs it. Running from within Gradle as a task is already a convenience but if you do serious benchmarking you shouldn't really run from Gradle.

Last, there's the fatjar version. This version can be standalone, I don't really care.

If you run it outside of Gradle then you likely want to do it the JMH way, which is the app/fat/uber jar that is documented in JMH. In this case I don't think we really need to be using shadow anymore since Gradle's jar task can do all that now too.

Yes and no. Gradle can do it, but shadow has way more configuration. Merging jars is not a trivial thing. There are descriptors, for example, which may need to be merged (think of Groovy extension class descriptors).

I don't see much value in producing any other jar artifacts since it isn't common, in the JMH way of things, to exec java with a long line of jars in the class path including one that has the generated benchmark byte codes. (let Gradle do that ugliness for you)

There's one value in that it keeps the structure simple and maintainable. Each step is separate. Generate classes, compile, package in a jar, generate uber jar, ... It also makes it possible to integrate with other plugins if need be.

@melix melix merged commit 032767b into melix:master Aug 2, 2019
@melix
Copy link
Owner

melix commented Aug 2, 2019

So given this PR improves the situation I decided to merge it.
I still think we should redesign a bit the architecture of this plugin which is too messy but at least what it does now is consistent.
I will issue an rc2 soon.

@melix
Copy link
Owner

melix commented Aug 2, 2019

And 0.5.0-rc-2 is out with your changes.

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

Successfully merging this pull request may close these issues.

3 participants