> On July 24, 2018, 5: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)] > > ``` > > Benjamin Bannier wrote: > 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.
If we don't add the padding after the head, I expect we will still get the clang-tidy warning: ``` consider reordering the fields or adding explicit padding members [clang-analyzer-optin.performance.Padding] ``` For consistency, I suggested to @dario that we apply both the alignas and the padding for both variables. - James ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68001/#review206395 ----------------------------------------------------------- On July 24, 2018, 2: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, 2: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 > >
