From 6d013cc7beaf37a3f4d10100788daae3e6348cc2 Mon Sep 17 00:00:00 2001 From: Peter Hofer Date: Tue, 20 Dec 2022 11:28:26 +0100 Subject: [PATCH 1/2] Use gaps in superclasses for fields, including monitor fields. --- .../svm/hosted/HostedConfiguration.java | 6 +- .../svm/hosted/meta/UniverseBuilder.java | 185 +++++++++++------- 2 files changed, 119 insertions(+), 72 deletions(-) diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/HostedConfiguration.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/HostedConfiguration.java index 233b0af0fe71..462f302db699 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/HostedConfiguration.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/HostedConfiguration.java @@ -172,8 +172,7 @@ public MetaAccessExtensionProvider createCompilationMetaAccessExtensionProvider( public void findAllFieldsForLayout(HostedUniverse universe, @SuppressWarnings("unused") HostedMetaAccess metaAccess, @SuppressWarnings("unused") Map universeFields, - ArrayList rawFields, - ArrayList orderedFields, HostedInstanceClass clazz) { + ArrayList rawFields, ArrayList allFields, HostedInstanceClass clazz) { for (AnalysisField aField : clazz.getWrapped().getInstanceFields(false)) { HostedField hField = universe.lookup(aField); @@ -184,9 +183,10 @@ public void findAllFieldsForLayout(HostedUniverse universe, @SuppressWarnings("u * The array or bitset field of a hybrid is not materialized, so it needs no * field offset. */ - orderedFields.add(hField); + allFields.add(hField); } else if (hField.isAccessed()) { rawFields.add(hField); + allFields.add(hField); } } } diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/meta/UniverseBuilder.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/meta/UniverseBuilder.java index 6dd5b9299ab5..d7e3f7d549c2 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/meta/UniverseBuilder.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/meta/UniverseBuilder.java @@ -31,6 +31,7 @@ import java.util.BitSet; import java.util.Collection; import java.util.Collections; +import java.util.Comparator; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -419,7 +420,9 @@ public static boolean isKnownImmutableType(Class clazz) { } private void layoutInstanceFields() { - layoutInstanceFields(hUniverse.getObjectClass(), ConfigurationValues.getObjectLayout().getFirstFieldOffset(), new HostedField[0], false); + BitSet usedBytes = new BitSet(); + usedBytes.set(0, ConfigurationValues.getObjectLayout().getFirstFieldOffset()); + layoutInstanceFields(hUniverse.getObjectClass(), new HostedField[0], false, usedBytes); } private static boolean mustReserveLengthField(HostedInstanceClass clazz) { @@ -434,44 +437,50 @@ private static boolean mustReserveLengthField(HostedInstanceClass clazz) { return false; } - private void layoutInstanceFields(HostedInstanceClass clazz, int superSize, HostedField[] superFields, boolean superFieldsContendedPadding) { + private static void reserve(BitSet usedBytes, int offset, int size) { + int offsetAfter = offset + size; + assert usedBytes.previousSetBit(offsetAfter - 1) < offset; // (also matches -1) + usedBytes.set(offset, offsetAfter); + } + + private static void reserveAtEnd(BitSet usedBytes, int size) { + int endOffset = usedBytes.length(); + usedBytes.set(endOffset, endOffset + size); + } + + private void layoutInstanceFields(HostedInstanceClass clazz, HostedField[] superFields, boolean superFieldsContendedPadding, BitSet usedBytes) { ArrayList rawFields = new ArrayList<>(); - ArrayList orderedFields = new ArrayList<>(); + ArrayList allFields = new ArrayList<>(); ObjectLayout layout = ConfigurationValues.getObjectLayout(); - HostedConfiguration.instance().findAllFieldsForLayout(hUniverse, hMetaAccess, hUniverse.fields, rawFields, orderedFields, clazz); + HostedConfiguration.instance().findAllFieldsForLayout(hUniverse, hMetaAccess, hUniverse.fields, rawFields, allFields, clazz); - int startSize = superSize; + int superSize = usedBytes.length(); if (clazz.getAnnotation(DeoptimizedFrame.ReserveDeoptScratchSpace.class) != null) { - assert startSize <= DeoptimizedFrame.getScratchSpaceOffset(); - startSize = DeoptimizedFrame.getScratchSpaceOffset() + layout.getDeoptScratchSpace(); + reserve(usedBytes, DeoptimizedFrame.getScratchSpaceOffset(), layout.getDeoptScratchSpace()); } if (mustReserveLengthField(clazz)) { - VMError.guarantee(startSize == layout.getArrayLengthOffset()); - int fieldSize = layout.sizeInBytes(JavaKind.Int); - startSize += fieldSize; + int lengthOffset = layout.getArrayLengthOffset(); + int lengthSize = layout.sizeInBytes(JavaKind.Int); + reserve(usedBytes, lengthOffset, lengthSize); - /* - * Set start after typecheck slots field, if the hybrid class has one. For now, only - * DynamicHubs can have such field(s). - */ + // Type check fields in DynamicHub. if (clazz.equals(hMetaAccess.lookupJavaType(DynamicHub.class))) { /* Each type check id slot is 2 bytes. */ - startSize += typeCheckBuilder.getNumTypeCheckSlots() * 2; + int slotsSize = typeCheckBuilder.getNumTypeCheckSlots() * 2; + reserve(usedBytes, lengthOffset + lengthSize, slotsSize); } } - int nextOffset = startSize; - boolean hasLeadingPadding = superFieldsContendedPadding; boolean isClassContended = clazz.isAnnotationPresent(Contended.class); if (!hasLeadingPadding && (isClassContended || (clazz.getSuperclass() != null && clazz.getSuperclass().isAnnotationPresent(Contended.class)))) { - nextOffset += getContendedPadding(); + reserveAtEnd(usedBytes, getContendedPadding()); hasLeadingPadding = true; } - int beginOfFieldsOffset = nextOffset; + int sizeWithoutFields = usedBytes.length(); /* * Sort and group fields so that fields marked @Contended(tag) are grouped by their tag, @@ -487,99 +496,137 @@ private void layoutInstanceFields(HostedInstanceClass clazz, int superSize, Host ArrayList uncontendedFields = contentionGroups.remove(uncontendedSentinel); if (uncontendedFields != null) { assert !uncontendedFields.isEmpty(); - nextOffset = placeFields(uncontendedFields, nextOffset, orderedFields, layout); + placeFields(uncontendedFields, usedBytes, 0, layout); } for (ArrayList groupFields : contentionGroups.values()) { - boolean placedFieldsBefore = (nextOffset != beginOfFieldsOffset); + boolean placedFieldsBefore = (usedBytes.length() != sizeWithoutFields); if (placedFieldsBefore || !hasLeadingPadding) { - nextOffset += getContendedPadding(); + reserveAtEnd(usedBytes, getContendedPadding()); } - nextOffset = placeFields(groupFields, nextOffset, orderedFields, layout); + int firstOffset = usedBytes.length(); + placeFields(groupFields, usedBytes, firstOffset, layout); + usedBytes.set(firstOffset, usedBytes.length()); // prevent subclass fields } boolean fieldsTrailingPadding = !contentionGroups.isEmpty(); if (fieldsTrailingPadding) { - nextOffset += getContendedPadding(); + reserveAtEnd(usedBytes, getContendedPadding()); } - int endOfFieldsOffset = nextOffset; + if (isClassContended) { // prevent injection of any subclass fields + usedBytes.set(superSize, usedBytes.length()); + } + BitSet usedBytesInSubclasses = null; + if (clazz.subTypes.length != 0) { + usedBytesInSubclasses = (BitSet) usedBytes.clone(); + } - /* - * Compute the offsets of the "synthetic" fields for this class (but not subclasses). - * Synthetic fields are put after all the instance fields. They are included in the instance - * size, but not in the offset passed to subclasses. - */ + // Reserve "synthetic" fields in this class (but not subclasses) below. + int sizeWithoutSyntheticFields = usedBytes.length(); // A reference to a {@link java.util.concurrent.locks.ReentrantLock for "synchronized" or // Object.wait() and Object.notify() and friends. if (clazz.needMonitorField()) { - final int referenceFieldAlignmentAndSize = layout.getReferenceSize(); - nextOffset = NumUtil.roundUp(nextOffset, referenceFieldAlignmentAndSize); - clazz.setMonitorFieldOffset(nextOffset); - nextOffset += referenceFieldAlignmentAndSize; + int size = layout.getReferenceSize(); + int endOffset = usedBytes.length(); + int offset = findGapForField(usedBytes, 0, size, endOffset); + if (offset == -1) { + offset = endOffset + getAlignmentAdjustment(endOffset, size); + } + reserve(usedBytes, offset, size); + clazz.setMonitorFieldOffset(offset); } - boolean placedSyntheticFields = (nextOffset != endOfFieldsOffset); - if (isClassContended && (contentionGroups.isEmpty() || placedSyntheticFields)) { - nextOffset += getContendedPadding(); + boolean appendedSyntheticFields = (sizeWithoutSyntheticFields != usedBytes.length()); + if (isClassContended && (contentionGroups.isEmpty() || appendedSyntheticFields)) { + reserveAtEnd(usedBytes, getContendedPadding()); } - clazz.instanceFieldsWithoutSuper = orderedFields.toArray(new HostedField[orderedFields.size()]); - clazz.instanceSize = layout.alignUp(nextOffset); - clazz.afterFieldsOffset = nextOffset; + /* + * This sequence of fields is returned by ResolvedJavaType.getInstanceFields, which requires + * them to in "natural order", i.e., sorted by location (offset). + */ + allFields.sort(FIELD_LOCATION_COMPARATOR); + + clazz.instanceFieldsWithoutSuper = allFields.toArray(new HostedField[0]); + clazz.afterFieldsOffset = usedBytes.length(); + clazz.instanceSize = layout.alignUp(clazz.afterFieldsOffset); if (clazz.instanceFieldsWithoutSuper.length == 0) { clazz.instanceFieldsWithSuper = superFields; } else if (superFields.length == 0) { clazz.instanceFieldsWithSuper = clazz.instanceFieldsWithoutSuper; } else { - clazz.instanceFieldsWithSuper = Arrays.copyOf(superFields, superFields.length + clazz.instanceFieldsWithoutSuper.length); - System.arraycopy(clazz.instanceFieldsWithoutSuper, 0, clazz.instanceFieldsWithSuper, superFields.length, clazz.instanceFieldsWithoutSuper.length); + HostedField[] instanceFieldsWithSuper = Arrays.copyOf(superFields, superFields.length + clazz.instanceFieldsWithoutSuper.length); + System.arraycopy(clazz.instanceFieldsWithoutSuper, 0, instanceFieldsWithSuper, superFields.length, clazz.instanceFieldsWithoutSuper.length); + // Fields must be sorted by location, and we might have put a field between super fields + Arrays.sort(instanceFieldsWithSuper, FIELD_LOCATION_COMPARATOR); + clazz.instanceFieldsWithSuper = instanceFieldsWithSuper; } for (HostedType subClass : clazz.subTypes) { if (subClass.isInstanceClass()) { - /* - * Derived classes ignore the monitor field of the super-class and start the layout - * of their fields right after the instance fields of the super-class. This is - * possible because each class that needs a synthetic field gets its own synthetic - * field at the end of its instance fields. - */ - layoutInstanceFields((HostedInstanceClass) subClass, endOfFieldsOffset, clazz.instanceFieldsWithSuper, fieldsTrailingPadding); + layoutInstanceFields((HostedInstanceClass) subClass, clazz.instanceFieldsWithSuper, fieldsTrailingPadding, (BitSet) usedBytesInSubclasses.clone()); } } } + private static final Comparator FIELD_LOCATION_COMPARATOR = (a, b) -> { + if (!a.hasLocation() || !b.hasLocation()) { // hybrid fields + return Boolean.compare(a.hasLocation(), b.hasLocation()); + } + return Integer.compare(a.getLocation(), b.getLocation()); + }; + private static int getContendedPadding() { Integer value = SubstrateOptions.ContendedPaddingWidth.getValue(); return (value > 0) ? value : 0; // no alignment required, placing fields takes care of it } - private static int placeFields(ArrayList fields, int firstOffset, ArrayList orderedFields, ObjectLayout layout) { - int nextOffset = firstOffset; - while (!fields.isEmpty()) { - boolean progress = false; - for (int i = 0; i < fields.size(); i++) { - HostedField field = fields.get(i); - int fieldSize = layout.sizeInBytes(field.getStorageKind()); - - if (nextOffset % fieldSize == 0) { - field.setLocation(nextOffset); - nextOffset += fieldSize; + private static void placeFields(ArrayList fields, BitSet usedBytes, int minOffset, ObjectLayout layout) { + int lastSearchSize = -1; + boolean lastSearchSuccess = false; + for (HostedField field : fields) { + int fieldSize = layout.sizeInBytes(field.getStorageKind()); + int offset = -1; + int endOffset = usedBytes.length(); + if (lastSearchSuccess || lastSearchSize != fieldSize) { + offset = findGapForField(usedBytes, minOffset, fieldSize, endOffset); + lastSearchSuccess = (offset != -1); + lastSearchSize = fieldSize; + } + if (offset == -1) { + offset = endOffset + getAlignmentAdjustment(endOffset, fieldSize); + } + reserve(usedBytes, offset, fieldSize); + field.setLocation(offset); + } + } - fields.remove(i); - orderedFields.add(field); - progress = true; - break; + private static int findGapForField(BitSet usedBytes, int minOffset, int fieldSize, int endOffset) { + int candidateOffset = -1; + int candidateSize = -1; + int offset = usedBytes.nextClearBit(minOffset); + while (offset < endOffset) { + int size = usedBytes.nextSetBit(offset + 1) - offset; + int adjustment = getAlignmentAdjustment(offset, fieldSize); + if (size >= adjustment + fieldSize) { // fit + if (candidateOffset == -1 || size < candidateSize) { // better fit + candidateOffset = offset + adjustment; + candidateSize = size; } } - if (!progress) { - // Insert padding byte and try again. - nextOffset++; - } + offset = usedBytes.nextClearBit(offset + size); } - return nextOffset; + return candidateOffset; + } + + private static int getAlignmentAdjustment(int offset, int alignment) { + int bits = alignment - 1; + assert (alignment & bits) == 0 : "expecting power of 2"; + int alignedOffset = (offset + bits) & ~bits; + return alignedOffset - offset; } private void layoutStaticFields() { From 546da18b049e463139c60a2a9d8cda509d17fcd7 Mon Sep 17 00:00:00 2001 From: Peter Hofer Date: Mon, 9 Jan 2023 14:45:47 +0100 Subject: [PATCH 2/2] Fix and simplify @Contended handling. --- .../svm/hosted/meta/UniverseBuilder.java | 47 ++++++------------- 1 file changed, 15 insertions(+), 32 deletions(-) diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/meta/UniverseBuilder.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/meta/UniverseBuilder.java index d7e3f7d549c2..fda323e5b647 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/meta/UniverseBuilder.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/meta/UniverseBuilder.java @@ -422,7 +422,7 @@ public static boolean isKnownImmutableType(Class clazz) { private void layoutInstanceFields() { BitSet usedBytes = new BitSet(); usedBytes.set(0, ConfigurationValues.getObjectLayout().getFirstFieldOffset()); - layoutInstanceFields(hUniverse.getObjectClass(), new HostedField[0], false, usedBytes); + layoutInstanceFields(hUniverse.getObjectClass(), new HostedField[0], usedBytes); } private static boolean mustReserveLengthField(HostedInstanceClass clazz) { @@ -448,14 +448,13 @@ private static void reserveAtEnd(BitSet usedBytes, int size) { usedBytes.set(endOffset, endOffset + size); } - private void layoutInstanceFields(HostedInstanceClass clazz, HostedField[] superFields, boolean superFieldsContendedPadding, BitSet usedBytes) { + private void layoutInstanceFields(HostedInstanceClass clazz, HostedField[] superFields, BitSet usedBytes) { ArrayList rawFields = new ArrayList<>(); ArrayList allFields = new ArrayList<>(); ObjectLayout layout = ConfigurationValues.getObjectLayout(); HostedConfiguration.instance().findAllFieldsForLayout(hUniverse, hMetaAccess, hUniverse.fields, rawFields, allFields, clazz); - int superSize = usedBytes.length(); if (clazz.getAnnotation(DeoptimizedFrame.ReserveDeoptScratchSpace.class) != null) { reserve(usedBytes, DeoptimizedFrame.getScratchSpaceOffset(), layout.getDeoptScratchSpace()); } @@ -473,22 +472,19 @@ private void layoutInstanceFields(HostedInstanceClass clazz, HostedField[] super } } - boolean hasLeadingPadding = superFieldsContendedPadding; - boolean isClassContended = clazz.isAnnotationPresent(Contended.class); - if (!hasLeadingPadding && (isClassContended || (clazz.getSuperclass() != null && clazz.getSuperclass().isAnnotationPresent(Contended.class)))) { - reserveAtEnd(usedBytes, getContendedPadding()); - hasLeadingPadding = true; - } - - int sizeWithoutFields = usedBytes.length(); - /* - * Sort and group fields so that fields marked @Contended(tag) are grouped by their tag, - * placing unannotated fields first in the object, and so that within groups, a) all Object - * fields are consecutive, and b) bigger types come first. + * Group fields annotated @Contended(tag) by their tag, where a tag of "" implies a group + * for each individual field. Each group gets padded at the beginning and end to avoid false + * sharing. Unannotated fields are placed in a separate group which is not padded unless the + * class itself is annotated @Contended. + * + * Sort so that in each group, Object fields are consecutive, and bigger types come first. */ Object uncontendedSentinel = new Object(); - Function getAnnotationGroup = field -> Optional.ofNullable(field.getAnnotation(Contended.class)). map(Contended::value).orElse(uncontendedSentinel); + Object unannotatedGroup = clazz.isAnnotationPresent(Contended.class) ? new Object() : uncontendedSentinel; + Function getAnnotationGroup = field -> Optional.ofNullable(field.getAnnotation(Contended.class)) + .map(a -> "".equals(a.value()) ? new Object() : a.value()) + .orElse(unannotatedGroup); Map> contentionGroups = rawFields.stream() .sorted(HostedUniverse.FIELD_COMPARATOR_RELAXED) .collect(Collectors.groupingBy(getAnnotationGroup, Collectors.toCollection(ArrayList::new))); @@ -500,30 +496,22 @@ private void layoutInstanceFields(HostedInstanceClass clazz, HostedField[] super } for (ArrayList groupFields : contentionGroups.values()) { - boolean placedFieldsBefore = (usedBytes.length() != sizeWithoutFields); - if (placedFieldsBefore || !hasLeadingPadding) { - reserveAtEnd(usedBytes, getContendedPadding()); - } + reserveAtEnd(usedBytes, getContendedPadding()); int firstOffset = usedBytes.length(); placeFields(groupFields, usedBytes, firstOffset, layout); usedBytes.set(firstOffset, usedBytes.length()); // prevent subclass fields } - boolean fieldsTrailingPadding = !contentionGroups.isEmpty(); - if (fieldsTrailingPadding) { + if (!contentionGroups.isEmpty()) { reserveAtEnd(usedBytes, getContendedPadding()); } - if (isClassContended) { // prevent injection of any subclass fields - usedBytes.set(superSize, usedBytes.length()); - } BitSet usedBytesInSubclasses = null; if (clazz.subTypes.length != 0) { usedBytesInSubclasses = (BitSet) usedBytes.clone(); } // Reserve "synthetic" fields in this class (but not subclasses) below. - int sizeWithoutSyntheticFields = usedBytes.length(); // A reference to a {@link java.util.concurrent.locks.ReentrantLock for "synchronized" or // Object.wait() and Object.notify() and friends. @@ -538,11 +526,6 @@ private void layoutInstanceFields(HostedInstanceClass clazz, HostedField[] super clazz.setMonitorFieldOffset(offset); } - boolean appendedSyntheticFields = (sizeWithoutSyntheticFields != usedBytes.length()); - if (isClassContended && (contentionGroups.isEmpty() || appendedSyntheticFields)) { - reserveAtEnd(usedBytes, getContendedPadding()); - } - /* * This sequence of fields is returned by ResolvedJavaType.getInstanceFields, which requires * them to in "natural order", i.e., sorted by location (offset). @@ -567,7 +550,7 @@ private void layoutInstanceFields(HostedInstanceClass clazz, HostedField[] super for (HostedType subClass : clazz.subTypes) { if (subClass.isInstanceClass()) { - layoutInstanceFields((HostedInstanceClass) subClass, clazz.instanceFieldsWithSuper, fieldsTrailingPadding, (BitSet) usedBytesInSubclasses.clone()); + layoutInstanceFields((HostedInstanceClass) subClass, clazz.instanceFieldsWithSuper, (BitSet) usedBytesInSubclasses.clone()); } } }