Skip to content

Commit

Permalink
Replace dynamic_cast<>
Browse files Browse the repository at this point in the history
Summary:
Replace dynamic_cast<> so that users can choose to build with RTTI off, so that they can save several bytes per object, and get tiny more memory available.
Some nontrivial changes:
1. Add Comparator::GetRootComparator() to get around the internal comparator hack
2. Add the two experiemental functions to DB
3. Add TableFactory::GetOptionString() to avoid unnecessary casting to get the option string
4. Since 3 is done, move the parsing option functions for table factory to table factory files too, to be symmetric.
Closes facebook#2645

Differential Revision: D5502723

Pulled By: siying

fbshipit-source-id: fd13cec5601cf68a554d87bfcf056f2ffa5fbf7c
  • Loading branch information
siying authored and facebook-github-bot committed Jul 28, 2017
1 parent e85f2c6 commit 21696ba
Show file tree
Hide file tree
Showing 38 changed files with 684 additions and 551 deletions.
1 change: 1 addition & 0 deletions HISTORY.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
## Unreleased
### New Features
* Add Iterator::Refresh(), which allows users to update the iterator state so that they can avoid some initialization costs of recreating iterators.
* Replace dynamic_cast<> (except unit test) so people can choose to build with RTTI off. With make, release mode is by default built with -fno-rtti and debug mode is built without it. Users can override it by setting USE_RTTI=0 or 1.

## 5.7.0 (07/13/2017)
### Public API Change
Expand Down
12 changes: 12 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,19 @@ endif
ifeq ($(DEBUG_LEVEL),0)
OPT += -DNDEBUG
DISABLE_WARNING_AS_ERROR=1

ifneq ($(USE_RTTI), 1)
CXXFLAGS += -fno-rtti
else
CXXFLAGS += -DROCKSDB_USE_RTTI
endif
else
ifneq ($(USE_RTTI), 0)
CXXFLAGS += -DROCKSDB_USE_RTTI
else
CXXFLAGS += -fno-rtti
endif

$(warning Warning: Compiling in debug mode. Don't use the resulting binary in production)
endif

Expand Down
5 changes: 5 additions & 0 deletions cache/clock_cache.cc
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,11 @@ std::shared_ptr<Cache> NewClockCache(size_t capacity, int num_shard_bits,
#include <atomic>
#include <deque>

// "tbb/concurrent_hash_map.h" requires RTTI if exception is enabled.
// Disable it so users can chooose to disable RTTI.
#ifndef ROCKSDB_USE_RTTI
#define TBB_USE_EXCEPTIONS 0
#endif
#include "tbb/concurrent_hash_map.h"

#include "cache/sharded_cache.h"
Expand Down
6 changes: 4 additions & 2 deletions db/convenience.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,18 @@
#include "rocksdb/convenience.h"

#include "db/db_impl.h"
#include "util/cast_util.h"

namespace rocksdb {

void CancelAllBackgroundWork(DB* db, bool wait) {
(dynamic_cast<DBImpl*>(db->GetRootDB()))->CancelAllBackgroundWork(wait);
(static_cast_with_check<DBImpl, DB>(db->GetRootDB()))
->CancelAllBackgroundWork(wait);
}

Status DeleteFilesInRange(DB* db, ColumnFamilyHandle* column_family,
const Slice* begin, const Slice* end) {
return (dynamic_cast<DBImpl*>(db->GetRootDB()))
return (static_cast_with_check<DBImpl, DB>(db->GetRootDB()))
->DeleteFilesInRange(column_family, begin, end);
}

Expand Down
6 changes: 3 additions & 3 deletions db/db_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -235,11 +235,11 @@ class DBImpl : public DB {
ColumnFamilyHandle* column_family,
ColumnFamilyMetaData* metadata) override;

// experimental API
Status SuggestCompactRange(ColumnFamilyHandle* column_family,
const Slice* begin, const Slice* end);
const Slice* begin, const Slice* end) override;

Status PromoteL0(ColumnFamilyHandle* column_family, int target_level);
Status PromoteL0(ColumnFamilyHandle* column_family,
int target_level) override;

// Similar to Write() but will call the callback once on the single write
// thread to determine whether it is safe to perform the write.
Expand Down
3 changes: 3 additions & 0 deletions db/dbformat.h
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,9 @@ class InternalKeyComparator : public Comparator {

int Compare(const InternalKey& a, const InternalKey& b) const;
int Compare(const ParsedInternalKey& a, const ParsedInternalKey& b) const;
virtual const Comparator* GetRootComparator() const override {
return user_comparator_->GetRootComparator();
}
};

// Modules in this directory should keep internal keys wrapped inside
Expand Down
12 changes: 5 additions & 7 deletions db/experimental.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,20 +14,18 @@ namespace experimental {

Status SuggestCompactRange(DB* db, ColumnFamilyHandle* column_family,
const Slice* begin, const Slice* end) {
auto dbimpl = dynamic_cast<DBImpl*>(db);
if (dbimpl == nullptr) {
return Status::InvalidArgument("Didn't recognize DB object");
if (db == nullptr) {
return Status::InvalidArgument("DB is empty");
}

return dbimpl->SuggestCompactRange(column_family, begin, end);
return db->SuggestCompactRange(column_family, begin, end);
}

Status PromoteL0(DB* db, ColumnFamilyHandle* column_family, int target_level) {
auto dbimpl = dynamic_cast<DBImpl*>(db);
if (dbimpl == nullptr) {
if (db == nullptr) {
return Status::InvalidArgument("Didn't recognize DB object");
}
return dbimpl->PromoteL0(column_family, target_level);
return db->PromoteL0(column_family, target_level);
}

#else // ROCKSDB_LITE
Expand Down
5 changes: 3 additions & 2 deletions db/wal_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#include "rocksdb/env.h"
#include "rocksdb/options.h"
#include "rocksdb/write_batch.h"
#include "util/cast_util.h"
#include "util/coding.h"
#include "util/file_reader_writer.h"
#include "util/filename.h"
Expand Down Expand Up @@ -273,8 +274,8 @@ namespace {
struct CompareLogByPointer {
bool operator()(const std::unique_ptr<LogFile>& a,
const std::unique_ptr<LogFile>& b) {
LogFileImpl* a_impl = dynamic_cast<LogFileImpl*>(a.get());
LogFileImpl* b_impl = dynamic_cast<LogFileImpl*>(b.get());
LogFileImpl* a_impl = static_cast_with_check<LogFileImpl, LogFile>(a.get());
LogFileImpl* b_impl = static_cast_with_check<LogFileImpl, LogFile>(b.get());
return *a_impl < *b_impl;
}
};
Expand Down
4 changes: 3 additions & 1 deletion env/mock_env.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include <algorithm>
#include <chrono>
#include "port/sys_time.h"
#include "util/cast_util.h"
#include "util/murmurhash.h"
#include "util/random.h"
#include "util/rate_limiter.h"
Expand Down Expand Up @@ -711,7 +712,8 @@ Status MockEnv::LockFile(const std::string& fname, FileLock** flock) {
}

Status MockEnv::UnlockFile(FileLock* flock) {
std::string fn = dynamic_cast<MockEnvFileLock*>(flock)->FileName();
std::string fn =
static_cast_with_check<MockEnvFileLock, FileLock>(flock)->FileName();
{
MutexLock lock(&mutex_);
if (file_map_.find(fn) != file_map_.end()) {
Expand Down
4 changes: 4 additions & 0 deletions examples/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ ifndef DISABLE_JEMALLOC
PLATFORM_CXXFLAGS += $(JEMALLOC_INCLUDE)
endif

ifneq ($(USE_RTTI), 1)
CXXFLAGS += -fno-rtti
endif

.PHONY: clean librocksdb

all: simple_example column_families_example compact_files_example c_simple_example optimistic_transaction_example transaction_example compaction_filter_example options_file_example
Expand Down
6 changes: 5 additions & 1 deletion examples/compaction_filter_example.cc
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,11 @@ int main() {

MyFilter filter;

system("rm -rf /tmp/rocksmergetest");
int ret = system("rm -rf /tmp/rocksmergetest");
if (ret != 0) {
fprintf(stderr, "Error deleting /tmp/rocksmergetest, code: %d\n", ret);
return ret;
}
rocksdb::Options options;
options.create_if_missing = true;
options.merge_operator.reset(new MyMerge);
Expand Down
4 changes: 4 additions & 0 deletions include/rocksdb/comparator.h
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,10 @@ class Comparator {
// Simple comparator implementations may return with *key unchanged,
// i.e., an implementation of this method that does nothing is correct.
virtual void FindShortSuccessor(std::string* key) const = 0;

// if it is a wrapped comparator, may return the root one.
// return itself it is not wrapped.
virtual const Comparator* GetRootComparator() const { return this; }
};

// Return a builtin comparator that uses lexicographic byte-wise
Expand Down
11 changes: 11 additions & 0 deletions include/rocksdb/db.h
Original file line number Diff line number Diff line change
Expand Up @@ -1097,6 +1097,17 @@ class DB {
virtual Status GetPropertiesOfTablesInRange(
ColumnFamilyHandle* column_family, const Range* range, std::size_t n,
TablePropertiesCollection* props) = 0;

virtual Status SuggestCompactRange(ColumnFamilyHandle* column_family,
const Slice* begin, const Slice* end) {
return Status::NotSupported("SuggestCompactRange() is not implemented.");
}

virtual Status PromoteL0(ColumnFamilyHandle* column_family,
int target_level) {
return Status::NotSupported("PromoteL0() is not implemented.");
}

#endif // ROCKSDB_LITE

// Needed for StackableDB
Expand Down
6 changes: 6 additions & 0 deletions include/rocksdb/table.h
Original file line number Diff line number Diff line change
Expand Up @@ -467,6 +467,12 @@ class TableFactory {
// RocksDB prints configurations at DB Open().
virtual std::string GetPrintableTableOptions() const = 0;

virtual Status GetOptionString(std::string* opt_string,
const std::string& delimiter) const {
return Status::NotSupported(
"The table factory doesn't implement GetOptionString().");
}

// Returns the raw pointer of the table options that is used by this
// TableFactory, or nullptr if this function is not supported.
// Since the return value is a raw pointer, the TableFactory owns the
Expand Down
11 changes: 11 additions & 0 deletions include/rocksdb/utilities/stackable_db.h
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,17 @@ class StackableDB : public DB {
return db_->GetUpdatesSince(seq_number, iter, read_options);
}

virtual Status SuggestCompactRange(ColumnFamilyHandle* column_family,
const Slice* begin,
const Slice* end) override {
return db_->SuggestCompactRange(column_family, begin, end);
}

virtual Status PromoteL0(ColumnFamilyHandle* column_family,
int target_level) override {
return db_->PromoteL0(column_family, target_level);
}

virtual ColumnFamilyHandle* DefaultColumnFamily() const override {
return db_->DefaultColumnFamily();
}
Expand Down
4 changes: 3 additions & 1 deletion monitoring/histogram.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include <stdio.h>

#include "port/port.h"
#include "util/cast_util.h"

namespace rocksdb {

Expand Down Expand Up @@ -255,7 +256,8 @@ void HistogramImpl::Add(uint64_t value) {

void HistogramImpl::Merge(const Histogram& other) {
if (strcmp(Name(), other.Name()) == 0) {
Merge(dynamic_cast<const HistogramImpl&>(other));
Merge(
*static_cast_with_check<const HistogramImpl, const Histogram>(&other));
}
}

Expand Down
5 changes: 4 additions & 1 deletion monitoring/histogram_windowing.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

#include "monitoring/histogram_windowing.h"
#include "monitoring/histogram.h"
#include "util/cast_util.h"

#include <algorithm>

Expand Down Expand Up @@ -64,7 +65,9 @@ void HistogramWindowingImpl::Add(uint64_t value){

void HistogramWindowingImpl::Merge(const Histogram& other) {
if (strcmp(Name(), other.Name()) == 0) {
Merge(dynamic_cast<const HistogramWindowingImpl&>(other));
Merge(
*static_cast_with_check<const HistogramWindowingImpl, const Histogram>(
&other));
}
}

Expand Down
Loading

0 comments on commit 21696ba

Please sign in to comment.