cmu-db / peloton

The Self-Driving Database Management System
http://pelotondb.io
Apache License 2.0
2.03k stars 624 forks source link

Array allocation of `TupleHeader` fails compilation on Ubuntu 18.04 #1433

Open pmenon opened 6 years ago

pmenon commented 6 years ago

I just tried to compile latest master on Ubuntu 18.04 GCC7, and it looks to be failing compilation when instantiating the array of TupleHeaders in the TileGroup here. The problem stems from the fact that TupleHeader is marked as 64-byte aligned (i.e., __attribute__((aligned(64)))) , but array allocations don't accept alignment parameters and, hence, don't guarantee alignment.

We can add the compiler flag -faligned-new to handle it - not sure how it plays with GCC5/6. We also don't support Ubuntu 18.04 officially, but wanted to track it when we do start.

mbutrovich commented 6 years ago

I suspect that the answer is no since it's probably the same underlying implementation, but does it work with struct alignas(64) TupleHeader on the type definition instead of __attribute__((aligned(64)))?

mbutrovich commented 6 years ago

Nope, doesn't seem like that'll change anything. In the end it might just be a premature optimization that would be safe to remove. Sorry for causing trouble on 18.04/GCC7.

I was trying to get each tuple's header to be cache line aligned, but perhaps it's too early to worry about that. Interestingly if I take out the attribute, I always end up with a 64-byte aligned base address for the array anyway, but that's probably just getting lucky.

pmenon commented 6 years ago

@mbutrovich You're just getting lucky, the standard makes no guarantees on alignment.

I understand the motivation, it's probably is okay to keep it. If you really need alignment, over-allocate and use std::align. We do this for parallel execution when allocating per-thread states.