Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

index must be align #21

Open
marakew opened this issue May 8, 2024 · 11 comments
Open

index must be align #21

marakew opened this issue May 8, 2024 · 11 comments

Comments

@marakew
Copy link

marakew commented May 8, 2024

uint32_t tail_{0};

atomic (rust and C++) by default has align
but uint32_t has no align
so false sharing will be decrease performance

@marakew
Copy link
Author

marakew commented May 8, 2024

use align as less from https://en.cppreference.com/w/cpp/atomic/atomic_ref

struct Counters { int a; int b; }; // user-defined trivially-copyable type
alignas(std::atomic_ref<Counters>::required_alignment) Counters counter;
std::atomic_ref<Counters> cnt(counter); // specialization for the user-defined type

@marakew
Copy link
Author

marakew commented May 8, 2024

but align by cacheline will be better
like from
https://github.com/tokio-rs/tokio/blob/master/tokio/src/util/cacheline.rs

and align for atomic too

@marakew
Copy link
Author

marakew commented May 8, 2024

boost has align for all fields
i dont known why rust doest align

https://www.boost.org/doc/libs/1_63_0/boost/fiber/detail/context_mpsc_queue.hpp

@marakew
Copy link
Author

marakew commented May 8, 2024

so strange
tokio improve mpsc align for tokio channel but not for internal queue
tokio-rs/tokio#5830

@8sileus
Copy link
Owner

8sileus commented May 8, 2024

Can I simply understand that to solve false-sharing?

@marakew
Copy link
Author

marakew commented May 8, 2024

original work streal spmc from go
https://github.com/golang/go/blob/4ed358b57efdad9ed710be7f4fc51495a7620ce2/src/runtime/runtime2.go#L668
does not use align
but i dont known how align work for go

but my opinion align must be present
article https://alic.dev/blog/false-sharing

@marakew
Copy link
Author

marakew commented May 8, 2024

so you can close issues

in my own implementation i will use cache align for indexs and buffer

@8sileus
Copy link
Owner

8sileus commented May 8, 2024

so you can close issues

in my own implementation i will use cache align for indexs and buffer

Yes
I'm not very good at this, but I think you're right
I am busy with graduation at the moment, and I will do it when I have time

@marakew
Copy link
Author

marakew commented May 8, 2024

iam look inside many implementation of spmc queue
some implementation has align, some not
may be it has sense, spmc queue in work steal case, most single produce/single consume queue
so multimple consumer rare consume
but its need make a tests for this

@marakew
Copy link
Author

marakew commented May 9, 2024

tail_ index can change to atomic
and all drect access to "relaxed" model
atomic_ref remove

this because golang has no memory model like C++ and has no "relaxed" memory model
tokio was not precise while porting from golang

8sileus added a commit that referenced this issue May 14, 2024
@8sileus
Copy link
Owner

8sileus commented May 14, 2024

tail_ index can change to atomic and all drect access to "relaxed" model atomic_ref remove

this because golang has no memory model like C++ and has no "relaxed" memory model tokio was not precise while porting from golang

temporary update
If there is a better plan in the future, I will update it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants