smart_ptr::detail::as_allocator::allocate() adds alignment padding when it's not needed

[David Aue <david@…>]

smart_ptr::detail::as_allocator::allocate() {

enum {

M = boost::alignment_of<type>::value

}; std::size_t n1 = count * sizeof(value_type); std::size_t n2 = data.size * sizeof(type); std::size_t n3 = n2 + M; CA ca(allocator()); void* p1 = ca.allocate(n1 + n3);

Allocation is for two items of size n1 and n2. Alignment size is added to n2 to create n3 to there is room to align n2 properly.

if n1 % M == 0 then alignment is already fine and the extra M bytes is wasted.

[Peter Dimov]

I don't see why. We can only depend on the alignment of p1 as being suitable for an object of type value_type. n1 % M == 0 doesn't seem to give us anything.

[David Aue <david@…>]

pointer allocate(size_type count, const_void_pointer = 0) {
    enum {
        M = boost::alignment_of<type>::value
    };
    std::size_t n1 = count * sizeof(value_type);
    std::size_t n2 = data.size * sizeof(type);
    std::size_t n3 = n2 + M;
    CA ca(allocator());
    void* p1 = ca.allocate(n1 + n3);
    void* p2 = static_cast<char*>(p1) + n1;
    (void)boost::alignment::align(M, n2, p2, n3);
    *data.result = static_cast<type*>(p2);
    return static_cast<value_type*>(p1);
}


I call:
boost::allocate_shared<int[]>(allocator, 4);

Inside allocate() I see this:

n1 == 20 for ms_init_tag
n2 == 16 for the 4 ints
M == 4
n3 == 20

ca.allocate is called with n1+n3 for a total of 40 bytes.

p1 == some_address
p2 == some_address + n1
boost::alignment::align called to fix p2 for 4 byte alignment
p2 is unchanged by align

So we allocated 40 bytes and we are using the first 20 for ms_init and the next 16 for the int[4] and then there are 4 unused bytes that are not affecting our 4 byte alignment requirement.

[Glen Fernandes]

One optimization I could do is instead of adding alignof(type) bytes:

• If alignof(value_type) >= align(type) then do not add any additional padding.
• Else add alignof(type) - 1 bytes for padding.

[David Aue <david@…>]

That sounds perfect.

[Glen Fernandes]

In fact, one step further:

• If alignof(value_type) >= align(type) then do not add any additional padding.
• Else add alignof(type) - alignof(value_type) bytes for padding.

[Peter Dimov]

CA is allocator<char> and there is no guarantee for p1 to be aligned at anything, regardless of what the alignment of value_type is.

Although I suppose that the code already assumes that p1 is aligned at alignof(value_type) anyway. But this assumption doesn't seem quite correct.

[Glen Fernandes]

Good point. The above math was under the assumption that for any value_type the allocate() function for any given allocator would yield memory that is at least alignof(std::max_align_t) bytes aligned.

It doesn't seem right to make this assumption, and nothing in [allocator.requirements] provides such a guarantee anyway, so I'll update the implementation accordingly.

[Peter Dimov]

We should probably use ::rebind<type_with_alignment<alignof(value_type)>::type> to allocate, and then the optimization would hold.

[Glen Fernandes]

But type_with_alignment<alignof(value_type)>::type wouldn't have size 1, right? It would have size at least alignof(value_type) bytes and allocate() takes number of objects.

[Peter Dimov]

Yes, of course. The number of objects would need to be adjusted accordingly.

[Glen Fernandes]

This should be fixed in develop now after I rewrote all of allocate_shared for arrays. It will probably release with 1.65 unless the tests have enough time to cycle for 1.64 and there are no issues.

[Glen Fernandes]

Will release in 1.64.