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

idea for removing locks around thread stats #696

Closed
dormando opened this issue Jul 2, 2020 · 1 comment
Closed

idea for removing locks around thread stats #696

dormando opened this issue Jul 2, 2020 · 1 comment
Labels
idea work goal

Comments

@dormando
Copy link
Member

dormando commented Jul 2, 2020

Thread stats are locally locked; meaning the stats are only ever changed by the same thread that owns the lock. There is one lock per worker thread. (single writer, potentially many but realistically one reader)

The only other times the locks are held are for running the "stats" command, or I guess anything that internally gathers per-thread stats.

It might be tempting to simply replace them all with individual atomics, but that will likely cause a slowdown as the various atomic counters still need memory barriers (or worse; SMP-synchronization/etc). Atomics might be useful for some of the remaining stats which use the global STATS_LOCK(), since they are more likely to contend in the first place.

There're two paths I like, and I like the second one better:

  1. If the thread stats are all 8-byte aligned, writes/reads are atomic on at least x86 systems. Then you simply remove all the locks. This doesn't work on 32-bit systems and may not work on systems with looser memory architectures (IE; ARM).
  2. Run the stats call on a side thread and use the worker notify pipes to gather snapshots of the per-thread stats.

We have the concept of side threads (lru_crawler, log watcher), which with a little more work could have more generalized code. This could be used to create an admin/stats side thread. When a stats command is issued, the socket gets handed to the side thread. The side thread then writes a command to the notify pipelines of each worker thread; they then memcpy their stats and check in.

The side thread then tallies with what should be mostly existing code, and all of the thread stats locks are removed completely.

I like the idea so I'm documenting it, but throwing it down as low-priority. I haven't even done any testing to show if commenting out the thread locks improves IPC at all. I imagine it barely moves the needle. A bigger potential benefit for doing this would mean we could add more stats with less concern for impacting performance.

@dormando dormando changed the title idea for killing locks around thread stats idea for removing locks around thread stats Jul 2, 2020
@dormando
Copy link
Member Author

dormando commented Nov 1, 2023

meh

@dormando dormando closed this as not planned Won't fix, can't repro, duplicate, stale Nov 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
idea work goal
Projects
None yet
Development

No branches or pull requests

1 participant