Discussion:
RFR (XS) 8059595: Verifier::verify is wasting time before is_eligible_for_verification check
Aleksey Shipilev
2014-10-02 10:17:48 UTC
Permalink
Hi,

Please review a tiny performance improvement in Verifier:
http://cr.openjdk.java.net/~shade/8059595/webrev.00/
https://bugs.openjdk.java.net/browse/JDK-8059595

The minuscule performance problem is that we check for verification
eligibility after doing a heavy-lifting for klass->external_name() and
such. The solution is to move the local initialization after the
eligibility check. That involves moving the PENDING_EXCEPTION checking
block under the eligibility check, since the locals are used there.
After you do that, it seems prudent to just invert the branch for the
same effect.

Anyone savvy with Verifier code (Harold?) can confirm this is safe change?

Testing: JPRT, targeted performance test on Linux x86_64/fastdebug.

Thanks,
-Aleksey.
Keith McGuigan
2014-10-02 12:34:03 UTC
Permalink
Hi Aleksey,

I think this looks fine (and safe). Reviewed.

--
- Keith

On Thu, Oct 2, 2014 at 6:17 AM, Aleksey Shipilev <
Post by Aleksey Shipilev
Hi,
http://cr.openjdk.java.net/~shade/8059595/webrev.00/
https://bugs.openjdk.java.net/browse/JDK-8059595
The minuscule performance problem is that we check for verification
eligibility after doing a heavy-lifting for klass->external_name() and
such. The solution is to move the local initialization after the
eligibility check. That involves moving the PENDING_EXCEPTION checking
block under the eligibility check, since the locals are used there.
After you do that, it seems prudent to just invert the branch for the
same effect.
Anyone savvy with Verifier code (Harold?) can confirm this is safe change?
Testing: JPRT, targeted performance test on Linux x86_64/fastdebug.
Thanks,
-Aleksey.
--
[image: twitter-icon-large.png]

Keith McGuigan

@kamggg

kmcguigan at twitter.com
harold seigel
2014-10-02 12:51:13 UTC
Permalink
Hi Aleksey,

The change looks good.

Thanks, Harold
Post by Aleksey Shipilev
Hi,
http://cr.openjdk.java.net/~shade/8059595/webrev.00/
https://bugs.openjdk.java.net/browse/JDK-8059595
The minuscule performance problem is that we check for verification
eligibility after doing a heavy-lifting for klass->external_name() and
such. The solution is to move the local initialization after the
eligibility check. That involves moving the PENDING_EXCEPTION checking
block under the eligibility check, since the locals are used there.
After you do that, it seems prudent to just invert the branch for the
same effect.
Anyone savvy with Verifier code (Harold?) can confirm this is safe change?
Testing: JPRT, targeted performance test on Linux x86_64/fastdebug.
Thanks,
-Aleksey.
Aleksey Shipilev
2014-10-02 13:16:57 UTC
Permalink
Thanks Harold and Keith!

If current testing and reviews are enough, please sponsor:
http://cr.openjdk.java.net/~shade/8059595/8059595.changeset

-Aleksey.
Post by Keith McGuigan
Hi Aleksey,
The change looks good.
Thanks, Harold
Post by Aleksey Shipilev
Hi,
http://cr.openjdk.java.net/~shade/8059595/webrev.00/
https://bugs.openjdk.java.net/browse/JDK-8059595
The minuscule performance problem is that we check for verification
eligibility after doing a heavy-lifting for klass->external_name() and
such. The solution is to move the local initialization after the
eligibility check. That involves moving the PENDING_EXCEPTION checking
block under the eligibility check, since the locals are used there.
After you do that, it seems prudent to just invert the branch for the
same effect.
Anyone savvy with Verifier code (Harold?) can confirm this is safe change?
Testing: JPRT, targeted performance test on Linux x86_64/fastdebug.
Thanks,
-Aleksey.
harold seigel
2014-10-02 13:33:04 UTC
Permalink
Hi Aleksey,

I'll sponsor the change.

Harold
Post by Aleksey Shipilev
Thanks Harold and Keith!
http://cr.openjdk.java.net/~shade/8059595/8059595.changeset
-Aleksey.
Post by Keith McGuigan
Hi Aleksey,
The change looks good.
Thanks, Harold
Post by Aleksey Shipilev
Hi,
http://cr.openjdk.java.net/~shade/8059595/webrev.00/
https://bugs.openjdk.java.net/browse/JDK-8059595
The minuscule performance problem is that we check for verification
eligibility after doing a heavy-lifting for klass->external_name() and
such. The solution is to move the local initialization after the
eligibility check. That involves moving the PENDING_EXCEPTION checking
block under the eligibility check, since the locals are used there.
After you do that, it seems prudent to just invert the branch for the
same effect.
Anyone savvy with Verifier code (Harold?) can confirm this is safe change?
Testing: JPRT, targeted performance test on Linux x86_64/fastdebug.
Thanks,
-Aleksey.
Aleksey Shipilev
2014-10-02 18:03:10 UTC
Permalink
Thanks, Harold!

-Aleksey.
Post by Keith McGuigan
Hi Aleksey,
I'll sponsor the change.
Harold
Post by Aleksey Shipilev
Thanks Harold and Keith!
http://cr.openjdk.java.net/~shade/8059595/8059595.changeset
-Aleksey.
Post by Keith McGuigan
Hi Aleksey,
The change looks good.
Thanks, Harold
Post by Aleksey Shipilev
Hi,
http://cr.openjdk.java.net/~shade/8059595/webrev.00/
https://bugs.openjdk.java.net/browse/JDK-8059595
The minuscule performance problem is that we check for verification
eligibility after doing a heavy-lifting for klass->external_name() and
such. The solution is to move the local initialization after the
eligibility check. That involves moving the PENDING_EXCEPTION checking
block under the eligibility check, since the locals are used there.
After you do that, it seems prudent to just invert the branch for the
same effect.
Anyone savvy with Verifier code (Harold?) can confirm this is safe change?
Testing: JPRT, targeted performance test on Linux x86_64/fastdebug.
Thanks,
-Aleksey.
Loading...