--- old/hotspot/src/share/vm/classfile/classFileParser.cpp 2017-06-29 10:45:51.998755826 +0200 +++ new/hotspot/src/share/vm/classfile/classFileParser.cpp 2017-06-29 10:45:51.822752644 +0200 @@ -1469,6 +1469,28 @@ } return atype; } + + // Attempt to redistribute NONSTATIC_VALUETYPE alloc type + FieldAllocationType redistribute_value(bool oops, int value_size) { + FieldAllocationType atype = NONSTATIC_VALUETYPE; + switch (value_size) { + case 1: + atype = NONSTATIC_BYTE; + break; + case 2: + atype = NONSTATIC_SHORT; + break; + case 4: + atype = (oops) ? NONSTATIC_OOP : NONSTATIC_WORD; + break; + default: + assert(is_size_aligned(value_size, BytesPerLong), "Unaligned value %d", value_size); + return atype; + } + count[atype]++; + count[NONSTATIC_VALUETYPE]--; // Leave only large variable sized behind + return atype; + } }; // Side-effects: populates the _fields, _fields_annotations, @@ -3812,7 +3834,7 @@ // Layout fields and fill in FieldLayoutInfo. Could use more refactoring! void ClassFileParser::layout_fields(ConstantPool* cp, - const FieldAllocationCount* fac, + FieldAllocationCount* fac, const ClassAnnotationCollector* parsed_annotations, FieldLayoutInfo* info, TRAPS) { @@ -3835,6 +3857,13 @@ // The layout code below will also ignore the static fields. int nonstatic_contended_count = 0; FieldAllocationCount fac_contended; + + // Redistribute value types if possible, gather their klasses, oopmap count... + unsigned int value_type_oop_map_count = 0; + int large_value_count = 0; + ValueKlass** nonstatic_value_type_klasses = + NEW_RESOURCE_ARRAY_IN_THREAD(THREAD, ValueKlass*, fac->count[NONSTATIC_VALUETYPE]); + for (AllFieldStream fs(_fields, cp); !fs.done(); fs.next()) { FieldAllocationType atype = (FieldAllocationType) fs.allocation_type(); if (fs.is_contended()) { @@ -3843,6 +3872,25 @@ nonstatic_contended_count++; } } + else if (atype == NONSTATIC_VALUETYPE) { + Symbol* signature = fs.signature(); + Klass* klass = SystemDictionary::resolve_or_fail(signature, + Handle(THREAD, _loader_data->class_loader()), + _protection_domain, true, CHECK); + assert(klass != NULL && klass->access_flags().is_value_type(), "Sanity check"); + ValueKlass* vklass = ValueKlass::cast(klass); + if (vklass->contains_oops()) { + value_type_oop_map_count += vklass->nonstatic_oop_map_count(); + } + int raw_value_size = vklass->raw_value_byte_size(); + FieldAllocationType atype = fac->redistribute_value(vklass->contains_oops(), raw_value_size); + if (atype == NONSTATIC_VALUETYPE) { + // Failed to slot in with the others, allocate large values last jlong align for safe copy + nonstatic_value_type_klasses[large_value_count++] = vklass; + } else { + fs.coerce_allocation_type(NONSTATIC_VALUETYPE, (int) atype); + } + } } @@ -3903,43 +3951,7 @@ unsigned int nonstatic_short_count = fac->count[NONSTATIC_SHORT] - fac_contended.count[NONSTATIC_SHORT]; unsigned int nonstatic_byte_count = fac->count[NONSTATIC_BYTE] - fac_contended.count[NONSTATIC_BYTE]; unsigned int nonstatic_oop_count = fac->count[NONSTATIC_OOP] - fac_contended.count[NONSTATIC_OOP]; - - int static_value_type_count = 0; - int nonstatic_value_type_count = 0; - int* nonstatic_value_type_indexes = NULL; - Klass** nonstatic_value_type_klasses = NULL; - unsigned int value_type_oop_map_count = 0; - - int max_nonstatic_value_type = fac->count[NONSTATIC_VALUETYPE] + 1; - - nonstatic_value_type_indexes = NEW_RESOURCE_ARRAY_IN_THREAD(THREAD, int, - max_nonstatic_value_type); - for (int i = 0; i < max_nonstatic_value_type; i++) { - nonstatic_value_type_indexes[i] = -1; - } - nonstatic_value_type_klasses = NEW_RESOURCE_ARRAY_IN_THREAD(THREAD, Klass*, - max_nonstatic_value_type); - - for (AllFieldStream fs(_fields, _cp); !fs.done(); fs.next()) { - if (fs.allocation_type() == STATIC_VALUETYPE) { - static_value_type_count++; - } else if (fs.allocation_type() == NONSTATIC_VALUETYPE) { - Symbol* signature = fs.signature(); - Klass* klass = SystemDictionary::resolve_or_fail(signature, - Handle(THREAD, _loader_data->class_loader()), - _protection_domain, true, CHECK); - assert(klass != NULL, "Sanity check"); - assert(klass->access_flags().is_value_type(), "Value type expected"); - nonstatic_value_type_indexes[nonstatic_value_type_count] = fs.index(); - nonstatic_value_type_klasses[nonstatic_value_type_count] = klass; - nonstatic_value_type_count++; - - ValueKlass* vklass = ValueKlass::cast(klass); - if (vklass->contains_oops()) { - value_type_oop_map_count += vklass->nonstatic_oop_map_count(); - } - } - } + unsigned int nonstatic_value_type_count = fac->count[NONSTATIC_VALUETYPE]; // Total non-static fields count, including every contended field unsigned int nonstatic_fields_count = fac->count[NONSTATIC_DOUBLE] + fac->count[NONSTATIC_WORD] + @@ -4153,16 +4165,10 @@ break; case NONSTATIC_VALUETYPE: { - Klass* klass = nonstatic_value_type_klasses[next_value_type_index]; - assert(klass != NULL, "Klass should have been loaded and resolved earlier"); - assert(klass->access_flags().is_value_type(),"Must be a value type"); - ValueKlass* vklass = ValueKlass::cast(klass); real_offset = next_nonstatic_valuetype_offset; - next_nonstatic_valuetype_offset += (vklass->size_helper()) * wordSize - vklass->first_field_offset(); - // aligning next value type on a 64 bits boundary - next_nonstatic_valuetype_offset = align_size_up(next_nonstatic_valuetype_offset, BytesPerLong); - next_value_type_index += 1; + assert(next_value_type_index < large_value_count, "Incorrect number of value alloc types"); + ValueKlass* vklass = nonstatic_value_type_klasses[next_value_type_index++]; if (vklass->contains_oops()) { // add flatten oop maps int diff = real_offset - vklass->first_field_offset(); const OopMapBlock* map = vklass->start_of_nonstatic_oop_maps(); @@ -4172,6 +4178,7 @@ map++; } } + next_nonstatic_valuetype_offset += vklass->raw_value_byte_size(); } break; case NONSTATIC_OOP: @@ -4342,7 +4349,7 @@ } int notaligned_nonstatic_fields_end; - if (nonstatic_value_type_count != 0) { + if (has_nonstatic_value_fields) { notaligned_nonstatic_fields_end = next_nonstatic_valuetype_offset; } else { notaligned_nonstatic_fields_end = next_nonstatic_padded_offset; --- old/hotspot/src/share/vm/classfile/classFileParser.hpp 2017-06-29 10:45:52.554765881 +0200 +++ new/hotspot/src/share/vm/classfile/classFileParser.hpp 2017-06-29 10:45:52.394762988 +0200 @@ -494,7 +494,7 @@ // lays out fields in class and returns the total oopmap count void layout_fields(ConstantPool* cp, - const FieldAllocationCount* fac, + FieldAllocationCount* fac, const ClassAnnotationCollector* parsed_annotations, FieldLayoutInfo* info, TRAPS); --- old/hotspot/src/share/vm/oops/fieldInfo.hpp 2017-06-29 10:45:53.050774851 +0200 +++ new/hotspot/src/share/vm/oops/fieldInfo.hpp 2017-06-29 10:45:52.894772030 +0200 @@ -199,6 +199,15 @@ _shorts[high_packed_offset] = extract_high_short_from_int(val); } + // Values types may be coerced to a allocation different type + void coerce_allocation_type(int from, int type) { + assert(allocation_type() == from, "Unexpected from state from: %d type: %d", from, allocation_type()); + u2 lo = _shorts[low_packed_offset]; + _shorts[low_packed_offset] = ((type << FIELDINFO_TAG_SIZE)) & 0xFFFF; + _shorts[low_packed_offset] &= ~FIELDINFO_TAG_MASK; + _shorts[low_packed_offset] |= FIELDINFO_TAG_TYPE_PLAIN; + } + void set_allocation_type(int type) { u2 lo = _shorts[low_packed_offset]; switch(lo & FIELDINFO_TAG_MASK) { --- old/hotspot/src/share/vm/oops/fieldStreams.hpp 2017-06-29 10:45:53.538783677 +0200 +++ new/hotspot/src/share/vm/oops/fieldStreams.hpp 2017-06-29 10:45:53.370780638 +0200 @@ -161,6 +161,10 @@ return field()->allocation_type(); } + void coerce_allocation_type(int from, int type) { + return field()->coerce_allocation_type(from, type); + } + void set_offset(int offset) { field()->set_offset(offset); } --- old/hotspot/test/runtime/valhalla/valuetypes/ValueTypeCreation.java 2017-06-29 10:45:54.034792646 +0200 +++ new/hotspot/test/runtime/valhalla/valuetypes/ValueTypeCreation.java 2017-06-29 10:45:53.878789825 +0200 @@ -19,8 +19,8 @@ public void run() { testPoint(); testLong8(); - // Embedded oops not yet supported - //testPerson(); + testPerson(); + testComposite(); } void testPoint() { @@ -46,8 +46,107 @@ void testPerson() { Person person = Person.create(1, "John", "Smith"); - Asserts.assertEquals(person.getId(), 1L, "Id field incorrect"); + Asserts.assertEquals(person.getId(), 1, "Id field incorrect"); Asserts.assertEquals(person.getFirstName(), "John", "First name incorrect"); Asserts.assertEquals(person.getLastName(), "Smith", "Last name incorrect"); } + + void testComposite() { + short a = 3; + int b = 7; + SmallEmbed se = SmallEmbed.create((byte) 3, (byte) 7); + Long8Value long8Value = Long8Value.create(4711, 13, 3147, 11, 3, 1, 0, 8); + Person person = Person.create(11, "Jane", "Wayne"); + Composition comp = Composition.create(a, se, long8Value, person); + Composition.check(comp, a, se, long8Value, person); + ValueHolder holder = ValueHolder.create(a, b, se, comp); + ValueHolder.check(holder, a, b, se, comp); + } + + static final __ByValue class SmallEmbed { + final byte a; + final byte b; + + private SmallEmbed() { + a = (byte)0; + b = (byte)0; + } + + __ValueFactory static SmallEmbed create(byte a, byte b) { + SmallEmbed se = __MakeDefault SmallEmbed(); + se.a = a; + se.b = b; + return se; + } + + static void check(SmallEmbed value, byte a, byte b) { + Asserts.assertEquals(value.a, a, "Field a incorrect"); + Asserts.assertEquals(value.b, b, "Field a incorrect"); + } + } + + static final __ByValue class Composition { + final short a; + final SmallEmbed se; + final Long8Value long8Value; + final Person person; + + private Composition() { + a = (short) 0; + se = SmallEmbed.create((byte)0, (byte)0); + long8Value = Long8Value.create(0, 0, 0, 0, 0, 0, 0, 0); + person = Person.create(0, null, null); + } + + __ValueFactory static Composition create(short a, SmallEmbed se, Long8Value long8Value, Person person) { + Composition composition = __MakeDefault Composition(); + composition.a = a; + composition.se = se; + composition.long8Value = long8Value; + composition.person = person; + return composition; + } + + static void check(Composition value, short a, SmallEmbed se, Long8Value long8Value, Person person) { + Asserts.assertEquals(value.a, a, "Field a incorrect"); + SmallEmbed.check(value.se, se.a, se.b); + Long8Value.check(value.long8Value, + long8Value.longField1, + long8Value.longField2, + long8Value.longField3, + long8Value.longField4, + long8Value.longField5, + long8Value.longField6, + long8Value.longField7, + long8Value.longField8); + Asserts.assertEquals(value.person.id, person.id, "Person.id"); + Asserts.assertEquals(value.person.firstName, person.firstName, "Person.firstName"); + Asserts.assertEquals(value.person.lastName, person.lastName, "Person.lastName"); + } + } + + static class ValueHolder { + short a; + int b; + SmallEmbed small; + Composition comp; + + ValueHolder(short a, int b, SmallEmbed small, Composition comp) { + this.a = a; + this.b = b; + this.small = small; + this.comp = comp; + } + + static ValueHolder create(short a, int b, SmallEmbed small, Composition comp) { + return new ValueHolder(a, b, small, comp); + } + + static void check(ValueHolder holder, short a, int b, SmallEmbed small, Composition comp) { + Asserts.assertEquals(holder.a, a, "Field a incorrect"); + Asserts.assertEquals(holder.b, b, "Field b incorrect"); + SmallEmbed.check(holder.small, small.a, small.b); + Composition.check(holder.comp, comp.a, comp.se, comp.long8Value, comp.person); + } + } }