[C++] New API for clean handling of variable length keys #282
Conversation
|
|
ufminhas
changed the title
| { } | ||
|
|
||
| inline uint32_t size() const { | ||
| return Key::size(key_length_); |
There was a problem hiding this comment.
key_length_ is uint64_t which throws warning as error here.
| return KeyHash(Utility::HashBytesUint8(key_data_, key_length_)); | ||
| } | ||
| inline void write_deep_key_at(Key* dst) const { | ||
| Key::Create(dst, key_length_, key_data_); |
There was a problem hiding this comment.
key_length_ is uint64_t which throws warning as error here.
| @@ -89,7 +90,10 @@ class PendingContext : public IAsyncContext { | |||
| entry = entry_; | |||
| } | |||
|
|
|||
| virtual const key_t& key() const = 0; | |||
| virtual uint32_t key_size() const = 0; | |||
| virtual void write_deep_key_at(key_t* dst) const = 0; | |||
There was a problem hiding this comment.
any perf implication of virtual here?
There was a problem hiding this comment.
The 'virtual' is OK: the sync contexts (e.g. PendingUpsertContext)have the finalized class (with overrides marked as 'final') as template parameter, and the compiler will be able to get rid of the virtual call. The async contexts (e.g. AsyncPendingUpsertContext) will need to go through the virtual call, but the async contexts codepath only triggers when there is a disk IO, so the cost of the virtual call does not matter (the disk IO takes much longer already).
Also I was not introducing the 'virtual' func thing: the code was already doing this, just look at the line being removed (the virtual const key_t& key() function).
|
Merge updates from cc-latest-mods |
In this diff, we propose the new API for clean handling of VariantLengthKey.
Previously, handling VariantLengthKey is extremely tricky and fragile (if possible at all),
and requires writing code that any project with reasonable code quality standards would not tolerate.
This is even causing bugs in our own codebase (e.g. Compaction contexts, and even the test that tests VariantLengthKey is buggy itself).
We propose a backward-compatible new API to handle VariantLengthKey cleanly.
We add a new concept -- ShallowKey. This class is required to provide the same APIs
as the Key class (size(), GetHash() and operator==()), but unlike Key class,
which raw contents (interpreted as a uint8_t* string) is directly written into the log,
ShallowKey's internal representation does not matter.
In addition to the existing APIs, a new API with prototype
void write_deep_key_at(Key* dst) const
is required, which should write the bytestream-representation of the Key into address 'dst'.
In order to use the new API, all you need to do is to change the key() API in Context to return
ShallowKey type, instead of key_t type. Example:
struct UpsertContext
{
using key_t = Key;
// uncomment below to use new API
// ShallowKey key();
// uncomment below to use old API
// key_t key();
};
|
Amazing work, thanks for your contribution! |
badrishc
changed the title
|
In this diff, we propose the new API for clean handling of VariantLengthKey.
Previously, handling VariantLengthKey is extremely tricky and fragile (if possible at all),
and requires writing code that any project with reasonable code quality standards would not tolerate.
This is even causing bugs in our own codebase (e.g. Compaction contexts, and even the test that tests VariantLengthKey is buggy itself).
We propose a backward-compatible new API to handle VariantLengthKey cleanly.
We add a new concept -- ShallowKey. This class is required to provide the same APIs
as the Key class (size(), GetHash() and operator==()), but unlike Key class,
which raw contents (interpreted as a uint8_t* string) is directly written into the log,
ShallowKey's internal representation does not matter.
In addition to the existing APIs, a new API with prototype
void write_deep_key_at(Key* dst) const
is required, which should write the bytestream-representation of the Key into address 'dst'.
In order to use the new API, all you need to do is to change the key() API in Context to return
ShallowKey type, instead of key_t type. Example:
struct UpsertContext
{
using key_t = Key;
};