RFR (L) 8171392 Move Klass pointers outside of ConstantPool entries so ConstantPool can be read-only

[coleen.phillimore at oracle.com]

This looks really good!

http://cr.openjdk.java.net/~iklam/jdk10/8171392_make_constantpool_read_only.v01/src/share/vm/oops/constantPool.cpp.udiff.html

+ // Add one extra element to tags for storing ConstantPool::flags().
+ Array<u1>* tags = 
MetadataFactory::new_writeable_array<u1>(loader_data, length+1, 0, 
CHECK_NULL); ... + assert(tags->length()-1 == _length, "invariant"); // 
tags->at(_length) is flags()


I think this is left over, since _flags didn't get moved after all.

+ Klass** adr = this_cp->resolved_klasses()->adr_at(resolved_klass_index);
+ OrderAccess::release_store_ptr((Klass* volatile *)adr, k);
+ // The interpreter assumes when the tag is stored, the klass is resolved
+ // and the Klass* is a klass rather than a Symbol*, so we need
+ // hardware store ordering here.
+ this_cp->release_tag_at_put(which, JVM_CONSTANT_Class);
+ return k;

The comment still refers to the switch between Symbol* and Klass* in the 
constant pool.  The entry in the Klass array should be NULL.

+ int name_index = extract_high_short_from_int(*int_at_addr(which));

Can you put klass_name_index_at() in the constantPool.hpp header file 
(so it's inlined) and have all the places where you get name_index use 
this function?  So we don't have to know in multiple places that 
extract_high_short_from_int() is where the name index is. And in 
constantPool.hpp, for unresolved_klass_at_put() add a comment about what 
the new format of the constant pool entry is {name_index, 
resolved_klass_index}. I'm happy to see this work nearing completion!  
The "constant" pool should be constant! thanks, Coleen
On 4/11/17 2:26 AM, Ioi Lam wrote:
> Hi,please review the following change 
> https://bugs.openjdk.java.net/browse/JDK-8171392 
> http://cr.openjdk.java.net/~iklam/jdk10/8171392_make_constantpool_read_only.v01/ 
> *Summary:** *   Before:   + ConstantPool::klass_at(i) finds the Klass 
> from the i-th slot of ConstantPool.   + When a klass is resolved, the 
> ConstantPool is modified to store the Klass pointer.   After:   + 
> ConstantPool::klass_at(i) finds the at this->_resolved_klasses->at(i) 
>   + When a klass is resolved, _resolved_klasses->at(i) is modified.   
> In addition:     + I moved _resolved_references and _reference_map 
> from ConstantPool       to ConstantPoolCache.     + Now _flags is no 
> longer modified for shared ConstantPools.   As a result, none of the 
> fields in shared ConstantPools are modified at run time,   so we can 
> move them into the RO region in the CDS archive. *Testing:** *   - 
> Benchmark results show no performance regression, despite the extra 
> level of     indirection, which has a negligible overhead for the 
> interpreter.   - RBT testing for tier2 and tier3. *Ports:** *   - I 
> have tested only the Oracle-support ports. For the aarch64, ppc and 
> s390 ports,     I have added some code without testing (it's probably 
> incomplete)   - Port owners, please check if my patch work for you, 
> and I can incorporate your     changes in my push. Alternatively, you 
> can wait for my push and provide fixes     (if necessary) in a new 
> changeset, and I will be happy to sponsor it. Thanks - Ioi