Skip to content

Commit

Permalink
Abandon use of folly::Optional (facebook#6036)
Browse files Browse the repository at this point in the history
Summary:
Had complications with LITE build and valgrind test.
Reverts/fixes small parts of PR facebook#6007
Pull Request resolved: facebook#6036

Test Plan:
make LITE=1 all check
and
ROCKSDB_VALGRIND_RUN=1 DISABLE_JEMALLOC=1 make -j24 db_bloom_filter_test && ROCKSDB_VALGRIND_RUN=1 DISABLE_JEMALLOC=1 ./db_bloom_filter_test

Differential Revision: D18512238

Pulled By: pdillinger

fbshipit-source-id: 37213cf0d309edf11c483fb4b2fb6c02c2cf2b28
  • Loading branch information
pdillinger authored and facebook-github-bot committed Nov 14, 2019
1 parent 6123611 commit 00d58a3
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 18 deletions.
2 changes: 1 addition & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -470,7 +470,7 @@ endif()
include_directories(${PROJECT_SOURCE_DIR})
include_directories(${PROJECT_SOURCE_DIR}/include)
include_directories(SYSTEM ${PROJECT_SOURCE_DIR}/third-party/gtest-1.8.1/fused-src)
if(NOT ROCKSDB_LITE)
if(WITH_FOLLY_DISTRIBUTED_MUTEX)
include_directories(${PROJECT_SOURCE_DIR}/third-party/folly)
endif()
find_package(Threads REQUIRED)
Expand Down
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ else
PLATFORM_CXXFLAGS += -isystem $(GTEST_DIR)
endif

ifeq ($(filter -DROCKSDB_LITE,$(OPT)),)
ifeq ($(USE_FOLLY_DISTRIBUTED_MUTEX),1)
FOLLY_DIR = ./third-party/folly
# AIX: pre-defined system headers are surrounded by an extern "C" block
ifeq ($(PLATFORM), OS_AIX)
Expand Down
1 change: 0 additions & 1 deletion TARGETS
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,6 @@ ROCKSDB_PREPROCESSOR_FLAGS = [
# Directories with files for #include
"-I" + REPO_PATH + "include/",
"-I" + REPO_PATH,
"-I" + REPO_PATH + "third-party/folly/",
]

ROCKSDB_ARCH_PREPROCESSOR_FLAGS = {
Expand Down
1 change: 0 additions & 1 deletion buckifier/targets_cfg.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@
# Directories with files for #include
"-I" + REPO_PATH + "include/",
"-I" + REPO_PATH,
"-I" + REPO_PATH + "third-party/folly/",
]
ROCKSDB_ARCH_PREPROCESSOR_FLAGS = {
Expand Down
31 changes: 17 additions & 14 deletions db/db_bloom_filter_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,6 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. See the AUTHORS file for names of contributors.

#ifndef ROCKSDB_LITE
#include <folly/Optional.h>
#endif // ROCKSDB_LITE
#include "db/db_test_util.h"
#include "port/stack_trace.h"
#include "rocksdb/perf_context.h"
Expand All @@ -19,6 +16,12 @@ namespace rocksdb {

namespace {
using BFP = BloomFilterPolicy;

namespace BFP2 {
// Extends BFP::Mode with option to use Plain table
using PseudoMode = int;
static constexpr PseudoMode kPlainTable = -1;
} // namespace BFP2
} // namespace

// DB tests related to bloom filter.
Expand Down Expand Up @@ -867,8 +870,7 @@ TEST_F(DBBloomFilterTest, MemtablePrefixBloomOutOfDomain) {
#ifndef ROCKSDB_LITE
class BloomStatsTestWithParam
: public DBBloomFilterTest,
public testing::WithParamInterface<
std::tuple<folly::Optional<BFP::Mode>, bool>> {
public testing::WithParamInterface<std::tuple<BFP2::PseudoMode, bool>> {
public:
BloomStatsTestWithParam() {
bfp_impl_ = std::get<0>(GetParam());
Expand All @@ -878,21 +880,22 @@ class BloomStatsTestWithParam
options_.prefix_extractor.reset(rocksdb::NewFixedPrefixTransform(4));
options_.memtable_prefix_bloom_size_ratio =
8.0 * 1024.0 / static_cast<double>(options_.write_buffer_size);
if (bfp_impl_) {
if (bfp_impl_ == BFP2::kPlainTable) {
assert(!partition_filters_); // not supported in plain table
PlainTableOptions table_options;
options_.table_factory.reset(NewPlainTableFactory(table_options));
} else {
BlockBasedTableOptions table_options;
table_options.hash_index_allow_collision = false;
if (partition_filters_) {
assert(*bfp_impl_ != BFP::kDeprecatedBlock);
assert(bfp_impl_ != BFP::kDeprecatedBlock);
table_options.partition_filters = partition_filters_;
table_options.index_type =
BlockBasedTableOptions::IndexType::kTwoLevelIndexSearch;
}
table_options.filter_policy.reset(new BFP(10, *bfp_impl_));
table_options.filter_policy.reset(
new BFP(10, static_cast<BFP::Mode>(bfp_impl_)));
options_.table_factory.reset(NewBlockBasedTableFactory(table_options));
} else {
assert(!partition_filters_); // not supported in plain table
PlainTableOptions table_options;
options_.table_factory.reset(NewPlainTableFactory(table_options));
}
options_.env = env_;

Expand All @@ -909,7 +912,7 @@ class BloomStatsTestWithParam
static void SetUpTestCase() {}
static void TearDownTestCase() {}

folly::Optional<BFP::Mode> bfp_impl_;
BFP2::PseudoMode bfp_impl_;
bool partition_filters_;
Options options_;
};
Expand Down Expand Up @@ -1030,7 +1033,7 @@ INSTANTIATE_TEST_CASE_P(
std::make_tuple(BFP::kLegacyBloom, true),
std::make_tuple(BFP::kFastLocalBloom, false),
std::make_tuple(BFP::kFastLocalBloom, true),
std::make_tuple(folly::Optional<BFP::Mode>(), false)));
std::make_tuple(BFP2::kPlainTable, false)));

namespace {
void PrefixScanInit(DBBloomFilterTest* dbtest) {
Expand Down

0 comments on commit 00d58a3

Please sign in to comment.