From 03064cbbb2c00c3e6e41a78e8111d14a020f7d6f Mon Sep 17 00:00:00 2001 From: costan Date: Tue, 4 Sep 2018 09:31:27 -0700 Subject: [PATCH] Simplify Limiter in env_posix.cc. Now that we require C++11, we can use std::atomic, which has primitives for most of the logic we need. As a bonus, the happy path for Limiter::Acquire() and Limiter::Release() only performs one atomic operation. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=211469518 --- util/env_posix.cc | 58 ++++++++++++++++++++++++------------------------------- 1 file changed, 25 insertions(+), 33 deletions(-) diff --git a/util/env_posix.cc b/util/env_posix.cc index 51844ad..18e7664 100644 --- a/util/env_posix.cc +++ b/util/env_posix.cc @@ -16,9 +16,12 @@ #include #include #include + +#include #include #include #include + #include "leveldb/env.h" #include "leveldb/slice.h" #include "port/port.h" @@ -53,52 +56,41 @@ static Status PosixError(const std::string& context, int err_number) { // Helper class to limit resource usage to avoid exhaustion. // Currently used to limit read-only file descriptors and mmap file usage -// so that we do not end up running out of file descriptors, virtual memory, -// or running into kernel performance problems for very large databases. +// so that we do not run out of file descriptors or virtual memory, or run into +// kernel performance problems for very large databases. class Limiter { public: - // Limit maximum number of resources to |n|. - Limiter(intptr_t n) { - SetAllowed(n); - } + // Limit maximum number of resources to |max_acquires|. + Limiter(int max_acquires) : acquires_allowed_(max_acquires) {} + + Limiter(const Limiter&) = delete; + Limiter operator=(const Limiter&) = delete; // If another resource is available, acquire it and return true. // Else return false. - bool Acquire() LOCKS_EXCLUDED(mu_) { - if (GetAllowed() <= 0) { - return false; - } - MutexLock l(&mu_); - intptr_t x = GetAllowed(); - if (x <= 0) { - return false; - } else { - SetAllowed(x - 1); + bool Acquire() { + int old_acquires_allowed = + acquires_allowed_.fetch_sub(1, std::memory_order_relaxed); + + if (old_acquires_allowed > 0) return true; - } + + acquires_allowed_.fetch_add(1, std::memory_order_relaxed); + return false; } // Release a resource acquired by a previous call to Acquire() that returned // true. - void Release() LOCKS_EXCLUDED(mu_) { - MutexLock l(&mu_); - SetAllowed(GetAllowed() + 1); + void Release() { + acquires_allowed_.fetch_add(1, std::memory_order_relaxed); } private: - port::Mutex mu_; - port::AtomicPointer allowed_; - - intptr_t GetAllowed() const { - return reinterpret_cast(allowed_.Acquire_Load()); - } - - void SetAllowed(intptr_t v) EXCLUSIVE_LOCKS_REQUIRED(mu_) { - allowed_.Release_Store(reinterpret_cast(v)); - } - - Limiter(const Limiter&); - void operator=(const Limiter&); + // The number of available resources. + // + // This is a counter and is not tied to the invariants of any other class, so + // it can be operated on safely using std::memory_order_relaxed. + std::atomic acquires_allowed_; }; class PosixSequentialFile: public SequentialFile {