Optimize MVCCGet path. #459
Conversation
| @@ -188,6 +188,13 @@ func PutProto(engine Engine, key proto.EncodedKey, msg gogoproto.Message) (keyBy | |||
| // key was not found. On success, returns the length in bytes of the | |||
| // key and the value. | |||
| func GetProto(engine Engine, key proto.EncodedKey, msg gogoproto.Message) (ok bool, keyBytes, valBytes int64, err error) { | |||
| type protoGetter interface { | |||
There was a problem hiding this comment.
Any interfaces used should generally be public and documented; it can be surprising when you pass an Engine to a function and a method that is not a part of the Engine interface is called. Or we could just add GetProto to the Engine interface to ensure that Batch and Snapshot implement it and pass it along. (there's no good way for wrappers to implement protoGetter iff their wrapped object does).
Instead of a new method on Engine, we may want to change the interface of Engine.Get to take an object to receive the bytes (similar to json.Unmarshaler) instead of returning the bytes. There aren't that many references to Engine.Get so it shouldn't be too hard to change.
There was a problem hiding this comment.
I like the Unmarshaler idea. I'll play around with that tomorrow.
There was a problem hiding this comment.
I decided to just add GetProto to Engine and ValueProto to Iterator. Not as general as the Unmarshaler idea, but simpler.
Provide fast-paths for retrieving unmarshalling proto values that do not copy the data into a Go slice. Avoid unnecessary allocations when encoding mvcc keys. benchmark old ns/op new ns/op delta BenchmarkMVCCScan1Version1Row 11322 10721 -5.31% BenchmarkMVCCScan1Version10Rows 36566 27190 -25.64% BenchmarkMVCCScan1Version100Rows 276241 217066 -21.42% BenchmarkMVCCScan1Version1000Rows 2205243 1834826 -16.80% BenchmarkMVCCGet1Version 3739 3153 -15.67% benchmark old MB/s new MB/s speedup BenchmarkMVCCScan1Version1Row 90.44 95.51 1.06x BenchmarkMVCCScan1Version10Rows 280.04 376.60 1.34x BenchmarkMVCCScan1Version100Rows 370.69 471.74 1.27x BenchmarkMVCCScan1Version1000Rows 464.35 558.09 1.20x BenchmarkMVCCGet1Version 273.83 324.77 1.19x
petermattis
force-pushed
the
pmattis/mvcc-get-opt
branch
from
9d44e5a to
b7a9255
Compare
|
Not quite ready for another look. I have to track down the stuff about a 2GB data limit and what the appropriate constant is to use there. |
Add Engine.GetProto and Iterator.ValueProto functions to the Engine and Iterator interfaces. Provide implementations of these functions for the existing engines and iterators.
petermattis
force-pushed
the
pmattis/mvcc-get-opt
branch
from
590adf4 to
1eda5c2
Compare
|
This is ready for another look. |
|
LGTM |
| metaKey := mvccEncodeKey(buf.key[0:0], key) | ||
| ok, _, _, err := engine.GetProto(metaKey, &buf.meta) | ||
| if err != nil || !ok { | ||
| getBufferPool.Put(buf) |
There was a problem hiding this comment.
Any reason not to defer this?
There was a problem hiding this comment.
None. Switched to using defer. The perf numbers are unchanged.
| } | ||
|
|
||
| // getEarlierFunc fetches an earlier version of a key starting at | ||
| // start and ending at end. Returns the value as a byte slice, the | ||
| // timestamp of the earlier version, a boolean indicating whether a | ||
| // version value or metadata was found, and error, if applicable. | ||
| type getEarlierFunc func(engine Engine, start, end proto.EncodedKey) (proto.RawKeyValue, error) | ||
| type getValueFunc func(engine Engine, start, end proto.EncodedKey, |
There was a problem hiding this comment.
getEarlierFunc seemed like a clearer name to me.
There was a problem hiding this comment.
I was confused by the previous code that getEarlierFunc was being used to retrieve both the metadata and the values. The getValueFunc is only used to retrieve values. This seems cleaner to me.
Provide fast-paths for retrieving unmarshalling proto values that do not
copy the data into a Go slice. Avoid unnecessary allocations when
encoding mvcc keys.
benchmark old ns/op new ns/op delta
BenchmarkMVCCScan1Version1Row 11322 10721 -5.31%
BenchmarkMVCCScan1Version10Rows 36566 27190 -25.64%
BenchmarkMVCCScan1Version100Rows 276241 217066 -21.42%
BenchmarkMVCCScan1Version1000Rows 2205243 1834826 -16.80%
BenchmarkMVCCGet1Version 3739 3153 -15.67%
benchmark old MB/s new MB/s speedup
BenchmarkMVCCScan1Version1Row 90.44 95.51 1.06x
BenchmarkMVCCScan1Version10Rows 280.04 376.60 1.34x
BenchmarkMVCCScan1Version100Rows 370.69 471.74 1.27x
BenchmarkMVCCScan1Version1000Rows 464.35 558.09 1.20x
BenchmarkMVCCGet1Version 273.83 324.77 1.19x