Skip to content

Commit 7d8e41e

Browse files
pwnallcmumford
authored andcommitted
leveldb: Replace AtomicPointer with std::atomic.
This CL removes AtomicPointer from leveldb's port interface. Its usage is replaced with std::atomic<> from the C++11 standard library. AtomicPointer was used to wrap flags, numbers, and pointers, so its instances are replaced with std::atomic<bool>, std::atomic<int>, std::atomic<size_t> and std::atomic<Node*>. This CL does not revise the memory ordering. AtomicPointer's methods are replaced mechanically with their std::atomic equivalents, even when the underlying usage is incorrect. (Example: DBImpl::has_imm_ is written using release stores, even though it is always read using relaxed ordering.) Revising the memory ordering is left for future CLs. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=237865146
1 parent dd90626 commit 7d8e41e

File tree

13 files changed

+210
-419
lines changed

13 files changed

+210
-419
lines changed

db/db_bench.cc

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@
3434
// seekrandom -- N random seeks
3535
// open -- cost of opening a DB
3636
// crc32c -- repeated crc32c of 4K of data
37-
// acquireload -- load N*1000 times
3837
// Meta operations:
3938
// compact -- Compact the entire DB
4039
// stats -- Print DB stats
@@ -57,7 +56,6 @@ static const char* FLAGS_benchmarks =
5756
"crc32c,"
5857
"snappycomp,"
5958
"snappyuncomp,"
60-
"acquireload,"
6159
;
6260

6361
// Number of key/values to place in database
@@ -510,8 +508,6 @@ class Benchmark {
510508
method = &Benchmark::Compact;
511509
} else if (name == Slice("crc32c")) {
512510
method = &Benchmark::Crc32c;
513-
} else if (name == Slice("acquireload")) {
514-
method = &Benchmark::AcquireLoad;
515511
} else if (name == Slice("snappycomp")) {
516512
method = &Benchmark::SnappyCompress;
517513
} else if (name == Slice("snappyuncomp")) {
@@ -639,22 +635,6 @@ class Benchmark {
639635
thread->stats.AddMessage(label);
640636
}
641637

642-
void AcquireLoad(ThreadState* thread) {
643-
int dummy;
644-
port::AtomicPointer ap(&dummy);
645-
int count = 0;
646-
void *ptr = nullptr;
647-
thread->stats.AddMessage("(each op is 1000 loads)");
648-
while (count < 100000) {
649-
for (int i = 0; i < 1000; i++) {
650-
ptr = ap.Acquire_Load();
651-
}
652-
count++;
653-
thread->stats.FinishedSingleOp();
654-
}
655-
if (ptr == nullptr) exit(1); // Disable unused variable warning.
656-
}
657-
658638
void SnappyCompress(ThreadState* thread) {
659639
RandomGenerator gen;
660640
Slice input = gen.Generate(Options().block_size);

db/db_impl.cc

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#include <stdio.h>
99

1010
#include <algorithm>
11+
#include <atomic>
1112
#include <set>
1213
#include <string>
1314
#include <vector>
@@ -132,10 +133,11 @@ DBImpl::DBImpl(const Options& raw_options, const std::string& dbname)
132133
dbname_(dbname),
133134
table_cache_(new TableCache(dbname_, options_, TableCacheSize(options_))),
134135
db_lock_(nullptr),
135-
shutting_down_(nullptr),
136+
shutting_down_(false),
136137
background_work_finished_signal_(&mutex_),
137138
mem_(nullptr),
138139
imm_(nullptr),
140+
has_imm_(false),
139141
logfile_(nullptr),
140142
logfile_number_(0),
141143
log_(nullptr),
@@ -144,14 +146,12 @@ DBImpl::DBImpl(const Options& raw_options, const std::string& dbname)
144146
background_compaction_scheduled_(false),
145147
manual_compaction_(nullptr),
146148
versions_(new VersionSet(dbname_, &options_, table_cache_,
147-
&internal_comparator_)) {
148-
has_imm_.Release_Store(nullptr);
149-
}
149+
&internal_comparator_)) {}
150150

151151
DBImpl::~DBImpl() {
152-
// Wait for background work to finish
152+
// Wait for background work to finish.
153153
mutex_.Lock();
154-
shutting_down_.Release_Store(this); // Any non-null value is ok
154+
shutting_down_.store(true, std::memory_order_release);
155155
while (background_compaction_scheduled_) {
156156
background_work_finished_signal_.Wait();
157157
}
@@ -547,7 +547,7 @@ void DBImpl::CompactMemTable() {
547547
Status s = WriteLevel0Table(imm_, &edit, base);
548548
base->Unref();
549549

550-
if (s.ok() && shutting_down_.Acquire_Load()) {
550+
if (s.ok() && shutting_down_.load(std::memory_order_acquire)) {
551551
s = Status::IOError("Deleting DB during memtable compaction");
552552
}
553553

@@ -562,7 +562,7 @@ void DBImpl::CompactMemTable() {
562562
// Commit to the new state
563563
imm_->Unref();
564564
imm_ = nullptr;
565-
has_imm_.Release_Store(nullptr);
565+
has_imm_.store(false, std::memory_order_release);
566566
DeleteObsoleteFiles();
567567
} else {
568568
RecordBackgroundError(s);
@@ -610,7 +610,8 @@ void DBImpl::TEST_CompactRange(int level, const Slice* begin,
610610
}
611611

612612
MutexLock l(&mutex_);
613-
while (!manual.done && !shutting_down_.Acquire_Load() && bg_error_.ok()) {
613+
while (!manual.done && !shutting_down_.load(std::memory_order_acquire) &&
614+
bg_error_.ok()) {
614615
if (manual_compaction_ == nullptr) { // Idle
615616
manual_compaction_ = &manual;
616617
MaybeScheduleCompaction();
@@ -652,7 +653,7 @@ void DBImpl::MaybeScheduleCompaction() {
652653
mutex_.AssertHeld();
653654
if (background_compaction_scheduled_) {
654655
// Already scheduled
655-
} else if (shutting_down_.Acquire_Load()) {
656+
} else if (shutting_down_.load(std::memory_order_acquire)) {
656657
// DB is being deleted; no more background compactions
657658
} else if (!bg_error_.ok()) {
658659
// Already got an error; no more changes
@@ -673,7 +674,7 @@ void DBImpl::BGWork(void* db) {
673674
void DBImpl::BackgroundCall() {
674675
MutexLock l(&mutex_);
675676
assert(background_compaction_scheduled_);
676-
if (shutting_down_.Acquire_Load()) {
677+
if (shutting_down_.load(std::memory_order_acquire)) {
677678
// No more background work when shutting down.
678679
} else if (!bg_error_.ok()) {
679680
// No more background work after a background error.
@@ -752,7 +753,7 @@ void DBImpl::BackgroundCompaction() {
752753

753754
if (status.ok()) {
754755
// Done
755-
} else if (shutting_down_.Acquire_Load()) {
756+
} else if (shutting_down_.load(std::memory_order_acquire)) {
756757
// Ignore compaction errors found during shutting down
757758
} else {
758759
Log(options_.info_log,
@@ -919,9 +920,9 @@ Status DBImpl::DoCompactionWork(CompactionState* compact) {
919920
std::string current_user_key;
920921
bool has_current_user_key = false;
921922
SequenceNumber last_sequence_for_key = kMaxSequenceNumber;
922-
for (; input->Valid() && !shutting_down_.Acquire_Load(); ) {
923+
for (; input->Valid() && !shutting_down_.load(std::memory_order_acquire); ) {
923924
// Prioritize immutable compaction work
924-
if (has_imm_.NoBarrier_Load() != nullptr) {
925+
if (has_imm_.load(std::memory_order_relaxed)) {
925926
const uint64_t imm_start = env_->NowMicros();
926927
mutex_.Lock();
927928
if (imm_ != nullptr) {
@@ -1014,7 +1015,7 @@ Status DBImpl::DoCompactionWork(CompactionState* compact) {
10141015
input->Next();
10151016
}
10161017

1017-
if (status.ok() && shutting_down_.Acquire_Load()) {
1018+
if (status.ok() && shutting_down_.load(std::memory_order_acquire)) {
10181019
status = Status::IOError("Deleting DB during compaction");
10191020
}
10201021
if (status.ok() && compact->builder != nullptr) {
@@ -1378,7 +1379,7 @@ Status DBImpl::MakeRoomForWrite(bool force) {
13781379
logfile_number_ = new_log_number;
13791380
log_ = new log::Writer(lfile);
13801381
imm_ = mem_;
1381-
has_imm_.Release_Store(imm_);
1382+
has_imm_.store(true, std::memory_order_release);
13821383
mem_ = new MemTable(internal_comparator_);
13831384
mem_->Ref();
13841385
force = false; // Do not force another compaction if have room

db/db_impl.h

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,11 @@
55
#ifndef STORAGE_LEVELDB_DB_DB_IMPL_H_
66
#define STORAGE_LEVELDB_DB_DB_IMPL_H_
77

8+
#include <atomic>
89
#include <deque>
910
#include <set>
11+
#include <string>
12+
1013
#include "db/dbformat.h"
1114
#include "db/log_writer.h"
1215
#include "db/snapshot.h"
@@ -136,11 +139,11 @@ class DBImpl : public DB {
136139

137140
// State below is protected by mutex_
138141
port::Mutex mutex_;
139-
port::AtomicPointer shutting_down_;
142+
std::atomic<bool> shutting_down_;
140143
port::CondVar background_work_finished_signal_ GUARDED_BY(mutex_);
141144
MemTable* mem_;
142145
MemTable* imm_ GUARDED_BY(mutex_); // Memtable being compacted
143-
port::AtomicPointer has_imm_; // So bg thread can detect non-null imm_
146+
std::atomic<bool> has_imm_; // So bg thread can detect non-null imm_
144147
WritableFile* logfile_;
145148
uint64_t logfile_number_ GUARDED_BY(mutex_);
146149
log::Writer* log_;

0 commit comments

Comments
 (0)