RFR (M): CR 8003985: Support @Contended Annotation (original) (raw)

David Holmes david.holmes at oracle.com
Thu Nov 29 19:35:28 PST 2012

On 30/11/2012 4:56 AM, Aleksey Shipilev wrote:

Thanks for review, Christian!

On 11/29/2012 10:28 PM, Christian Thalinger wrote:

The webrev for the change is here: http://shipilev.net/pub/jdk/hotspot/contended/webrev-4/

Is this a review for a push? Yes, I'm pretty much done with functionality, it passes the tests, and flies through JPRT without problems.

This work can not be pushed until the JEP has moved to the Funded state.

Meanwhile we should refine any coding issues regarding style/indents/copyrights etc.

I'd also like to know what testing has been done so far from the regular hotspot testsuites. There may be places where layout assumptions are being made eg oops first.

Note this is not a Review - I'm not familiar enough with this code to review, sorry.

Thanks, David

hotspot/src/share/vm/classfile/classFileParser.cpp:

Is it possible to factor the while-loop to align the fields into a method? The whole method ClassFileParser::parseClassFile cries for refactoring. Alas, separating the contended handling code would be more messy than just fitting in the same flow, since it uses quite of few locals within that method. hotspot/src/share/vm/classfile/classFileParser.hpp: + bool hasContendedAnnotation() { return hasannotation(sunmiscContended); } We don't use camel-case method names. OK. hotspot/src/share/vm/classfile/vmSymbols.hpp: Please keep the indent aligned. OK. hotspot/src/share/vm/oops/fieldInfo.hpp: I wonder if we have a spare accessflags bit somewhere we could use for iscontended. Looking at jvm.h, I see there is only single bit left, want me to burn it with contended flag? jdk/src/share/classes/sun/misc/Contended.java: Misses copyright header. Will update. -Aleksey.

More information about the hotspot-dev mailing list