> On July 20, 2018, 11:27 p.m., Benjamin Bannier wrote:
> > 3rdparty/libprocess/src/mpsc_linked_queue.hpp
> > Lines 184-188 (patched)
> > <https://reviews.apache.org/r/68001/diff/1/?file=2061817#file2061817line186>
> >
> > Wouldn't it be possible to avoid all the manual padding by aligning
> > both `head` and `tail` on their own cache lines?
> >
> > // We align `head` to 64 bytes (x86 cache line size) to guarantee
> > it to
> > // be put on a new cache line. This is to prevent false sharing with
> > // other objects that could otherwise end up on the same cache line
> > as
> > // this queue. We also align `tail` to avoid false sharing of `head`
> > // with `tail` and to avoid false sharing with other objects.
> >
> > // We assume a x86_64 cache line size of 64 bytes.
> > //
> > // TODO(drexin): Programatically get the cache line size.
> > #define CACHE_LINE 64
> >
> > alignas(CACHE_LINE) std::atomic<Node<T>*> head;
> > alignas(CACHE_LINE) Node<T>* tail;
> >
> > #undef CACHE_LINE
> >
> > Wouldn't this satisfy the guarantees you list in your comment?
> >
> > It would also allows us to avoid the macro which has a number of issues
> > (`__COUNTER__` is a GNU extension, missing check of `sizeof(var) < 64`).
I should have used a real variable for `CACHE_LINE` above to make this less
nasty, e.g.,
static constexpr std::size_t CACHE_LINE = 64; // Requires e.g., `#include
<cstddef>`.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68001/#review206289
-----------------------------------------------------------
On July 20, 2018, 6:51 p.m., Dario Rexin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68001/
> -----------------------------------------------------------
>
> (Updated July 20, 2018, 6:51 p.m.)
>
>
> Review request for Benjamin Bannier.
>
>
> 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/1/
>
>
> Testing
> -------
>
> make check & benchmarks
>
>
> Thanks,
>
> Dario Rexin
>
>
