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

Pulled varargs test out of the loop to avoid checking on each parameter #352

Closed
wants to merge 1 commit into from

Conversation

Boereck
Copy link

@Boereck Boereck commented Jul 21, 2014

I realized that a lot of time is spent in the isVarArgs method, because it is called for every parameter, but the result will always be the same in each loop iteration.
So I pulled the check out of the loop. For functions with no parameter it introduces one more check and variable assignment, but the gain for methods with >1 parameter is dramatic.
This change makes one of the applications I am working on 13 to 15 percent faster on my machine.

@dblock
Copy link
Member

dblock commented Jul 23, 2014

Looks legit, @twall will comment. Can you please squash these commits and add a line to the CHANGES.md? Thx.

@Boereck
Copy link
Author

Boereck commented Jul 23, 2014

I realized that the double check locking will only work if Library$Handler$FunctionInfo is an immutable class (because of possible compiler reordering and Java Memory Model guarantees). So I will fix that, add a short description to the CHANGES.md and squash the commits either later today or tomorrow.

The Method#isVarargs method is now referenced statically to avoid costs
of looking it up on each native method call with at least one parameter.
This also can save a lot of memory, since on every Class#getMethod()
call a new Method object is returned.

Added null check

uses the double-checked locking to speed up subsequent calls to the same
function.

Made Function$Handler$FunctionInfo immutable class to make double check
locking work correctly.

Added description of changes to CHANGES.md
@twall
Copy link
Contributor

twall commented Jul 24, 2014

LGTM.

@dblock
Copy link
Member

dblock commented Jul 24, 2014

I'll merge.

@twall, the Linux build intermittently crashes, maybe you can check it out, I wasn't able to figure out what causes this.

@dblock
Copy link
Member

dblock commented Jul 24, 2014

Merged via 7f2c689.

@dblock dblock closed this Jul 24, 2014
@twall
Copy link
Contributor

twall commented Jul 25, 2014

I started looking at this but didn’t find anything conclusive. Will continue…

On Jul 24, 2014, at 9:21 AM, Daniel Doubrovkine (dB.) @dblockdotorg notifications@github.com wrote:

I'll merge.

@twall, the Linux build intermittently crashes, maybe you can check it out, I wasn't able to figure out what causes this.


Reply to this email directly or view it on GitHub.

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.

None yet

3 participants