Boost C++ Libraries: Ticket #11312: smart_ptr::detail::as_allocator::allocate() adds alignment padding when it's not needed

Peter Dimov — Fri, 15 May 2015 23:28:17 GMT

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.

owner changed

Peter Dimov — Fri, 15 May 2015 23:29:40 GMT

owner changed from Peter Dimov to Glen Fernandes

Sat, 16 May 2015 00:31:47 GMT

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 — Sun, 17 May 2015 05:08:16 GMT

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.

Sun, 17 May 2015 06:07:29 GMT

That sounds perfect.

Glen Fernandes — Sun, 17 May 2015 06:42:57 GMT

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 — Sun, 17 May 2015 10:20:00 GMT

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 — Sun, 17 May 2015 16:30:07 GMT

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 — Sun, 17 May 2015 17:15:53 GMT

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

Glen Fernandes — Sun, 17 May 2015 18:10:04 GMT

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 — Mon, 18 May 2015 10:39:20 GMT

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

version changed

Glen Fernandes — Thu, 08 Sep 2016 15:53:13 GMT

version Boost 1.57.0 → Boost 1.63.0

status, milestone changed; resolution set

Glen Fernandes — Wed, 01 Mar 2017 13:15:29 GMT

status new → closed
resolution → fixed
milestone To Be Determined → Boost 1.64.0

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.

version, milestone changed

Glen Fernandes — Fri, 03 Mar 2017 13:36:14 GMT

version Boost 1.63.0 → Boost 1.57.0
milestone Boost 1.64.0 → Boost 1.65.0

Will release in 1.64.

milestone changed

Glen Fernandes — Sun, 26 Mar 2017 21:54:57 GMT

milestone Boost 1.65.0 → Boost 1.64.0