From 00d58a370eb59a002b21d827e1710c1b437c9e6e Mon Sep 17 00:00:00 2001 From: Peter Dillinger Date: Thu, 14 Nov 2019 14:00:58 -0800 Subject: [PATCH] Abandon use of folly::Optional (#6036) Summary: Had complications with LITE build and valgrind test. Reverts/fixes small parts of PR https://github.com/facebook/rocksdb/issues/6007 Pull Request resolved: https://github.com/facebook/rocksdb/pull/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 --- CMakeLists.txt | 2 +- Makefile | 2 +- TARGETS | 1 - buckifier/targets_cfg.py | 1 - db/db_bloom_filter_test.cc | 31 +++++++++++++++++-------------- 5 files changed, 19 insertions(+), 18 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 94aad1a191..45693df502 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -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) diff --git a/Makefile b/Makefile index ebdb53f1f9..b440f408c0 100644 --- a/Makefile +++ b/Makefile @@ -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) diff --git a/TARGETS b/TARGETS index 07e5fe6c88..ab1f24cd76 100644 --- a/TARGETS +++ b/TARGETS @@ -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 = { diff --git a/buckifier/targets_cfg.py b/buckifier/targets_cfg.py index b93f79bb7b..0ecd6fdda7 100644 --- a/buckifier/targets_cfg.py +++ b/buckifier/targets_cfg.py @@ -83,7 +83,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 = { diff --git a/db/db_bloom_filter_test.cc b/db/db_bloom_filter_test.cc index 7d6c917295..a3a47e7d54 100644 --- a/db/db_bloom_filter_test.cc +++ b/db/db_bloom_filter_test.cc @@ -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 -#endif // ROCKSDB_LITE #include "db/db_test_util.h" #include "port/stack_trace.h" #include "rocksdb/perf_context.h" @@ -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. @@ -867,8 +870,7 @@ TEST_F(DBBloomFilterTest, MemtablePrefixBloomOutOfDomain) { #ifndef ROCKSDB_LITE class BloomStatsTestWithParam : public DBBloomFilterTest, - public testing::WithParamInterface< - std::tuple, bool>> { + public testing::WithParamInterface> { public: BloomStatsTestWithParam() { bfp_impl_ = std::get<0>(GetParam()); @@ -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(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_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_; @@ -909,7 +912,7 @@ class BloomStatsTestWithParam static void SetUpTestCase() {} static void TearDownTestCase() {} - folly::Optional bfp_impl_; + BFP2::PseudoMode bfp_impl_; bool partition_filters_; Options options_; }; @@ -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(), false))); + std::make_tuple(BFP2::kPlainTable, false))); namespace { void PrefixScanInit(DBBloomFilterTest* dbtest) {