openraft icon indicating copy to clipboard operation
openraft copied to clipboard

Get rid of dependency of tokio if possible

Open drmingdrmer opened this issue 3 years ago • 4 comments

  • tokio::Runtime seems to be only used in tests.
  • tokio::time::Duration and other time-related types are just re-export.
  • tokio::sync::Mutex can be just replaced with std::sync::Mutex. They do not live across any await.

Get rid of some tokio dependency to make it clear for building a runtime simulator.

Originally posted by @drmingdrmer in https://github.com/datafuselabs/openraft/issues/205#issuecomment-1059000844

drmingdrmer avatar Mar 12 '22 07:03 drmingdrmer

👋 Thanks for opening this issue!

Get help or engage by:

  • /help : to print help messages.
  • /assignme : to assign this issue to you.

github-actions[bot] avatar Mar 12 '22 07:03 github-actions[bot]

tokio::sync::Mutex can be just replaced with std::sync::Mutex. They do not live across any await.

It's not just that the tokio mutex can live across an await, but also that it doesn't block the current thread. Using a std::sync::Mutex in an async environment could cause the runtime to exhausted all this thread.

Another possible benefit of using the tokio specific mutex, is that with the (currently unstable) tokio tracing feature, these mutex contain a tracing::Span.

A possible alternative would be futures::lock::Mutex as this mutex is IIRC runtime agnostic.

See tokio documentation for reference: https://docs.rs/tokio/1.19.2/tokio/sync/struct.Mutex.html

xanderio avatar Jun 22 '22 12:06 xanderio

I did mean not to use tokio::Mutex at all.

I mean somewhere tokio::Mutex is used but actually, a std::Mutex is quite enough.

BTW the span embedded in the tokio sync primitives is really sweet.

drmingdrmer avatar Jun 22 '22 13:06 drmingdrmer

Synchronous mutexes are sometimes in fact preferable (they anyway can't be locked over await). I didn't check where we do have mutexes in openraft, but the one for shutdown state, for example, is used very seldom (once when shutting down and once when querying the error), so that's a good fit.

Async mutexes (tokio or not) are typically quire heavy-weight, so depending on what they are protecting, you might end up with worse performance. Especially for uncontended mutexes, a synchronous mutex might be a good alternative. The best, do not use any mutex at all and just use message passing and similar (via queues, for instance).

BTW, there are also implicit synchronous mutexes in memory allocation (which is also one of the reasons to prevent memory allocation wherever possible, since they make it expensive - they are typically well-contended).

Other than that, @xanderio is in principle right - I also advocate against using synchronous primitives (coming from a team where we work on machines with hundreds of cores - that makes any contended synchronous mutex bring the system to a halt).

schreter avatar Jun 22 '22 14:06 schreter