storage/reader: Avoid some copy in mvcc reader

[MyonKeminta]

Signed-off-by: MyonKeminta MyonKeminta@users.noreply.github.com

What have you changed? (mandatory)

This PR tries to avoid some clones of keys in mvcc reader.

What are the type of the changes? (mandatory)

• Improvement (change which is an improvement to an existing feature)

How has this PR been tested? (mandatory)

By unit tests

Does this PR affect documentation (docs) update? (mandatory)

No

Does this PR affect tidb-ansible update? (mandatory)

No

[MyonKeminta]

@breeswish @Connor1996 PTAL!

[Connor1996]

LGTM

[breezewish]

@MyonKeminta Do you have some interests to work on provide references (instead of owned values) in Mvcc Scanners? This can further eliminate copies. According to my benchmarks there can be 7% improvements in the integration benchmark (using a MySQL client)

[MyonKeminta]

@breeswish Of course! But I think I need some time to think about how to do that 🤔

[breezewish]

@MyonKeminta I once had an experiment over this. The code is pretty awful but the outcome is great. The main challenge (of writing elegant code) is that currently we use Key structure, which enforces type safety of encoded values or raw values, which is great, but itself only support owned values. There is no way to enforce type safety by only providing references. For example, our Cursor::seek and other friends accept &Key, which requires an owned Vec<..> being built (and then pass it as a reference). This fundamentally make it impossible to be zero copy (because what we receive from RocksDB is just a &[u8] ).

[breezewish]

Previous dirty attempt: breezewish@96afbb4

[siddontang]

Thanks @morefreeze
Have you done some bechmarks about this?

[morefreeze]

Sorry, why this involves me? I have no idea about these changes. Is that a misspell?

[breezewish]

@morefreeze Yes it should be a misspell, nevermind :)

[MyonKeminta]

@siddontang Not yet. I didn't expect this PR to significantly improve the performance, but I just thought that it's never bad to remove the unnecessary copies.
@breeswish Thank you! I'll check it later. By the way, I'm considering whether we can let the Key able to represent both owned value and references by using unsafe code... 🤔

[breezewish]

@MyonKeminta How about just KeyRef for references and Key for owned values? (I don't know. just a brain storm)

[MyonKeminta]

@breeswish I don't know either before a try 😂