mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benjamin Bannier <benjamin.bann...@mesosphere.io>
Subject Re: Review Request 68001: Fix padding in MpscLinkedQueue.
Date Tue, 24 Jul 2018 17:40:07 GMT


> 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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message