From 52c5159bc0ffa4aaa0b3f062b0fdc360dd1382bd Mon Sep 17 00:00:00 2001 From: sale Date: Wed, 7 Dec 2016 13:20:53 +0000 Subject: [PATCH] Bloom filter code review changes --- ...asic_bloom_filter.hpp => bloom_filter.hpp} | 19 ++-- .../bloom/basic_bloom_filter.cpp | 6 +- .../concurrent/concurrent_bloom_map.cpp | 90 +------------------ tests/unit/basic_bloom_filter.cpp | 8 +- 4 files changed, 24 insertions(+), 99 deletions(-) rename include/data_structures/bloom/{basic_bloom_filter.hpp => bloom_filter.hpp} (75%) diff --git a/include/data_structures/bloom/basic_bloom_filter.hpp b/include/data_structures/bloom/bloom_filter.hpp similarity index 75% rename from include/data_structures/bloom/basic_bloom_filter.hpp rename to include/data_structures/bloom/bloom_filter.hpp index 99e26f0aa..33da0df80 100644 --- a/include/data_structures/bloom/basic_bloom_filter.hpp +++ b/include/data_structures/bloom/bloom_filter.hpp @@ -2,8 +2,17 @@ #include #include +/* + Implementation of a generic Bloom Filter. + + Read more about bloom filters here: + http://en.wikipedia.org/wiki/Bloom_filter + http://www.jasondavies.com/bloomfilter/ +*/ + +// Type specifies the type of data stored template -class BasicBloomFilter { +class BloomFilter { private: using HashFunction = std::function; using CompresionFunction = std::function; @@ -28,13 +37,13 @@ class BasicBloomFilter { } public: - BasicBloomFilter(std::vector funcs, - CompresionFunction compression = {}) + BloomFilter(std::vector funcs, + CompresionFunction compression = {}) : hashes_(funcs) { if (!compression) - compression_ = std::bind(&BasicBloomFilter::default_compression, this, + compression_ = std::bind(&BloomFilter::default_compression, this, std::placeholders::_1); - else + else compression_ = compression; buckets.resize(hashes_.size()); diff --git a/tests/benchmark/data_structures/bloom/basic_bloom_filter.cpp b/tests/benchmark/data_structures/bloom/basic_bloom_filter.cpp index 231993fcb..36a74506d 100644 --- a/tests/benchmark/data_structures/bloom/basic_bloom_filter.cpp +++ b/tests/benchmark/data_structures/bloom/basic_bloom_filter.cpp @@ -1,7 +1,7 @@ #include #include -#include "data_structures/bloom/basic_bloom_filter.hpp" +#include "data_structures/bloom/bloom_filter.hpp" #include "logging/default.hpp" #include "logging/streams/stdout.hpp" #include "utils/command_line/arguments.hpp" @@ -14,7 +14,7 @@ using utils::random::StringGenerator; using StringHashFunction = std::function; template -static void TestBloom(benchmark::State& state, BasicBloomFilter* +static void TestBloom(benchmark::State& state, BloomFilter* bloom, const std::vector& elements) { while(state.KeepRunning()) { for (int start = 0; start < state.range(0); start++) @@ -46,7 +46,7 @@ int main(int argc, char** argv) { hash1, hash2 }; - BasicBloomFilter bloom(funcs); + BloomFilter bloom(funcs); benchmark::RegisterBenchmark("SimpleBloomFilter Benchmark Test", BM_Bloom, &bloom, elements) diff --git a/tests/benchmark/data_structures/concurrent/concurrent_bloom_map.cpp b/tests/benchmark/data_structures/concurrent/concurrent_bloom_map.cpp index 17df96bb5..f305d8b20 100644 --- a/tests/benchmark/data_structures/concurrent/concurrent_bloom_map.cpp +++ b/tests/benchmark/data_structures/concurrent/concurrent_bloom_map.cpp @@ -1,7 +1,7 @@ #include #include -#include "data_structures/bloom/basic_bloom_filter.hpp" +#include "data_structures/bloom/bloom_filter.hpp" #include "data_structures/concurrent/concurrent_bloom_map.hpp" #include "logging/default.hpp" #include "logging/streams/stdout.hpp" @@ -50,21 +50,6 @@ static void InsertValue(benchmark::State& state, ConcurrentBloomMap* ma state.SetComplexityN(state.range(0)); } -/* - ConcurrentMap Deletion Benchmark Test -template -static void DeleteValue(benchmark::State& state, ConcurrentBloomMap* map, - const std::vector> elements) { - while (state.KeepRunning()) { - auto accessor = map->access(); - for (int start = 0; start < state.range(0); start++) { - accessor.remove(elements[start].first); - } - } - state.SetComplexityN(state.range(0)); -} -*/ - /* ConcurrentMap Contains Benchmark Test */ @@ -83,12 +68,6 @@ auto BM_InsertValue = [](benchmark::State& state, auto* map, auto& elements) { InsertValue(state, map, elements); }; -/* -auto BM_DeleteValue = [](benchmark::State& state, auto* map, auto elements) { - DeleteValue(state, map, elements); -}; -*/ - auto BM_ContainsValue = [](benchmark::State& state, auto* map, auto elements) { ContainsValue(state, map, elements); }; @@ -154,12 +133,12 @@ int main(int argc, char** argv) { hash1, hash2 }; - BasicBloomFilter bloom_filter_(funcs); + BloomFilter bloom_filter_(funcs); // maps used for testing //ConcurrentBloomMap ii_map; //ConcurrentBloomMap is_map; - using Filter = BasicBloomFilter; + using Filter = BloomFilter; ConcurrentBloomMap si_map(bloom_filter_); ConcurrentBloomMap ss_map(bloom_filter_); @@ -171,21 +150,6 @@ ss_map(bloom_filter_); auto ss_elems = utils::random::generate_vector(pssg, MAX_ELEMENTS); /* insertion Tests */ - /* - benchmark::RegisterBenchmark("InsertValue[Int, Int]", BM_InsertValue, &ii_map, - ii_elems) - ->RangeMultiplier(MULTIPLIER) - ->Range(1, MAX_ELEMENTS) - ->Complexity(benchmark::oN) - ->Threads(THREADS); - - benchmark::RegisterBenchmark("InsertValue[Int, String]", BM_InsertValue, - &is_map, is_elems) - ->RangeMultiplier(MULTIPLIER) - ->Range(1, MAX_ELEMENTS) - ->Complexity(benchmark::oN) - ->Threads(THREADS); - */ benchmark::RegisterBenchmark("InsertValue[String, Int]", BM_InsertValue, &si_map, si_elems) ->RangeMultiplier(MULTIPLIER) @@ -201,22 +165,6 @@ ss_map(bloom_filter_); ->Threads(THREADS); // Contains Benchmark Tests - - /* - benchmark::RegisterBenchmark("ContainsValue[Int, Int]", BM_ContainsValue, - &ii_map, ii_elems) - ->RangeMultiplier(MULTIPLIER) - ->Range(1, MAX_ELEMENTS) - ->Complexity(benchmark::oN) - ->Threads(THREADS); - - benchmark::RegisterBenchmark("ContainsValue[Int, String]", BM_ContainsValue, - &is_map, is_elems) - ->RangeMultiplier(MULTIPLIER) - ->Range(1, MAX_ELEMENTS) - ->Complexity(benchmark::oN) - ->Threads(THREADS); - */ benchmark::RegisterBenchmark("ContainsValue[String, Int]", BM_ContainsValue, &si_map, si_elems) ->RangeMultiplier(MULTIPLIER) @@ -231,38 +179,6 @@ ss_map(bloom_filter_); ->Complexity(benchmark::oN) ->Threads(THREADS); - // Deletion Banchamark Tests - /* - - benchmark::RegisterBenchmark("DeleteValue[Int, Int]", BM_DeleteValue, &ii_map, - ii_elems) - ->RangeMultiplier(MULTIPLIER) - ->Range(1, MAX_ELEMENTS) - ->Complexity(benchmark::oN) - ->Threads(THREADS); - - benchmark::RegisterBenchmark("DeleteValue[Int, String]", BM_DeleteValue, - &is_map, is_elems) - ->RangeMultiplier(MULTIPLIER) - ->Range(1, MAX_ELEMENTS) - ->Complexity(benchmark::oN) - ->Threads(THREADS); - - benchmark::RegisterBenchmark("DeleteValue[String, Int]", BM_DeleteValue, - &si_map, si_elems) - ->RangeMultiplier(MULTIPLIER) - ->Range(1, MAX_ELEMENTS) - ->Complexity(benchmark::oN) - ->Threads(THREADS); - - benchmark::RegisterBenchmark("DeleteValue[String, String]", BM_DeleteValue, - &ss_map, ss_elems) - ->RangeMultiplier(MULTIPLIER) - ->Range(1, MAX_ELEMENTS) - ->Complexity(benchmark::oN) - ->Threads(THREADS); - */ - benchmark::Initialize(&argc, argv); benchmark::RunSpecifiedBenchmarks(); diff --git a/tests/unit/basic_bloom_filter.cpp b/tests/unit/basic_bloom_filter.cpp index 77484ca08..ac4df7fc2 100644 --- a/tests/unit/basic_bloom_filter.cpp +++ b/tests/unit/basic_bloom_filter.cpp @@ -4,14 +4,14 @@ #include "utils/command_line/arguments.hpp" #include "utils/hashing/fnv64.hpp" -#include "data_structures/bloom/basic_bloom_filter.hpp" +#include "data_structures/bloom/bloom_filter.hpp" #pragma clang diagnostic push #pragma clang diagnostic ignored "-Wwritable-strings" using StringHashFunction = std::function; -TEST_CASE("BasicBloomFilter Test") { +TEST_CASE("BloomFilter Test") { StringHashFunction hash1 = fnv64; StringHashFunction hash2 = fnv1a64; @@ -22,10 +22,10 @@ TEST_CASE("BasicBloomFilter Test") { hash1, hash2 }; - BasicBloomFilter bloom(funcs); + BloomFilter bloom(funcs); std::string test = "test"; - std::string kifla = "pizda"; + std::string kifla = "kifla"; std::cout << hash1(test) << std::endl; std::cout << hash2(test) << std::endl;