Remove an assertion in allocation

[qinsoon]

This PR removes an assertion in the allocation that checks if size is a multiple of VM::MIN_ALIGNMENT. This assertion would fail for cases like allocating an object of 12 bytes with MIN_ALIGNMENT=8, which is actually reasonable. This assertion is not necessary, and it is very possibly that the code is from Java MMTk and is Java specific.

[caizixian]

Here's the API reference for the old MMTk https://www.jikesrvm.org/JavaDoc/org/mmtk/utility/alloc/Allocator.html#getMaximumAlignedSize-int-int-int-

[qinsoon]

Here's the API reference for the old MMTk https://www.jikesrvm.org/JavaDoc/org/mmtk/utility/alloc/Allocator.html#getMaximumAlignedSize-int-int-int-

I was aware of that method and the comment. However, the comment merely says there is such an assertion but does not justify why that assertion is needed.

[caizixian]

Here's my understanding: getMaximumAlignedSize estimates how much memory is needed to allocate an object. The result is then used to pick the appropriate allocator. The estimation is simple: if we have size + alignment - known_alignment free space left, then no matter where the cursor is, we can guarantee we have enough space left. An optimization is that, if we know that object sizes are multiples of known_alignment and the required alignment is <= known_alignment, then we can simply return the object size.

    if VM::MAX_ALIGNMENT <= VM::MIN_ALIGNMENT || alignment <= known_alignment {
        size
    } else {
        size + alignment - known_alignment
    }

Therefore, if you have size = 12, known_alignment=8, and alignment=8, the return value of the function (12) is wrong if you remove the assertion. 12 bytes is not enough to guarantee an alignment of 8.

[caizixian]

I think you should pass appropriate value as known_alignment for different VMs rather than deleting the assertion.

[qinsoon]

Here's my understanding: getMaximumAlignedSize estimates how much memory is needed to allocate an object. The result is then used to pick the appropriate allocator. The estimation is simple: if we have size + alignment - known_alignment free space left, then no matter where the cursor is, we can guarantee we have enough space left. An optimization is that, if we know that object sizes are multiples of known_alignment and the required alignment is <= known_alignment, then we can simply return the object size.

    if VM::MAX_ALIGNMENT <= VM::MIN_ALIGNMENT || alignment <= known_alignment {
        size
    } else {
        size + alignment - known_alignment
    }

Therefore, if you have size = 12, known_alignment=8, and alignment=8, the return value of the function (12) is wrong if you remove the assertion. 12 bytes is not enough to guarantee an alignment of 8.

I think you are right. I need to update the function implementation as well.

I further checked the Java MMTk code. getMaximumAlignedSize() is an upper-bound of the possible allocation size, which means actual size <= getMaximumAlignedSize(). For each allocation, we first check allocator based on getMaximumAlignedSize(). If the allocator is large object allocator, or free list allocator, we will actually allocate using getMaximumAlignedSize() due to the fact that each cell in those allocators are aligned and fix-sized. If the allocator is any other allocator, we will allocate the actual size, and do alignment - this has nothing to do with the getMaximumAlignedSize() any more.

I will update this PR to include more changes and related tests. Anyway, our goal is to allow allocation size that is not a multiple of min_alignment. If that requires more than just removing the assertion or changing this method, I will close this PR, create an issue for this and do another PR later for what is needed.

[qinsoon]

There is one more difference between mmtk-core and Java MMTk: mmtk-core no longer does checkAllocator(), as that is the binding's job. So that means, getMaximumAlignedSize() would only be used in large object allocator or freelist allocator to make sure we can do alignment in the size we required (maximum aligned size).

[qinsoon]

I am closing this PR. https://github.com/mmtk/mmtk-core/issues/730 describes the problem.