RFR (S): 8199567: [Nestmates] Cleanup instanceKlass.cpp (original) (raw)
David Holmes david.holmes at oracle.com
Tue Oct 23 20:59:56 UTC 2018
- Previous message: RFR (S): 8199567: [Nestmates] Cleanup instanceKlass.cpp
- Next message: RFR(XS): 8212220 add code to verify results to metaspace/stressDictionary/StressDictionary.java
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
Thanks for the review Coleen!
David
On 24/10/2018 4:09 AM, coleen.phillimore at oracle.com wrote:
This looks fine. Thank you both for suggesting and making the isklass() change. Coleen On 10/23/18 10:20 AM, Lois Foltan wrote: On 10/23/2018 8:36 AM, David Holmes wrote:
Hi Lois,
On 23/10/2018 10:14 PM, Lois Foltan wrote: On 10/22/2018 11:43 PM, David Holmes wrote:
Bug: https://bugs.openjdk.java.net/browse/JDK-8199567 Webrev: http://cr.openjdk.java.net/~dholmes/8199567/webrev/
This is a simple "cleanup" previously suggested by John Rose for checking nest membership. Rather than checking for name equality and then checking the loaded class, we check if there's a loaded class first and if not fallback to the name check. This favours the case where the nestmate classes are already resolved and loaded. Hi David, Looks good. One comment: Thanks for looking at it! oops/instanceKlass.cpp: - line #173: Consider changing the if conditional to "if (constants->tagat(cpindex).isklass())" I believe the "isklass()" method is the preferred way to compare a tag to JVMCONSTANTClass. Sure - webrev updated: http://cr.openjdk.java.net/~dholmes/8199567/webrev.v2/ Looks good, thank you for making that change! Lois Thanks, David Thanks, Lois
There was no observable performance difference. Testing: mach5 tiers 1-3 including nestmate tests Thanks, David
- Previous message: RFR (S): 8199567: [Nestmates] Cleanup instanceKlass.cpp
- Next message: RFR(XS): 8212220 add code to verify results to metaspace/stressDictionary/StressDictionary.java
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]