Skip to content

Commit 311519b

Browse files
yizhang82inikep
authored andcommitted
2x integer key decoding perf
Summary: In my testing this reduces the overhead of integer decoding by 2x. The idea of the fix is straight-forward: * Don't call Field* to get information, instead cache as much information as possible in our Rdb_field_packing, similar to InnoDB's row_prebuilt_t. This has a few advantages: * Avoid expensive virtual function calls * More cache friendly * Similarly, don't use field->move_field / move_field_offset to move field to the target buffer and then back - just write to the buffer using pre-calculated field offset directly * Use template integer unpacking function to enable compiler to unroll expensive loops into a constant number of mov instructions. NOTE: FDO data needs to be updated for this diff in order to take full advantage of this change. Reference Patch: facebook@9af9aa5 --- Porting Notes: There are some additional changes required to account for covering TEXT/BLOB changes, especially for blob I need to be able to call into ha_rocksdb->get_blob_buffer. I've added a new struct Rdb_unpack_func_context to be able to pass any additional required arguments without having to update all the unpack functions. For now I'm only passing in the table that we can use to get the ha_rocksdb handler. Reviewed By: Pushapgl Differential Revision: D25800694
1 parent 99062b1 commit 311519b

File tree

5 files changed

+232
-294
lines changed

5 files changed

+232
-294
lines changed

sql/field.cc

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6951,8 +6951,7 @@ Field_blob::Field_blob(uchar *ptr_arg, uchar *null_ptr_arg, uchar null_bit_arg,
69516951
/* TODO: why do not fill table->s->blob_field array here? */
69526952
}
69536953

6954-
static void store_blob_length(uchar *i_ptr, uint i_packlength,
6955-
uint32 i_number) {
6954+
void store_blob_length(uchar *i_ptr, uint i_packlength, uint32 i_number) {
69566955
switch (i_packlength) {
69576956
case 1:
69586957
assert(i_number <= 0xff);
@@ -6971,7 +6970,7 @@ static void store_blob_length(uchar *i_ptr, uint i_packlength,
69716970
}
69726971
}
69736972

6974-
uint32 Field_blob::get_length(const uchar *pos, uint packlength_arg) const {
6973+
uint32 Field_blob::get_length(const uchar *pos, uint packlength_arg) {
69756974
switch (packlength_arg) {
69766975
case 1:
69776976
return (uint32)pos[0];

sql/field.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3708,7 +3708,7 @@ class Field_blob : public Field_longstr {
37083708
return get_length(row_offset);
37093709
}
37103710
uint32 get_length(ptrdiff_t row_offset = 0) const;
3711-
uint32 get_length(const uchar *ptr, uint packlength) const;
3711+
static uint32 get_length(const uchar *ptr, uint packlength);
37123712
uint32 get_length(const uchar *ptr_arg) const;
37133713
/** Get a const pointer to the BLOB data of this field. */
37143714
const uchar *get_blob_data() const { return get_blob_data(ptr + packlength); }
@@ -3871,6 +3871,8 @@ class Field_blob : public Field_longstr {
38713871
int do_save_field_metadata(uchar *first_byte) const override;
38723872
};
38733873

3874+
void store_blob_length(uchar *i_ptr, uint i_packlength, uint32 i_number);
3875+
38743876
class Field_geom final : public Field_blob {
38753877
private:
38763878
const std::optional<gis::srid_t> m_srid;

storage/rocksdb/rdb_converter.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -817,7 +817,7 @@ int Rdb_converter::convert_record_from_storage_format(
817817
return err;
818818
}
819819

820-
if (!decode_value) {
820+
if (!decode_value || get_decode_fields()->size() == 0) {
821821
// We are done
822822
return HA_EXIT_SUCCESS;
823823
}

0 commit comments

Comments
 (0)