Skip to content

Commit 05709fb

Browse files
committed
Remove InitOnce from the port API.
This is not an API-breaking change, because it reduces the API that the leveldb embedder must implement. The project will build just fine against ports that still implement InitOnce. C++11 guarantees thread-safe initialization of static variables inside functions. This is a more restricted form of std::call_once or pthread_once_t (e.g., single call site), so the compiler might be able to generate better code [1]. Equally important, having less code in port_example.h makes it easier to port to other platforms. Due to the change above, this CL introduces a new approach for storing the singleton BytewiseComparatorImpl instance returned by BytewiseComparator(). The new approach avoids a dynamic memory allocation, which eliminates the false positive from LeakSanitizer reported in google#200 [1] https://stackoverflow.com/a/27206650/ ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=212348004
1 parent bb88f25 commit 05709fb

File tree

6 files changed

+104
-29
lines changed

6 files changed

+104
-29
lines changed

CMakeLists.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,7 @@ target_sources(leveldb
149149
"${PROJECT_SOURCE_DIR}/util/logging.cc"
150150
"${PROJECT_SOURCE_DIR}/util/logging.h"
151151
"${PROJECT_SOURCE_DIR}/util/mutexlock.h"
152+
"${PROJECT_SOURCE_DIR}/util/no_destructor.h"
152153
"${PROJECT_SOURCE_DIR}/util/options.cc"
153154
"${PROJECT_SOURCE_DIR}/util/random.h"
154155
"${PROJECT_SOURCE_DIR}/util/status.cc"
@@ -278,6 +279,7 @@ if(LEVELDB_BUILD_TESTS)
278279

279280
leveldb_test("${PROJECT_SOURCE_DIR}/util/env_test.cc")
280281
leveldb_test("${PROJECT_SOURCE_DIR}/util/status_test.cc")
282+
leveldb_test("${PROJECT_SOURCE_DIR}/util/no_destructor_test.cc")
281283

282284
if(NOT BUILD_SHARED_LIBS)
283285
leveldb_test("${PROJECT_SOURCE_DIR}/db/autocompact_test.cc")

port/port_example.h

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -62,16 +62,6 @@ class CondVar {
6262
void SignallAll();
6363
};
6464

65-
// Thread-safe initialization.
66-
// Used as follows:
67-
// static port::OnceType init_control = LEVELDB_ONCE_INIT;
68-
// static void Initializer() { ... do something ...; }
69-
// ...
70-
// port::InitOnce(&init_control, &Initializer);
71-
typedef intptr_t OnceType;
72-
#define LEVELDB_ONCE_INIT 0
73-
void InitOnce(port::OnceType*, void (*initializer)());
74-
7565
// A type that holds a pointer that can be read or written atomically
7666
// (i.e., without word-tearing.)
7767
class AtomicPointer {

port/port_stdcxx.h

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -84,14 +84,6 @@ class CondVar {
8484
Mutex* const mu_;
8585
};
8686

87-
using OnceType = std::once_flag;
88-
#define LEVELDB_ONCE_INIT {}
89-
90-
// Thinly wraps std::call_once.
91-
inline void InitOnce(OnceType* once, void (*initializer)()) {
92-
std::call_once(*once, *initializer);
93-
}
94-
9587
inline bool Snappy_Compress(const char* input, size_t length,
9688
::std::string* output) {
9789
#if HAVE_SNAPPY

util/comparator.cc

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,13 @@
33
// found in the LICENSE file. See the AUTHORS file for names of contributors.
44

55
#include <algorithm>
6-
#include <stdint.h>
6+
#include <cstdint>
7+
#include <string>
8+
79
#include "leveldb/comparator.h"
810
#include "leveldb/slice.h"
9-
#include "port/port.h"
1011
#include "util/logging.h"
12+
#include "util/no_destructor.h"
1113

1214
namespace leveldb {
1315

@@ -66,16 +68,9 @@ class BytewiseComparatorImpl : public Comparator {
6668
};
6769
} // namespace
6870

69-
static port::OnceType once = LEVELDB_ONCE_INIT;
70-
static const Comparator* bytewise;
71-
72-
static void InitModule() {
73-
bytewise = new BytewiseComparatorImpl;
74-
}
75-
7671
const Comparator* BytewiseComparator() {
77-
port::InitOnce(&once, InitModule);
78-
return bytewise;
72+
static NoDestructor<BytewiseComparatorImpl> singleton;
73+
return singleton.get();
7974
}
8075

8176
} // namespace leveldb

util/no_destructor.h

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
// Copyright (c) 2018 The LevelDB Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file. See the AUTHORS file for names of contributors.
4+
5+
#ifndef STORAGE_LEVELDB_UTIL_NO_DESTRUCTOR_H_
6+
#define STORAGE_LEVELDB_UTIL_NO_DESTRUCTOR_H_
7+
8+
#include <type_traits>
9+
#include <utility>
10+
11+
namespace leveldb {
12+
13+
// Wraps an instance whose destructor is never called.
14+
//
15+
// This is intended for use with function-level static variables.
16+
template<typename InstanceType>
17+
class NoDestructor {
18+
public:
19+
template <typename... ConstructorArgTypes>
20+
explicit NoDestructor(ConstructorArgTypes&&... constructor_args) {
21+
static_assert(sizeof(instance_storage_) >= sizeof(InstanceType),
22+
"instance_storage_ is not large enough to hold the instance");
23+
static_assert(
24+
alignof(decltype(instance_storage_)) >= alignof(InstanceType),
25+
"instance_storage_ does not meet the instance's alignment requirement");
26+
new (&instance_storage_) InstanceType(
27+
std::forward<ConstructorArgTypes>(constructor_args)...);
28+
}
29+
30+
~NoDestructor() = default;
31+
32+
NoDestructor(const NoDestructor&) = delete;
33+
NoDestructor& operator=(const NoDestructor&) = delete;
34+
35+
InstanceType* get() {
36+
return reinterpret_cast<InstanceType*>(&instance_storage_);
37+
}
38+
39+
private:
40+
typename
41+
std::aligned_storage<sizeof(InstanceType), alignof(InstanceType)>::type
42+
instance_storage_;
43+
};
44+
45+
} // namespace leveldb
46+
47+
#endif // STORAGE_LEVELDB_UTIL_NO_DESTRUCTOR_H_

util/no_destructor_test.cc

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
// Copyright (c) 2018 The LevelDB Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file. See the AUTHORS file for names of contributors.
4+
5+
#include <cstdint>
6+
#include <cstdlib>
7+
#include <utility>
8+
9+
#include "util/no_destructor.h"
10+
#include "util/testharness.h"
11+
12+
namespace leveldb {
13+
14+
namespace {
15+
16+
struct DoNotDestruct {
17+
public:
18+
DoNotDestruct(uint32_t a, uint64_t b) : a(a), b(b) {}
19+
~DoNotDestruct() { std::abort(); }
20+
21+
// Used to check constructor argument forwarding.
22+
uint32_t a;
23+
uint64_t b;
24+
};
25+
26+
constexpr const uint32_t kGoldenA = 0xdeadbeef;
27+
constexpr const uint64_t kGoldenB = 0xaabbccddeeffaabb;
28+
29+
} // namespace
30+
31+
class NoDestructorTest { };
32+
33+
TEST(NoDestructorTest, StackInstance) {
34+
NoDestructor<DoNotDestruct> instance(kGoldenA, kGoldenB);
35+
ASSERT_EQ(kGoldenA, instance.get()->a);
36+
ASSERT_EQ(kGoldenB, instance.get()->b);
37+
}
38+
39+
TEST(NoDestructorTest, StaticInstance) {
40+
static NoDestructor<DoNotDestruct> instance(kGoldenA, kGoldenB);
41+
ASSERT_EQ(kGoldenA, instance.get()->a);
42+
ASSERT_EQ(kGoldenB, instance.get()->b);
43+
}
44+
45+
} // namespace leveldb
46+
47+
int main(int argc, char** argv) {
48+
return leveldb::test::RunAllTests();
49+
}

0 commit comments

Comments
 (0)