From 43dde332cb5fa3113489655e615efe20a630d4c2 Mon Sep 17 00:00:00 2001 From: Peter Dillinger Date: Sat, 22 Feb 2020 07:59:38 -0800 Subject: [PATCH] Share kPageSize (and other small tweaks) (#6443) Summary: Make kPageSize extern const size_t (used in draft https://github.com/facebook/rocksdb/issues/6427) Make kLitteEndian constexpr bool Clarify a couple of comments Pull Request resolved: https://github.com/facebook/rocksdb/pull/6443 Test Plan: make check, CI Differential Revision: D20044558 Pulled By: pdillinger fbshipit-source-id: e0c5cc13229c82726280dc0ddcba4078346b8418 --- env/env_test.cc | 14 +++++--------- include/rocksdb/advanced_options.h | 8 +++----- include/rocksdb/filter_policy.h | 3 ++- port/port_posix.cc | 12 ++++++++++++ port/port_posix.h | 4 +++- port/win/port_win.cc | 3 +++ port/win/port_win.h | 15 ++++++--------- 7 files changed, 34 insertions(+), 25 deletions(-) diff --git a/env/env_test.cc b/env/env_test.cc index a0aa08f327..98f73d9a9d 100644 --- a/env/env_test.cc +++ b/env/env_test.cc @@ -42,14 +42,10 @@ #include "util/mutexlock.h" #include "util/string_util.h" -#ifdef OS_LINUX -static const size_t kPageSize = sysconf(_SC_PAGESIZE); -#else -static const size_t kPageSize = 4 * 1024; -#endif - namespace ROCKSDB_NAMESPACE { +using port::kPageSize; + static const int kDelayMicros = 100000; struct Deleter { @@ -881,9 +877,10 @@ TEST_F(EnvPosixTest, PositionedAppend) { // Verify the above std::unique_ptr seq_file; ASSERT_OK(env_->NewSequentialFile(ift.name() + "/f", &seq_file, options)); - char scratch[kPageSize * 2]; + size_t scratch_len = kPageSize * 2; + std::unique_ptr scratch(new char[scratch_len]); Slice result; - ASSERT_OK(seq_file->Read(sizeof(scratch), &result, scratch)); + ASSERT_OK(seq_file->Read(scratch_len, &result, scratch.get())); ASSERT_EQ(kPageSize + kBlockSize, result.size()); ASSERT_EQ('a', result[kBlockSize - 1]); ASSERT_EQ('b', result[kBlockSize]); @@ -977,7 +974,6 @@ TEST_P(EnvPosixTestWithParam, AllocateTest) { // allocate 100 MB size_t kPreallocateSize = 100 * 1024 * 1024; size_t kBlockSize = 512; - size_t kPageSize = 4096; size_t kDataSize = 1024 * 1024; auto data_ptr = NewAligned(kDataSize, 'A'); Slice data(data_ptr.get(), kDataSize); diff --git a/include/rocksdb/advanced_options.h b/include/rocksdb/advanced_options.h index 8e4f93743e..a72edbe059 100644 --- a/include/rocksdb/advanced_options.h +++ b/include/rocksdb/advanced_options.h @@ -151,7 +151,6 @@ enum UpdateStatus { // Return status For inplace update callback UPDATED = 2, // No inplace update. Merged value set }; - struct AdvancedColumnFamilyOptions { // The maximum number of write buffers that are built up in memory. // The default and the minimum number is 2, so that when 1 write buffer @@ -330,10 +329,9 @@ struct AdvancedColumnFamilyOptions { std::shared_ptr memtable_insert_with_hint_prefix_extractor = nullptr; - // Control locality of bloom filter probes to improve cache miss rate. - // This option only applies to memtable prefix bloom and plaintable - // prefix bloom. It essentially limits every bloom checking to one cache line. - // This optimization is turned off when set to 0, and positive number to turn + // Control locality of bloom filter probes to improve CPU cache hit rate. + // This option now only applies to plaintable prefix bloom. This + // optimization is turned off when set to 0, and positive number to turn // it on. // Default: 0 uint32_t bloom_locality = 0; diff --git a/include/rocksdb/filter_policy.h b/include/rocksdb/filter_policy.h index c7e4e720a6..03d6471cff 100644 --- a/include/rocksdb/filter_policy.h +++ b/include/rocksdb/filter_policy.h @@ -84,7 +84,8 @@ class FilterBitsReader { }; // Contextual information passed to BloomFilterPolicy at filter building time. -// Used in overriding FilterPolicy::GetBuilderWithContext(). +// Used in overriding FilterPolicy::GetBuilderWithContext(). References other +// structs because this is expected to be a temporary, stack-allocated object. struct FilterBuildingContext { // This constructor is for internal use only and subject to change. FilterBuildingContext(const BlockBasedTableOptions& table_options); diff --git a/port/port_posix.cc b/port/port_posix.cc index f5001411cc..e3ea5ca696 100644 --- a/port/port_posix.cc +++ b/port/port_posix.cc @@ -217,6 +217,18 @@ void cacheline_aligned_free(void *memblock) { free(memblock); } +static size_t GetPageSize() { +#if defined(OS_LINUX) || defined(_SC_PAGESIZE) + long v = sysconf(_SC_PAGESIZE); + if (v >= 1024) { + return static_cast(v); + } +#endif + // Default assume 4KB + return 4U * 1024U; +} + +const size_t kPageSize = GetPageSize(); } // namespace port } // namespace ROCKSDB_NAMESPACE diff --git a/port/port_posix.h b/port/port_posix.h index f08a4ee220..0c9c69833d 100644 --- a/port/port_posix.h +++ b/port/port_posix.h @@ -99,7 +99,7 @@ const int64_t kMaxInt64 = std::numeric_limits::max(); const int64_t kMinInt64 = std::numeric_limits::min(); const size_t kMaxSizet = std::numeric_limits::max(); -static const bool kLittleEndian = PLATFORM_IS_LITTLE_ENDIAN; +constexpr bool kLittleEndian = PLATFORM_IS_LITTLE_ENDIAN; #undef PLATFORM_IS_LITTLE_ENDIAN class CondVar; @@ -212,5 +212,7 @@ extern void Crash(const std::string& srcfile, int srcline); extern int GetMaxOpenFiles(); +extern const size_t kPageSize; + } // namespace port } // namespace ROCKSDB_NAMESPACE diff --git a/port/win/port_win.cc b/port/win/port_win.cc index 5a19c33a87..6e43b1b58f 100644 --- a/port/win/port_win.cc +++ b/port/win/port_win.cc @@ -262,5 +262,8 @@ void Crash(const std::string& srcfile, int srcline) { int GetMaxOpenFiles() { return -1; } +// Assume 4KB page size +const size_t kPageSize = 4U * 1024U; + } // namespace port } // namespace ROCKSDB_NAMESPACE diff --git a/port/win/port_win.h b/port/win/port_win.h index 80f68bcdee..76fc8cf39f 100644 --- a/port/win/port_win.h +++ b/port/win/port_win.h @@ -16,10 +16,6 @@ #define WIN32_LEAN_AND_MEAN #endif -// Assume that for everywhere -#undef PLATFORM_IS_LITTLE_ENDIAN -#define PLATFORM_IS_LITTLE_ENDIAN true - #include #include #include @@ -70,10 +66,6 @@ typedef SSIZE_T ssize_t; #endif -#ifndef PLATFORM_IS_LITTLE_ENDIAN -#define PLATFORM_IS_LITTLE_ENDIAN (__BYTE_ORDER == __LITTLE_ENDIAN) -#endif - namespace ROCKSDB_NAMESPACE { #define PREFETCH(addr, rw, locality) @@ -122,7 +114,10 @@ const size_t kMaxSizet = std::numeric_limits::max(); #endif //_MSC_VER -const bool kLittleEndian = true; +// "Windows is designed to run on little-endian computer architectures." +// https://docs.microsoft.com/en-us/windows/win32/sysinfo/registry-value-types +constexpr bool kLittleEndian = true; +#undef PLATFORM_IS_LITTLE_ENDIAN class CondVar; @@ -270,6 +265,8 @@ inline void cacheline_aligned_free(void *memblock) { #endif } +extern const size_t kPageSize; + // https://gcc.gnu.org/bugzilla/show_bug.cgi?id=52991 for MINGW32 // could not be worked around with by -mno-ms-bitfields #ifndef __MINGW32__