Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8307376

Alignment check on layouts used as sequence element is not correct

XMLWordPrintable

    • Icon: CSR CSR
    • Resolution: Approved
    • Icon: P2 P2
    • 21
    • core-libs
    • None
    • behavioral
    • minimal
    • The changes proposed in this CSR will make ill-behaved layouts illegal at construction. As such they will make some errors show up early in a program (which is generally a good thing).
    • Java API
    • SE

      Summary

      Sometimes layouts are used as elements in a "sequence" where the element is repeated, back to back, several times. In such cases the FFM API has to validate that the layout has certain properties when it comes to alignment, to preserve alignment under strided access.

      Problem

      Consider the layout for a Java int (e.g. ValueLayout.JAVA_INT). This layout is 4 bytes long and is aligned to 4 bytes. When creating a sequence layout from this Java int layout, everything is correct. After all, the size of the Java int layout is a multiple of its alignment (trivially). This ensures that strided access will not violate alignment constraints. E.g. accessing the 3rd element of a sequence of Java int layouts is guaranteed to be aligned (if the first element is aligned, that is).

      There are cases where this condition does not hold. Consider this layout expression:

      ValueLayout.JAVA_INT.withBitAlignment(64)

      We now have a layout that has an alignment constraint bigger than the size. This means that if we use this layout as a sequence element, access to the first sequence element might be aligned, but access to the second sequence element will not be (as the second element will be at offset 32, which is not compatible with an alignment constraint of 64).

      When considering value layouts, since the size and alignment of a value layout are guaranteed to be power of two (by construction), it is sufficient to rule out cases of a sequence element layout whose size is smaller than its alignment constraint. This is what the implementation does.

      Unfortunately, such a simple check does not scale to the general case where the sequence element layout can just be any layout. Consider the following layout expression:

      MemoryLayout.structLayout(ValueLayout.JAVA_INT, ValueLayout.JAVA_BYTE)

      Here we have a layout that has an alignment constraint set to 32 (as that is strictest alignment constraint among the struct layout members). The size of this layout is 40 (32 + 8). So, while it is true that the layout size is, in this case, bigger than the alignment constraint (40 > 32), this layout is not suitable to be used as a sequence element layout.

      Solution

      The solution is to tweak the implementation so that the correct alignment check is performed depending on which layout L is being tested:

      • if L is a value layout, then checking that L.byteSize() > L.byteAlignment() is still correct;
      • If L is some other layout, a slower check must be performed, specifically to ensure that L.byteSize() % L.bitAlignment() == 0

      Specification

      Below we report the new javadoc of the affected methods. Aside from some streamlining of the javadoc text, the important difference lies in the @throws clauses.

          /**
           * Creates a sequence layout with the given element layout and element count.
           *
           * @param elementCount the sequence element count.
           * @param elementLayout the sequence element layout.
           * @return the new sequence layout with the given element layout and size.
           * @throws IllegalArgumentException if {@code elementCount } is negative.
           * @throws IllegalArgumentException if {@code elementLayout.bitSize() % elementLayout.bitAlignment() != 0}.
           */
          static SequenceLayout sequenceLayout(long elementCount, MemoryLayout elementLayout) { ... }
          /**
           * Creates a sequence layout with the given element layout and the maximum element
           * count such that it does not overflow a {@code long}.
           *
           * This is equivalent to the following code:
           * {@snippet lang = java:
           * sequenceLayout(Long.MAX_VALUE / elementLayout.bitSize(), elementLayout);
           * }
           *
           * @param elementLayout the sequence element layout.
           * @return a new sequence layout with the given element layout and maximum element count.
           * @throws IllegalArgumentException if {@code elementLayout.bitSize() % elementLayout.bitAlignment() != 0}.
           */
          static SequenceLayout sequenceLayout(MemoryLayout elementLayout) { ... }
          /**
           * Returns a spliterator for this memory segment. The returned spliterator reports {@link Spliterator#SIZED},
           * {@link Spliterator#SUBSIZED}, {@link Spliterator#IMMUTABLE}, {@link Spliterator#NONNULL} and {@link Spliterator#ORDERED}
           * characteristics.
           * <p>
           * The returned spliterator splits this segment according to the specified element layout; that is,
           * if the supplied layout has size N, then calling {@link Spliterator#trySplit()} will result in a spliterator serving
           * approximately {@code S/N} elements (depending on whether N is even or not), where {@code S} is the size of
           * this segment. As such, splitting is possible as long as {@code S/N >= 2}. The spliterator returns segments that
           * have the same lifetime as that of this segment.
           * <p>
           * The returned spliterator effectively allows to slice this segment into disjoint {@linkplain #asSlice(long, long) slices},
           * which can then be processed in parallel by multiple threads.
           *
           * @param elementLayout the layout to be used for splitting.
           * @return the element spliterator for this segment
           * @throws IllegalArgumentException if {@code elementLayout.byteSize() == 0}.
           * @throws IllegalArgumentException if {@code byteSize() % elementLayout.byteSize() != 0}.
           * @throws IllegalArgumentException if {@code elementLayout.bitSize() % elementLayout.bitAlignment() != 0}.
           * @throws IllegalArgumentException if this segment is <a href="MemorySegment.html#segment-alignment">incompatible
           * with the alignment constraint</a> in the provided layout.
           */
          Spliterator<MemorySegment> spliterator(MemoryLayout elementLayout) { ... }
          /**
           * Returns a sequential {@code Stream} over disjoint slices (whose size matches that of the specified layout)
           * in this segment. Calling this method is equivalent to the following code:
           * {@snippet lang=java :
           * StreamSupport.stream(segment.spliterator(elementLayout), false);
           * }
           *
           * @param elementLayout the layout to be used for splitting.
           * @return a sequential {@code Stream} over disjoint slices in this segment.
           * @throws IllegalArgumentException if {@code elementLayout.byteSize() == 0}.
           * @throws IllegalArgumentException if {@code byteSize() % elementLayout.byteSize() != 0}.
           * @throws IllegalArgumentException if {@code elementLayout.bitSize() % elementLayout.bitAlignment() != 0}.
           * @throws IllegalArgumentException if this segment is <a href="MemorySegment.html#segment-alignment">incompatible
           * with the alignment constraint</a> in the provided layout.
           */
          Stream<MemorySegment> elements(MemoryLayout elementLayout) { ... }

            mcimadamore Maurizio Cimadamore
            mcimadamore Maurizio Cimadamore
            Jorn Vernee
            Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

              Created:
              Updated:
              Resolved: