Commit 580d5f19 authored by Nitin Garg's avatar Nitin Garg Committed by Facebook GitHub Bot

Replace spinlock with SharedMutex in DigestBuilder

Summary:
Spinlocks in user space are generally a bad idea due to tendency to
burn CPU in outlier events. Replace it with SharedMutex instead. The common
case cost could go up a bit should be better overall.

Reviewed By: yfeldblum

Differential Revision: D31654923

fbshipit-source-id: 77d249293458cd740904f777e00be1bd6834b62f
parent 5fadaded
...@@ -37,7 +37,7 @@ DigestT DigestBuilder<DigestT>::build() { ...@@ -37,7 +37,7 @@ DigestT DigestBuilder<DigestT>::build() {
digestPtrs.reserve(cpuLocalBuffers_.size()); digestPtrs.reserve(cpuLocalBuffers_.size());
for (auto& cpuLocalBuffer : cpuLocalBuffers_) { for (auto& cpuLocalBuffer : cpuLocalBuffers_) {
std::unique_lock<SpinLock> g(cpuLocalBuffer.mutex); std::unique_lock<SharedMutex> g(cpuLocalBuffer.mutex);
valuesVec.push_back(std::move(cpuLocalBuffer.buffer)); valuesVec.push_back(std::move(cpuLocalBuffer.buffer));
if (cpuLocalBuffer.digest) { if (cpuLocalBuffer.digest) {
digestPtrs.push_back(std::move(cpuLocalBuffer.digest)); digestPtrs.push_back(std::move(cpuLocalBuffer.digest));
...@@ -70,7 +70,7 @@ template <typename DigestT> ...@@ -70,7 +70,7 @@ template <typename DigestT>
void DigestBuilder<DigestT>::append(double value) { void DigestBuilder<DigestT>::append(double value) {
auto cpuLocalBuf = &cpuLocalBuffers_[AccessSpreader<>::cachedCurrent( auto cpuLocalBuf = &cpuLocalBuffers_[AccessSpreader<>::cachedCurrent(
cpuLocalBuffers_.size())]; cpuLocalBuffers_.size())];
std::unique_lock<SpinLock> g(cpuLocalBuf->mutex); std::unique_lock<SharedMutex> g(cpuLocalBuf->mutex);
cpuLocalBuf->buffer.push_back(value); cpuLocalBuf->buffer.push_back(value);
if (cpuLocalBuf->buffer.size() == bufferSize_) { if (cpuLocalBuf->buffer.size() == bufferSize_) {
if (!cpuLocalBuf->digest) { if (!cpuLocalBuf->digest) {
......
...@@ -19,7 +19,7 @@ ...@@ -19,7 +19,7 @@
#include <memory> #include <memory>
#include <folly/Memory.h> #include <folly/Memory.h>
#include <folly/SpinLock.h> #include <folly/SharedMutex.h>
namespace folly { namespace folly {
...@@ -55,9 +55,22 @@ class DigestBuilder { ...@@ -55,9 +55,22 @@ class DigestBuilder {
private: private:
struct alignas(hardware_destructive_interference_size) CpuLocalBuffer { struct alignas(hardware_destructive_interference_size) CpuLocalBuffer {
public: public:
mutable SpinLock mutex; mutable SharedMutex mutex;
std::vector<double> buffer; std::vector<double> buffer;
std::unique_ptr<DigestT> digest; std::unique_ptr<DigestT> digest;
CpuLocalBuffer() noexcept = default;
CpuLocalBuffer(CpuLocalBuffer&& other) noexcept
: buffer{std::move(other.buffer)}, digest{std::move(other.digest)} {}
CpuLocalBuffer& operator=(CpuLocalBuffer&& other) noexcept {
if (this != &other) {
buffer = std::move(other.buffer);
digest = std::move(other.digest);
}
return *this;
}
}; };
// cpulocalbuffer_alloc custom allocator is necessary until C++17 // cpulocalbuffer_alloc custom allocator is necessary until C++17
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment