RFR (S) CR 8014966: Add the proper Javadoc to @Contended (original) (raw)

Mike Duigou mike.duigou at oracle.com
Thu May 30 23:04:39 UTC 2013

Hi Aleksey;

Прогресс! (One of the few Russian words I know thanks to the ISS)

Since the statements are grouped by fields and classes, move this to the classes paragraph:

"A contention

Describing the behaviour of the class annotation by referencing unavailable functionality seems strange:

"the effect is

The reader is left wondering "how would I annotate multiple fields with the same anonymous tag?" Perhaps a statement in the fields paragraph that "To put multiple fields into the same anonymous group use the class level @C annotation".

"over all the unannotated fields of

This refers, of course, to fields without their own @C annotation not fields without annotations.

Addition:

The class level {@code @Contended} annotation is not inherited and has + * no effect on the fields [defined] in any sub-classes.

Addition:

"The effects of all

As I told Brian during the lambda javadoc efforts, writing specification is the best kind of task--demanding, exacting and tedious! :-)

Mike

On May 29 2013, at 04:20 , Aleksey Shipilev wrote:

Thanks for the review!

On 05/29/2013 04:28 AM, Martin Buchholz wrote: Yeah, we have the same problem with javadoc issuing very verbose warnings. I've been ignoring/filtering them and hoping they get fixed before jdk8 ships. Ok, reverted back to

.

+ * in concurrent contexts in which each instance of the annotated + * object is often accessed by a different thread.

Hmmm... makes it sound like it applies even if each instance is thread-confined (in a different thread) or is immutable. I think all the magic of ignoring @C when VM can prove the instance/fields would not experience contention, should be omitted from the documentation. The practical considerations also apply: since we can not at the moment retransform the class after the publication, it seems a good idea to treat all instances as potentially shared. This eases the reasoning (and documentation) significantly. "objects" are not annotated. Yes, should be "instances". I was thinking "classes, not objects, are annotated". Fixed. however, remain in force for all + * superclass and subclass instances

"remain in force for superclass instances" doesn't make sense to me. Do you mean "remain in force when fields are inherited in subclasses" Yes, that sounds better! Reverted back to David's version. It does sound even cleaner in that version. Maybe "instances of the annotated class are frequently accessed by multiple threads and have fields that are frequently written". "Accessed" matters there. False sharing also happens on reads. Also, the innocent read-only classes sometimes also need the protection from the adjacent writers' racket :) Hmmm... tricky ... This also then applies to pure immutable popular objects like Boolean.TRUE. But you want those kinds of objects to all be colocated and tightly packed with other such objects, not give each of them their own cache line. Yes, that's true. I do, however, think we have to have the provisioning for the read-only protected fields/instances. Sounds like a Quality of Implementation issue. In general, I think you DO want the subclass fields to be potentially in the same location as the superclass padding fields. A >: B A layout: ppppaaaapppp B layout: ppppaaaabbbbpppp which is not asking too much of a jit. I believe it is already common for VMs to allocate subclass fields in natural padding space of superclasses. This gets very tricky even in a slightly complicated case: A >: B A layout: ppppAAppppCCpppp B layout: ppppAABBppCCpppp Note that we can't move CC in B because the fields are bound statically; so we end up ruining the padding for both BB and CC. We can, of course, have the gap in the padding to tolerate this, but before knowing in advance what subclasses will be loaded next, we can not prepare enough; or, we can redefine all the classes up the hierarchy... (Current HotSpot classloader code is completely not ready for things like these). However, implementation issues aside: Allowing contended tags to be inherited will force users to look at the entire hierarchy to search for the colliding tags; or (what's worse) push the superclass maintainers to use cryptic tags so no possible subclasses can collide with the protected fields. IMO, that is way messier than pushing users to aggregate the semantically-close fields in the same class. Hence, I left the inheritance clause as is. The new webrev is here: http://cr.openjdk.java.net/~shade/8014966/webrev.03/ Thanks everyone, writing specs is fun! -Aleksey.

More information about the core-libs-dev mailing list