Skip to content

Commit

Permalink
using ThreadLocalPtr to hide ROCKSDB_SUPPORT_THREAD_LOCAL from public…
Browse files Browse the repository at this point in the history 
Summary:
… headers

facebook#2199 should not reference RocksDB-specific macros (like ROCKSDB_SUPPORT_THREAD_LOCAL in this case) to public headers, `iostats_context.h` and `perf_context.h`. We shouldn't do that because users have to provide these compiler flags when building their binary with RocksDB.

We should hide the thread local global variable inside our implementation and just expose a function api to retrieve these variables. It may break some users for now but good for long term.

make check -j64
Closes facebook#2380

Differential Revision: D5177896

Pulled By: lightmark

fbshipit-source-id: 6fcdfac57f2e2dcfe60992b7385c5403f6dcb390
  • Loading branch information
@lightmark @facebook-github-bot
lightmark authored and facebook-github-bot committed Jun 3, 2017
1 parent 138b87e commit 7f6c02d
Show file tree
Hide file tree
Showing 20 changed files with 257 additions and 263 deletions.
1 change: 1 addition & 0 deletions HISTORY.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
* Scheduling flushes and compactions in the same thread pool is no longer supported by setting `max_background_flushes=0`. Instead, users can achieve this by configuring their high-pri thread pool to have zero threads.
* Replace `Options::max_background_flushes`, `Options::max_background_compactions`, and `Options::base_background_compactions` all with `Options::max_background_jobs`, which automatically decides how many threads to allocate towards flush/compaction.
* options.delayed_write_rate by default take the value of options.rate_limiter rate.
* Replace global variable `IOStatsContext iostats_context` with `IOStatsContext* get_iostats_context()`; replace global variable `PerfContext perf_context` with `PerfContext* get_perf_context()`.

### New Features
* Change ticker/histogram statistics implementations to use core-local storage. This improves aggregation speed compared to our previous thread-local approach, particularly for applications with many threads.
Expand Down
8 changes: 4 additions & 4 deletions db/db_basic_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -366,9 +366,9 @@ TEST_F(DBBasicTest, FLUSH) {
ASSERT_OK(Flush(1));
ASSERT_OK(dbfull()->Put(writeOpt, handles_[1], "bar", "v1"));

perf_context.Reset();
get_perf_context()->Reset();
Get(1, "foo");
ASSERT_TRUE((int)perf_context.get_from_output_files_time > 0);
ASSERT_TRUE((int)get_perf_context()->get_from_output_files_time > 0);

ReopenWithColumnFamilies({"default", "pikachu"}, CurrentOptions());
ASSERT_EQ("v1", Get(1, "foo"));
Expand All @@ -381,9 +381,9 @@ TEST_F(DBBasicTest, FLUSH) {

ReopenWithColumnFamilies({"default", "pikachu"}, CurrentOptions());
ASSERT_EQ("v2", Get(1, "bar"));
perf_context.Reset();
get_perf_context()->Reset();
ASSERT_EQ("v2", Get(1, "foo"));
ASSERT_TRUE((int)perf_context.get_from_output_files_time > 0);
ASSERT_TRUE((int)get_perf_context()->get_from_output_files_time > 0);

writeOpt.disableWAL = false;
ASSERT_OK(dbfull()->Put(writeOpt, handles_[1], "bar", "v3"));
Expand Down
48 changes: 24 additions & 24 deletions db/db_bloom_filter_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -688,12 +688,12 @@ class BloomStatsTestWithParam
}
options_.env = env_;

perf_context.Reset();
get_perf_context()->Reset();
DestroyAndReopen(options_);
}

~BloomStatsTestWithParam() {
perf_context.Reset();
get_perf_context()->Reset();
Destroy(options_);
}

Expand Down Expand Up @@ -726,33 +726,33 @@ TEST_P(BloomStatsTestWithParam, BloomStatsTest) {

// check memtable bloom stats
ASSERT_EQ(value1, Get(key1));
ASSERT_EQ(1, perf_context.bloom_memtable_hit_count);
ASSERT_EQ(1, get_perf_context()->bloom_memtable_hit_count);
ASSERT_EQ(value3, Get(key3));
ASSERT_EQ(2, perf_context.bloom_memtable_hit_count);
ASSERT_EQ(0, perf_context.bloom_memtable_miss_count);
ASSERT_EQ(2, get_perf_context()->bloom_memtable_hit_count);
ASSERT_EQ(0, get_perf_context()->bloom_memtable_miss_count);

ASSERT_EQ("NOT_FOUND", Get(key2));
ASSERT_EQ(1, perf_context.bloom_memtable_miss_count);
ASSERT_EQ(2, perf_context.bloom_memtable_hit_count);
ASSERT_EQ(1, get_perf_context()->bloom_memtable_miss_count);
ASSERT_EQ(2, get_perf_context()->bloom_memtable_hit_count);

// sanity checks
ASSERT_EQ(0, perf_context.bloom_sst_hit_count);
ASSERT_EQ(0, perf_context.bloom_sst_miss_count);
ASSERT_EQ(0, get_perf_context()->bloom_sst_hit_count);
ASSERT_EQ(0, get_perf_context()->bloom_sst_miss_count);

Flush();

// sanity checks
ASSERT_EQ(0, perf_context.bloom_sst_hit_count);
ASSERT_EQ(0, perf_context.bloom_sst_miss_count);
ASSERT_EQ(0, get_perf_context()->bloom_sst_hit_count);
ASSERT_EQ(0, get_perf_context()->bloom_sst_miss_count);

// check SST bloom stats
ASSERT_EQ(value1, Get(key1));
ASSERT_EQ(1, perf_context.bloom_sst_hit_count);
ASSERT_EQ(1, get_perf_context()->bloom_sst_hit_count);
ASSERT_EQ(value3, Get(key3));
ASSERT_EQ(2, perf_context.bloom_sst_hit_count);
ASSERT_EQ(2, get_perf_context()->bloom_sst_hit_count);

ASSERT_EQ("NOT_FOUND", Get(key2));
ASSERT_EQ(1, perf_context.bloom_sst_miss_count);
ASSERT_EQ(1, get_perf_context()->bloom_sst_miss_count);
}

// Same scenario as in BloomStatsTest but using an iterator
Expand All @@ -773,21 +773,21 @@ TEST_P(BloomStatsTestWithParam, BloomStatsTestWithIter) {
ASSERT_OK(iter->status());
ASSERT_TRUE(iter->Valid());
ASSERT_EQ(value1, iter->value().ToString());
ASSERT_EQ(1, perf_context.bloom_memtable_hit_count);
ASSERT_EQ(0, perf_context.bloom_memtable_miss_count);
ASSERT_EQ(1, get_perf_context()->bloom_memtable_hit_count);
ASSERT_EQ(0, get_perf_context()->bloom_memtable_miss_count);

iter->Seek(key3);
ASSERT_OK(iter->status());
ASSERT_TRUE(iter->Valid());
ASSERT_EQ(value3, iter->value().ToString());
ASSERT_EQ(2, perf_context.bloom_memtable_hit_count);
ASSERT_EQ(0, perf_context.bloom_memtable_miss_count);
ASSERT_EQ(2, get_perf_context()->bloom_memtable_hit_count);
ASSERT_EQ(0, get_perf_context()->bloom_memtable_miss_count);

iter->Seek(key2);
ASSERT_OK(iter->status());
ASSERT_TRUE(!iter->Valid());
ASSERT_EQ(1, perf_context.bloom_memtable_miss_count);
ASSERT_EQ(2, perf_context.bloom_memtable_hit_count);
ASSERT_EQ(1, get_perf_context()->bloom_memtable_miss_count);
ASSERT_EQ(2, get_perf_context()->bloom_memtable_hit_count);

Flush();

Expand All @@ -798,19 +798,19 @@ TEST_P(BloomStatsTestWithParam, BloomStatsTestWithIter) {
ASSERT_OK(iter->status());
ASSERT_TRUE(iter->Valid());
ASSERT_EQ(value1, iter->value().ToString());
ASSERT_EQ(1, perf_context.bloom_sst_hit_count);
ASSERT_EQ(1, get_perf_context()->bloom_sst_hit_count);

iter->Seek(key3);
ASSERT_OK(iter->status());
ASSERT_TRUE(iter->Valid());
ASSERT_EQ(value3, iter->value().ToString());
ASSERT_EQ(2, perf_context.bloom_sst_hit_count);
ASSERT_EQ(2, get_perf_context()->bloom_sst_hit_count);

iter->Seek(key2);
ASSERT_OK(iter->status());
ASSERT_TRUE(!iter->Valid());
ASSERT_EQ(1, perf_context.bloom_sst_miss_count);
ASSERT_EQ(2, perf_context.bloom_sst_hit_count);
ASSERT_EQ(1, get_perf_context()->bloom_sst_miss_count);
ASSERT_EQ(2, get_perf_context()->bloom_sst_hit_count);
}

INSTANTIATE_TEST_CASE_P(BloomStatsTestWithParam, BloomStatsTestWithParam,
Expand Down
8 changes: 4 additions & 4 deletions db/db_iter_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -354,11 +354,11 @@ TEST_F(DBIteratorTest, DBIteratorPrevNext) {
SetPerfLevel(kEnableCount);
ASSERT_TRUE(GetPerfLevel() == kEnableCount);

perf_context.Reset();
get_perf_context()->Reset();
db_iter->SeekToLast();

ASSERT_TRUE(db_iter->Valid());
ASSERT_EQ(static_cast<int>(perf_context.internal_key_skipped_count), 7);
ASSERT_EQ(static_cast<int>(get_perf_context()->internal_key_skipped_count), 7);
ASSERT_EQ(db_iter->key().ToString(), "b");

SetPerfLevel(kDisable);
Expand Down Expand Up @@ -473,11 +473,11 @@ TEST_F(DBIteratorTest, DBIteratorPrevNext) {
SetPerfLevel(kEnableCount);
ASSERT_TRUE(GetPerfLevel() == kEnableCount);

perf_context.Reset();
get_perf_context()->Reset();
db_iter->SeekToLast();

ASSERT_TRUE(db_iter->Valid());
ASSERT_EQ(static_cast<int>(perf_context.internal_delete_skipped_count), 1);
ASSERT_EQ(static_cast<int>(get_perf_context()->internal_delete_skipped_count), 1);
ASSERT_EQ(db_iter->key().ToString(), "b");

SetPerfLevel(kDisable);
Expand Down
28 changes: 14 additions & 14 deletions db/db_iterator_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -974,19 +974,19 @@ TEST_F(DBIteratorTest, DBIteratorBoundTest) {
ASSERT_TRUE(iter->Valid());
ASSERT_EQ(iter->key().compare(("b1")), 0);

perf_context.Reset();
get_perf_context()->Reset();
iter->Next();

ASSERT_TRUE(iter->Valid());
ASSERT_EQ(static_cast<int>(perf_context.internal_delete_skipped_count), 2);
ASSERT_EQ(static_cast<int>(get_perf_context()->internal_delete_skipped_count), 2);

// now testing with iterate_bound
Slice prefix("c");
ro.iterate_upper_bound = &prefix;

iter.reset(db_->NewIterator(ro));

perf_context.Reset();
get_perf_context()->Reset();

iter->Seek("b");
ASSERT_TRUE(iter->Valid());
Expand All @@ -1001,7 +1001,7 @@ TEST_F(DBIteratorTest, DBIteratorBoundTest) {
// even though the key is deleted
// hence internal_delete_skipped_count should be 0
ASSERT_TRUE(!iter->Valid());
ASSERT_EQ(static_cast<int>(perf_context.internal_delete_skipped_count), 0);
ASSERT_EQ(static_cast<int>(get_perf_context()->internal_delete_skipped_count), 0);
}
}

Expand Down Expand Up @@ -1888,25 +1888,25 @@ TEST_F(DBIteratorTest, DBIteratorSkipRecentDuplicatesTest) {
#endif

// Seek iterator to a smaller key.
perf_context.Reset();
get_perf_context()->Reset();
iter->Seek("a");
ASSERT_TRUE(iter->Valid());
EXPECT_EQ("b", iter->key().ToString());
EXPECT_EQ("0", iter->value().ToString());

// Check that the seek didn't do too much work.
// Checks are not tight, just make sure that everything is well below 100.
EXPECT_LT(perf_context.internal_key_skipped_count, 4);
EXPECT_LT(perf_context.internal_recent_skipped_count, 8);
EXPECT_LT(perf_context.seek_on_memtable_count, 10);
EXPECT_LT(perf_context.next_on_memtable_count, 10);
EXPECT_LT(perf_context.prev_on_memtable_count, 10);
EXPECT_LT(get_perf_context()->internal_key_skipped_count, 4);
EXPECT_LT(get_perf_context()->internal_recent_skipped_count, 8);
EXPECT_LT(get_perf_context()->seek_on_memtable_count, 10);
EXPECT_LT(get_perf_context()->next_on_memtable_count, 10);
EXPECT_LT(get_perf_context()->prev_on_memtable_count, 10);

// Check that iterator did something like what we expect.
EXPECT_EQ(perf_context.internal_delete_skipped_count, 0);
EXPECT_EQ(perf_context.internal_merge_count, 0);
EXPECT_GE(perf_context.internal_recent_skipped_count, 2);
EXPECT_GE(perf_context.seek_on_memtable_count, 2);
EXPECT_EQ(get_perf_context()->internal_delete_skipped_count, 0);
EXPECT_EQ(get_perf_context()->internal_merge_count, 0);
EXPECT_GE(get_perf_context()->internal_recent_skipped_count, 2);
EXPECT_GE(get_perf_context()->seek_on_memtable_count, 2);
EXPECT_EQ(1, options.statistics->getTickerCount(
NUMBER_OF_RESEEKS_IN_ITERATION));
}
Expand Down
38 changes: 19 additions & 19 deletions db/db_properties_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -533,9 +533,9 @@ TEST_F(DBPropertiesTest, NumImmutableMemTable) {
ASSERT_TRUE(dbfull()->GetProperty(
handles_[1], "rocksdb.num-entries-active-mem-table", &num));
ASSERT_EQ(num, "1");
perf_context.Reset();
get_perf_context()->Reset();
Get(1, "k1");
ASSERT_EQ(1, static_cast<int>(perf_context.get_from_memtable_count));
ASSERT_EQ(1, static_cast<int>(get_perf_context()->get_from_memtable_count));

ASSERT_OK(dbfull()->Put(writeOpt, handles_[1], "k2", big_value));
ASSERT_TRUE(dbfull()->GetProperty(handles_[1],
Expand All @@ -548,12 +548,12 @@ TEST_F(DBPropertiesTest, NumImmutableMemTable) {
handles_[1], "rocksdb.num-entries-imm-mem-tables", &num));
ASSERT_EQ(num, "1");

perf_context.Reset();
get_perf_context()->Reset();
Get(1, "k1");
ASSERT_EQ(2, static_cast<int>(perf_context.get_from_memtable_count));
perf_context.Reset();
ASSERT_EQ(2, static_cast<int>(get_perf_context()->get_from_memtable_count));
get_perf_context()->Reset();
Get(1, "k2");
ASSERT_EQ(1, static_cast<int>(perf_context.get_from_memtable_count));
ASSERT_EQ(1, static_cast<int>(get_perf_context()->get_from_memtable_count));

ASSERT_OK(dbfull()->Put(writeOpt, handles_[1], "k3", big_value));
ASSERT_TRUE(dbfull()->GetProperty(
Expand All @@ -567,15 +567,15 @@ TEST_F(DBPropertiesTest, NumImmutableMemTable) {
ASSERT_TRUE(dbfull()->GetProperty(
handles_[1], "rocksdb.num-entries-imm-mem-tables", &num));
ASSERT_EQ(num, "2");
perf_context.Reset();
get_perf_context()->Reset();
Get(1, "k2");
ASSERT_EQ(2, static_cast<int>(perf_context.get_from_memtable_count));
perf_context.Reset();
ASSERT_EQ(2, static_cast<int>(get_perf_context()->get_from_memtable_count));
get_perf_context()->Reset();
Get(1, "k3");
ASSERT_EQ(1, static_cast<int>(perf_context.get_from_memtable_count));
perf_context.Reset();
ASSERT_EQ(1, static_cast<int>(get_perf_context()->get_from_memtable_count));
get_perf_context()->Reset();
Get(1, "k1");
ASSERT_EQ(3, static_cast<int>(perf_context.get_from_memtable_count));
ASSERT_EQ(3, static_cast<int>(get_perf_context()->get_from_memtable_count));

ASSERT_OK(Flush(1));
ASSERT_TRUE(dbfull()->GetProperty(handles_[1],
Expand Down Expand Up @@ -670,7 +670,7 @@ TEST_F(DBPropertiesTest, DISABLED_GetProperty) {
ASSERT_EQ(num, "0");
ASSERT_TRUE(dbfull()->GetProperty("rocksdb.estimate-num-keys", &num));
ASSERT_EQ(num, "1");
perf_context.Reset();
get_perf_context()->Reset();

ASSERT_OK(dbfull()->Put(writeOpt, "k2", big_value));
ASSERT_TRUE(dbfull()->GetProperty("rocksdb.num-immutable-mem-table", &num));
Expand Down Expand Up @@ -1228,16 +1228,16 @@ TEST_F(DBPropertiesTest, TablePropertiesNeedCompactTest) {

{
SetPerfLevel(kEnableCount);
perf_context.Reset();
get_perf_context()->Reset();
int c = 0;
std::unique_ptr<Iterator> iter(db_->NewIterator(ReadOptions()));
iter->Seek(Key(kMaxKey - 100));
while (iter->Valid() && iter->key().compare(Key(kMaxKey + 100)) < 0) {
iter->Next();
}
ASSERT_EQ(c, 0);
ASSERT_LT(perf_context.internal_delete_skipped_count, 30u);
ASSERT_LT(perf_context.internal_key_skipped_count, 30u);
ASSERT_LT(get_perf_context()->internal_delete_skipped_count, 30u);
ASSERT_LT(get_perf_context()->internal_key_skipped_count, 30u);
SetPerfLevel(kDisable);
}
}
Expand Down Expand Up @@ -1284,16 +1284,16 @@ TEST_F(DBPropertiesTest, NeedCompactHintPersistentTest) {
ASSERT_EQ(NumTableFilesAtLevel(0), 0);
{
SetPerfLevel(kEnableCount);
perf_context.Reset();
get_perf_context()->Reset();
int c = 0;
std::unique_ptr<Iterator> iter(db_->NewIterator(ReadOptions()));
for (iter->Seek(Key(0)); iter->Valid(); iter->Next()) {
c++;
}
ASSERT_EQ(c, 2);
ASSERT_EQ(perf_context.internal_delete_skipped_count, 0);
ASSERT_EQ(get_perf_context()->internal_delete_skipped_count, 0);
// We iterate every key twice. Is it a bug?
ASSERT_LE(perf_context.internal_key_skipped_count, 2);
ASSERT_LE(get_perf_context()->internal_key_skipped_count, 2);
SetPerfLevel(kDisable);
}
}
Expand Down
Loading

0 comments on commit 7f6c02d

Please sign in to comment.