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

Use Class#getConstructors to check if the Pointer.class based constructor exists #1108

Closed
wants to merge 1 commit into from

Conversation

bjorndarri
Copy link
Contributor

Instead of using Class#getConstructor(Pointer.class) with try-catch.

Relevant thread in JNA forum

And if you want to dive into the reasons why using exceptions for control flow is inefficient:
The Exceptional Performance of Lil' Exception

…ss#getConstructors and check if the Pointer.class based constructor exists
@matthiasblaesing
Copy link
Member

A benchmark comparing the two variants (slighty modified to be benchmarkable) can be found here:
https://github.com/matthiasblaesing/benchmark-structureinstantiation

The numbers I got:
Java 8: https://github.com/matthiasblaesing/benchmark-structureinstantiation/blob/master/src/site/run2
Java 11: https://github.com/matthiasblaesing/benchmark-structureinstantiation/blob/master/src/site/run1

Graphics:
https://github.com/matthiasblaesing/benchmark-structureinstantiation/raw/master/src/site/benchmark-structureinstantiation.ods

My conclusion:

  • There is a slightly reduced performance for the cases where a constructor is present, that takes a Pointer as parameter. There is a pretty huge margin of error, that might mask the real performance numbers.
  • For the no-argument constructor case, the performance nearly doubled.
  • If you need peek performance, provide constructor, that takes a pointer

@neilcsmith-net you might be interested in this.

@neilcsmith-net
Copy link
Contributor

@matthiasblaesing thanks for taking a look at that! 👍 Not as bad as I feared - the logic is similar to how the JDK will find the constructor, just not able to use internal data.

Was initially surprised that the no argument constructor wasn't closer in performance, then realised it's having to allocate memory?! Not sure that's possible to workaround while backwards compatible though? For that reason, my opinion for what it's worth is still that this fix shouldn't be made, and the exception should be logged explicitly. But if this makes it in, and it causes a noticeable overhead, I'll rewrite any remaining uses we have of this.

@matthiasblaesing
Copy link
Member

I rerun the benchmarks and also covered the option to only use the parameterless constructor. That variant comes at the price, that the structure constructor in that case always allocates memory for the structure and then switches to the provided pointer.
The iteration case (that is the suggested new mode) shows a slight reduction of ops/s for the pointer constructor case, that is well within the margin of error (-2.2% to -1.8%), but shows a massive improvement (82%-109%) for the case where no pointer based constructor is present.
Using only the parameterless constructor is not an option, as it s a massive performance penalty if the structure is based on an existing pointer (performance drops to about 50%). In addition higher native and heap memory (unnecessary allocation in the constructor) would be caused.
So at this point from a performance perspective every structure should implement a Pointer based constructor, but from a stability point of view, I would not cut support for structures without a pointer based constructor.
Long story short: I intend to merge this in the next few days.

@matthiasblaesing
Copy link
Member

Merged via 044cd6f. Thank you.

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