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

Explore MethodHandles usage #917

Closed
wants to merge 2 commits into from

Conversation

marko-bekhta
Copy link
Member

Do not merge this :) It's just to be able to post jmh results and do some experiments with MH stuff.

@gsmet I've tried a couple things that we talked about yesterday... Here are some first results based on such kind of changes:

  • these are for the case visible in this PR (when there's a validation of just one bean)
as is:

# Run complete. Total time: 00:00:40

Benchmark                                Mode  Cnt     Score     Error   Units
FooValidation.testSimpleBeanValidation  thrpt   20  1417.867 ± 140.205  ops/ms

just creating MH stuff in constructor: 

# Run complete. Total time: 00:00:40

Benchmark                                Mode  Cnt     Score     Error   Units
FooValidation.testSimpleBeanValidation  thrpt   20  1421.445 ± 196.217  ops/ms

having a private static final MH:

# Run complete. Total time: 00:00:40

Benchmark                                Mode  Cnt     Score    Error   Units
FooValidation.testSimpleBeanValidation  thrpt   20  1569.186 ± 56.427  ops/ms

Next I've tried to validate a collection of beans (as usual - 100 beans in a list) instead of one and the results I've got are (on a different machine):

as is:

# Run complete. Total time: 00:00:40

Benchmark                                Mode  Cnt  Score   Error   Units
FooValidation.testSimpleBeanValidation  thrpt   20  4.217 ± 0.210  ops/ms

simple MH field init in constructor:

# Run complete. Total time: 00:00:40

Benchmark                                Mode  Cnt  Score   Error   Units
FooValidation.testSimpleBeanValidation  thrpt   20  4.496 ± 0.391  ops/ms

private static final MH

# Run complete. Total time: 00:00:39

Benchmark                                Mode  Cnt  Score   Error   Units
FooValidation.testSimpleBeanValidation  thrpt   20  4.696 ± 0.169  ops/ms

I'm a bit surprised to see that big of a jump in case of one bean when a constant field used and such small when just a MH is used as a regular field.
I think that a validation of a collection gives a better picture. I'll play with it a bit more and post the results here.

Note: to see how I've "changed" the location impls just check a corresponding commit.

@gsmet
Copy link
Member

gsmet commented Feb 6, 2018

FWIW, the general opinion is that MethodHandles can only get better and be polished in the future. So that would be a good move anyway, if it's at least not slower than what we had.

So I would be +1 on pursuing this.

@gunnarmorling
Copy link
Member

@marko-bekhta, @gsmet, have you read Geoffrey's recent post on this: https://www.optaplanner.org/blog/2018/01/09/JavaReflectionButMuchFaster.html? It seems the best we can do would be to use code generation for accessing public members and fall back to method handles otherwise.

@gsmet
Copy link
Member

gsmet commented Feb 6, 2018

@gunnarmorling yes, we already discussed this post and John's results with Marko. We wanted to see what differences will it make in HV to understand better if this approach would be worth it or not.

Clearly, I'm against moving to bytecode generation in a micro. Not sure I would move to MethodHandle either tbh - especially since the gain is very minor.

I would say let's revisit it once we prepare a 6.1. Things might have evolved in the right way then (maybe by reducing the cost of not having the MH static final).

@gsmet gsmet changed the title Mh experiments Explore MethodHandles usage May 3, 2018
@gsmet
Copy link
Member

gsmet commented May 3, 2018

@marko-bekhta so on this one, I think it would be worth it to take a first baby step and migrate to MethodHandles even if they are not static final.

@marko-bekhta
Copy link
Member Author

+1 to introduce the handles first. If we do this right, then replacing it with generated classes shouldn't be a problem (if we could generate a class that we need :)).
We would need to add an option for users to pass MethodHandles.Lookup to us, right ?

@gsmet
Copy link
Member

gsmet commented May 3, 2018

We would need to add an option for users to pass MethodHandles.Lookup to us, right ?

Probably. What bugs me a little is that in the OSGi case, you will be forced to pass a lookup in addition to the current external class loader. I don't see a way to avoid that and this is unfortunate as it will break existing applications. Unless you see a workaround?

So for now, I would favor working on things that can bring something to the user as the JSON stuff.

@gsmet
Copy link
Member

gsmet commented Feb 17, 2019

This was superseded by #975 .

@gsmet gsmet closed this Feb 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants