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

Add benchmark to measure performance of VH adapters #175

Closed

Conversation

@mcimadamore
Copy link
Collaborator

@mcimadamore mcimadamore commented May 18, 2020

This patch adds a straightforward benchmark to measure performances of var handle adapters.

The benchmark is set up to read values from an int array, both with a var handle(obtained from MethodHandles), an associated MethodHandle (derived from the first var handle) and with a segment-based, memory access var handle.

Then the same test is repeated, but with an extra adaptation step inserted in the middle - rather than reading int values directly, the adapter turns ints
into instances of IntBox and the loop code doing the sum converts them back into ints.

Numbers are extremeluy solid on my machine:

TestAdaptVarHandles.mh_box_loop       avgt   30  0.306 ? 0.009  ms/op
TestAdaptVarHandles.mh_loop           avgt   30  0.297 ? 0.009  ms/op
TestAdaptVarHandles.segment_box_loop  avgt   30  0.308 ? 0.009  ms/op
TestAdaptVarHandles.segment_loop      avgt   30  0.307 ? 0.008  ms/op
TestAdaptVarHandles.vh_box_loop       avgt   30  0.296 ? 0.005  ms/op
TestAdaptVarHandles.vh_loop           avgt   30  0.291 ? 0.003  ms/op

I thought it would have been nice to add this to our benchmark suites since we do not have anything that tests VH adaptation directly.


Progress

  • Change must not contain extraneous whitespace
  • Change must be properly reviewed

Reviewers

Download

$ git fetch https://git.openjdk.java.net/panama-foreign pull/175/head:pull/175
$ git checkout pull/175

@bridgekeeper
Copy link

@bridgekeeper bridgekeeper bot commented May 18, 2020

👋 Welcome back mcimadamore! A progress list of the required criteria for merging this PR into foreign-memaccess will be added to the body of your pull request.

@openjdk openjdk bot added the rfr label May 18, 2020
@mlbridge
Copy link

@mlbridge mlbridge bot commented May 18, 2020

Webrevs

@JornVernee
Copy link
Member

@JornVernee JornVernee commented May 18, 2020

With the adapter creating IntBox, this benchmark is also testing whether C2 can eliminate the allocation of the box.

If the goal is to measure an adapted VarHandle against a non-adapted baseline VarHandle, I think it makes sense to have the adapters be the identity function, since then the only difference is whether an adapter is present or not.

WDYT?

@mcimadamore
Copy link
Collaborator Author

@mcimadamore mcimadamore commented May 18, 2020

I guess my (non stated) goal was to benchmark non trivial adaptation where users could create their own carriers on top of the foreign memory access API. I wanted to make sure that the VH machinery didn't add extra cost when doing so.

Copy link
Member

@JornVernee JornVernee left a comment

Looks good

@openjdk
Copy link

@openjdk openjdk bot commented May 18, 2020

@mcimadamore This change now passes all automated pre-integration checks, type /integrate in a new comment to proceed. After integration, the commit message will be:

Add benchmark to measure performance of VH adapters

Reviewed-by: jvernee, psandoz
  • If you would like to add a summary, use the /summary command.
  • To credit additional contributors, use the /contributor command.
  • To add additional solved issues, use the /issue command.

Since the source branch of this PR was last updated there has been 1 commit pushed to the foreign-memaccess branch:

  • 0bc9c6b: Move MemoryAddress::copy (more review feedback)

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid automatic rebasing, please merge foreign-memaccess into your branch, and then specify the current head hash when integrating, like this: /integrate 0bc9c6bc13acc7c8044219b883bf1e9848a37776.

➡️ To integrate this PR with the above commit message to the foreign-memaccess branch, type /integrate in a new comment.

@openjdk openjdk bot added the ready label May 18, 2020
@mcimadamore
Copy link
Collaborator Author

@mcimadamore mcimadamore commented May 18, 2020

/integrate

@openjdk openjdk bot closed this May 18, 2020
@openjdk openjdk bot added integrated and removed ready rfr labels May 18, 2020
@openjdk
Copy link

@openjdk openjdk bot commented May 18, 2020

@mcimadamore The following commits have been pushed to foreign-memaccess since your change was applied:

  • 0bc9c6b: Move MemoryAddress::copy (more review feedback)

Your commit was automatically rebased without conflicts.

Pushed as commit e2d2dba.

@mcimadamore mcimadamore deleted the mcimadamore:adaptVHbench branch Nov 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.