> On July 24, 2018, 7:05 p.m., James Peach wrote: > > 3rdparty/libprocess/src/mpsc_linked_queue.hpp > > Lines 27 (patched) > > <https://reviews.apache.org/r/68001/diff/2/?file=2063433#file2063433line27> > > > > Chatted with @bbannier, and he pointed out that we can replace > > `__COUNTER__` with something like this: > > > > ``` > > #define MPSC_CACHELINE_PAD(varname) \ > > uint8_t CAT(_pad, __, varname)[64 - (sizeof(var) % 64)] > > ```
I am not sure we still need the macro since the `alignas` will enforce the memory layout we want for `head` and `tail`, and we only really need to add it once to add additional padding after `tail`. I'd either suggest getting rid of the macro and just manually adding a padding variable after `tail` (its only required use site), or alternatively move its definition down into the class and `#undef`'ing it after last use. To me the former looks simpler, cleaner and easier to understand. - Benjamin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68001/#review206395 ----------------------------------------------------------- On July 24, 2018, 4:59 p.m., Dario Rexin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/68001/ > ----------------------------------------------------------- > > (Updated July 24, 2018, 4:59 p.m.) > > > Review request for Benjamin Bannier and James Peach. > > > Repository: mesos > > > Description > ------- > > This patch aligns the head of MpscLinkedQueue to a new cache line > and adds padding between head and tail to avoid false sharing > between to two and after tail to avoid false sharing with other > objects that could otherwise end up on the same cache line. > > > Diffs > ----- > > 3rdparty/libprocess/src/mpsc_linked_queue.hpp 48c95093d > > > Diff: https://reviews.apache.org/r/68001/diff/2/ > > > Testing > ------- > > make check & benchmarks > > > Thanks, > > Dario Rexin > >
