boostorg / uuid

Boost.org uuid module
http://boost.org/libs/uuid
Boost Software License 1.0
83 stars 67 forks source link

Align `uuid` to 16 bytes #139

Closed Lastique closed 2 months ago

Lastique commented 1 year ago

I propose adding BOOST_ALIGNMENT(16) attribute to the uuid type definition.

Aligning uuid type to 16 bytes can be beneficial for SIMD algorithms (e.g. uuid comparison operators), even on modern CPUs. Accessing vectors that are split across cache line or page boundaries can incur a significant performance penalty. Without aligning the uuid type this can happen in non-obvious and difficult to rectify cases. For example, in a flat_map<uuid, object*> with 64-bit pointers, every other element of such flat map will have a misaligned uuid key, and the lookup function will likely be affected by the aforementioned penalty.

Adding alignment to uuid will result in added padding in users' structs where previously UUIDs were unaligned. Where such increase in size is important, it is often easy to work around by reordering data members to place more data in the padding. However, this would be an ABI breaking change, so it should be mentioned in the release notes.

mclow commented 1 year ago

However, this would be an ABI breaking change, so it should be mentioned in the release notes

This should absolutely be called out.

jeking3 commented 1 year ago

Would people need a BOOST_UUID_NO_ALIGNMENT opt-out?

Lastique commented 1 year ago

On July 8, 2023 1:16:36 AM Jim King @.***> wrote:

Would people need a BOOST_UUID_NO_ALIGNMENT opt-out?

I think a macro that affects ABI like that would be quite dangerous, while its usefulness is questionable. I would not provide it.

pdimov commented 4 months ago

This sounds like something we would want, but

Adding alignment to uuid will result in added padding in users' structs where previously UUIDs were unaligned.

is an issue, as is having an extended alignment (which has been known to cause problems with e.g. std::vector.)

An interesting compromise would be having an alignment of 8, which won't help SSE2 loads, but will help uint64_t (and uint32_t) based access.

pdimov commented 4 months ago

Actually, the correct alignment wouldn't be a fixed 8, but alignas(std::uint64_t) (which would be 4 on 32 bit platforms).

Or maybe alignas(__uint128_t) on platforms having __uint128_t, although that's going to be the same in practice.

Lastique commented 4 months ago

The added benefit if 16-byte alignment, besides allowing aligned memory accesses, which is important for compilers making decisions on whether to generate vector instructions, is that an UUID will be guaranteed not to cross a page boundary. Even on CPUs with relatively fast unaligned loads and stores, memory accesses that cross page boundary are extra expensive.

I think, 8-byte alignment is not enough, and the benefits of 16-byte alignment outweigh the cons.

is an issue, as is having an extended alignment (which has been known to cause problems with e.g. std::vector.)

To my knowledge, memory allocators on 64-bit platforms (Windows, Linux, BSDs) align their allocations by 16 bytes minimum. So requiring 16-byte alignment doesn't count as "extended" alignment on those platforms.

Lastique commented 2 months ago

Per https://github.com/boostorg/uuid/pull/148#issuecomment-2209121169.