From d2505cb2966d016b4b28214eb81f4a38860549ea Mon Sep 17 00:00:00 2001 From: Steve Vinoski Date: Thu, 20 Dec 2012 16:14:11 -0500 Subject: [PATCH 3/4] allow Get() calls to avoid copies into std::string Add a new abstract base class leveldb::Value that applications can easily derive from to supply their own memory management for values retrieved via Get(). Add an internal class derived from Value that provides std::string management to preserve backward compatibility. Overload DBImpl::Get() to accept a Value*, and to preserve backward compatibility also keep the original version taking a std::string*. --- db/db_impl.cc | 23 +++++++++++++++++++++++ db/db_impl.h | 3 +++ db/db_test.cc | 5 +++++ db/memtable.cc | 2 +- db/memtable.h | 2 +- db/version_set.cc | 4 ++-- db/version_set.h | 2 +- include/leveldb/db.h | 13 +++++++++++++ 8 files changed, 49 insertions(+), 5 deletions(-) diff --git a/db/db_impl.cc b/db/db_impl.cc index 60c483fab11177fb2d37726f3b2a94720e4dd1ff..50ee5ef22d3416b7bf256b06659e4e705278515d 100644 --- a/db/db_impl.cc +++ b/db/db_impl.cc @@ -79,10 +79,26 @@ struct DBImpl::CompactionState { builder(NULL), total_bytes(0) { } }; +Value::~Value() {} + +class StringValue : public Value { + public: + explicit StringValue(std::string& val) : value_(val) {} + ~StringValue() {} + + StringValue& assign(const char* data, size_t size) { + value_.assign(data, size); + return *this; + } + + private: + std::string& value_; +}; + // Fix user-supplied options to be reasonable template static void ClipToRange(T* ptr, V minvalue, V maxvalue) { if (static_cast(*ptr) > maxvalue) *ptr = maxvalue; if (static_cast(*ptr) < minvalue) *ptr = minvalue; @@ -1110,10 +1126,17 @@ int64_t DBImpl::TEST_MaxNextLevelOverlappingBytes() { } Status DBImpl::Get(const ReadOptions& options, const Slice& key, std::string* value) { + StringValue stringvalue(*value); + return DBImpl::Get(options, key, &stringvalue); +} + +Status DBImpl::Get(const ReadOptions& options, + const Slice& key, + Value* value) { Status s; MutexLock l(&mutex_); SequenceNumber snapshot; if (options.snapshot != NULL) { snapshot = reinterpret_cast(options.snapshot)->number_; diff --git a/db/db_impl.h b/db/db_impl.h index 78f910356318cfdd3bb4ee029a50d8a76161037f..14c44d8751bc838ffd18e44f7398e1f728375b48 100644 --- a/db/db_impl.h +++ b/db/db_impl.h @@ -33,10 +33,13 @@ class DBImpl : public DB { virtual Status Delete(const WriteOptions&, const Slice& key); virtual Status Write(const WriteOptions& options, WriteBatch* updates); virtual Status Get(const ReadOptions& options, const Slice& key, std::string* value); + virtual Status Get(const ReadOptions& options, + const Slice& key, + Value* value); virtual Iterator* NewIterator(const ReadOptions&); virtual const Snapshot* GetSnapshot(); virtual void ReleaseSnapshot(const Snapshot* snapshot); virtual bool GetProperty(const Slice& property, std::string* value); virtual void GetApproximateSizes(const Range* range, int n, uint64_t* sizes); diff --git a/db/db_test.cc b/db/db_test.cc index 641fbabeeb6ed6e2537f024597c984cd4f3b846b..a1769d24aea84863716cd242de4457fb75dd0413 100644 --- a/db/db_test.cc +++ b/db/db_test.cc @@ -1879,10 +1879,15 @@ class ModelDB: public DB { virtual Status Get(const ReadOptions& options, const Slice& key, std::string* value) { assert(false); // Not implemented return Status::NotFound(key); } + virtual Status Get(const ReadOptions& options, + const Slice& key, Value* value) { + assert(false); // Not implemented + return Status::NotFound(key); + } virtual Iterator* NewIterator(const ReadOptions& options) { if (options.snapshot == NULL) { KVMap* saved = new KVMap; *saved = map_; return new ModelIter(saved, true); diff --git a/db/memtable.cc b/db/memtable.cc index bfec0a7e7a1dc210b44dd527b9547e33e829d9bb..82a875fc3abc6ca833c5a396f695652ff8a3dd52 100644 --- a/db/memtable.cc +++ b/db/memtable.cc @@ -103,11 +103,11 @@ void MemTable::Add(SequenceNumber s, ValueType type, memcpy(p, value.data(), val_size); assert((p + val_size) - buf == encoded_len); table_.Insert(buf); } -bool MemTable::Get(const LookupKey& key, std::string* value, Status* s) { +bool MemTable::Get(const LookupKey& key, Value* value, Status* s) { Slice memkey = key.memtable_key(); Table::Iterator iter(&table_); iter.Seek(memkey.data()); if (iter.Valid()) { // entry format is: diff --git a/db/memtable.h b/db/memtable.h index 9f41567cde23dfd645b19d290c6e4a4256804900..6c3f56699798c936531f153a1b45707668935b80 100644 --- a/db/memtable.h +++ b/db/memtable.h @@ -56,11 +56,11 @@ class MemTable { // If memtable contains a value for key, store it in *value and return true. // If memtable contains a deletion for key, store a NotFound() error // in *status and return true. // Else, return false. - bool Get(const LookupKey& key, std::string* value, Status* s); + bool Get(const LookupKey& key, Value* value, Status* s); private: ~MemTable(); // Private since only Unref() should be used to delete it struct KeyComparator { diff --git a/db/version_set.cc b/db/version_set.cc index b1256f90e1c2bc6f9f6f449029bed9266bbb55b9..f0a523930d3382983fddfe27ee700574ddd06b3d 100644 --- a/db/version_set.cc +++ b/db/version_set.cc @@ -260,11 +260,11 @@ enum SaverState { }; struct Saver { SaverState state; const Comparator* ucmp; Slice user_key; - std::string* value; + Value* value; }; } static void SaveValue(void* arg, const Slice& ikey, const Slice& v) { Saver* s = reinterpret_cast(arg); ParsedInternalKey parsed_key; @@ -329,11 +329,11 @@ void Version::ForEachOverlapping(Slice user_key, Slice internal_key, } } Status Version::Get(const ReadOptions& options, const LookupKey& k, - std::string* value, + Value* value, GetStats* stats) { Slice ikey = k.internal_key(); Slice user_key = k.user_key(); const Comparator* ucmp = vset_->icmp_.user_comparator(); Status s; diff --git a/db/version_set.h b/db/version_set.h index c4e7ac360b87d842ee9dbc0a2bf80f122a65dad7..2d31542cff63b9058c991e3bd0b67f41102edbed 100644 --- a/db/version_set.h +++ b/db/version_set.h @@ -68,11 +68,11 @@ class Version { // REQUIRES: lock is not held struct GetStats { FileMetaData* seek_file; int seek_file_level; }; - Status Get(const ReadOptions&, const LookupKey& key, std::string* val, + Status Get(const ReadOptions&, const LookupKey& key, Value* val, GetStats* stats); // Adds "stats" into the current state. Returns true if a new // compaction may need to be triggered, false otherwise. // REQUIRES: lock is held diff --git a/include/leveldb/db.h b/include/leveldb/db.h index a69704d297c3feb1f60dc2856d6f9709a8879a86..12f788ebf76341102832f45132388ef432db7e25 100644 --- a/include/leveldb/db.h +++ b/include/leveldb/db.h @@ -36,10 +36,21 @@ struct Range { Range() { } Range(const Slice& s, const Slice& l) : start(s), limit(l) { } }; +// Abstract holder for a DB value. +// This allows callers to manage their own value buffers and have +// DB values copied directly into those buffers. +class Value { + public: + virtual Value& assign(const char* data, size_t size) = 0; + + protected: + virtual ~Value(); +}; + // A DB is a persistent ordered map from keys to values. // A DB is safe for concurrent access from multiple threads without // any external synchronization. class DB { public: @@ -80,10 +91,12 @@ class DB { // a status for which Status::IsNotFound() returns true. // // May return some other Status on an error. virtual Status Get(const ReadOptions& options, const Slice& key, std::string* value) = 0; + virtual Status Get(const ReadOptions& options, + const Slice& key, Value* value) = 0; // Return a heap-allocated iterator over the contents of the database. // The result of NewIterator() is initially invalid (caller must // call one of the Seek methods on the iterator before using it). // -- 2.14.2