Bug#35531293 MySQL 8.0.33 is slower than MySQL 8.0.28 with queries

using JOINS

Description:
------------
  Some functions which were inline-ed in 8.0.28 were found to be
non-inline-ed in 8.0.33.

Fix:
----
  This commit makes the following functions inline which were
non-inlined in version 8.0.33:
    1. log_buffer_flush_to_disk(bool sync = true)
    2. log_get_checkpoint_age
    3. rec_get_nth_field_offs
    4. rec_get_nth_field
    5. rec_offs_nth_extern
    6. rec_init_offsets_comp_ordinary

Change-Id: I31c6a189d37c6c4cd92bd85cf2f184168cede59d

Author: ram1048

storage/innobase/btr/btr0btr.cc

@@ -600,8 +600,8 @@ static inline void btr_node_ptr_set_child_page_no(
   ut_ad(!rec_offs_comp(offsets) || rec_get_node_ptr_flag(rec));
 
   /* The child address is in the last field */
-  field = const_cast<byte *>(rec_get_nth_field(
-      nullptr, rec, offsets, rec_offs_n_fields(offsets) - 1, &len));
+  field = rec_get_nth_field(nullptr, rec, offsets,
+                            rec_offs_n_fields(offsets) - 1, &len);
 
   ut_ad(len == REC_NODE_PTR_SIZE);
 

storage/innobase/dict/dict0dd.cc

@@ -5526,7 +5526,7 @@ const char *dd_process_dd_tables_rec_and_mtr_commit(
   }
 
   /* Get the se_private_id field. */
-  field = (const byte *)rec_get_nth_field(
+  field = rec_get_nth_field(
       nullptr, rec, offsets,
       dd_object_table.field_number("FIELD_SE_PRIVATE_ID") + DD_FIELD_OFFSET,
       &len);
@@ -5598,10 +5598,11 @@ const char *dd_process_dd_partitions_rec_and_mtr_commit(
   }
 
   /* Get the se_private_id field. */
-  field = (const byte *)rec_get_nth_field(
+  field = rec_get_nth_field(
       nullptr, rec, offsets,
       dd_object_table.field_number("FIELD_SE_PRIVATE_ID") + DD_FIELD_OFFSET,
       &len);
+
   /* When table is partitioned table, the se_private_id is null. */
   if (len != 8) {
     *table = nullptr;
@@ -5663,7 +5664,7 @@ bool dd_process_dd_columns_rec(mem_heap_t *heap, const rec_t *rec,
   const dd::Object_table &dd_object_table = dd::get_dd_table<dd::Column>();
 
   /* Get the hidden attribute, and skip if it's a hidden column. */
-  field = (const byte *)rec_get_nth_field(
+  field = rec_get_nth_field(
       nullptr, rec, offsets,
       dd_object_table.field_number("FIELD_HIDDEN") + DD_FIELD_OFFSET, &len);
   hidden = static_cast<dd::Column::enum_hidden_type>(mach_read_from_1(field));
@@ -5674,24 +5675,24 @@ bool dd_process_dd_columns_rec(mem_heap_t *heap, const rec_t *rec,
   }
 
   /* Get the column name. */
-  field = (const byte *)rec_get_nth_field(
+  field = rec_get_nth_field(
       nullptr, rec, offsets,
       dd_object_table.field_number("FIELD_NAME") + DD_FIELD_OFFSET, &len);
   *col_name = mem_heap_strdupl(heap, (const char *)field, len);
 
   /* Get the position. */
-  field = (const byte *)rec_get_nth_field(
+  field = rec_get_nth_field(
       nullptr, rec, offsets,
       dd_object_table.field_number("FIELD_ORDINAL_POSITION") + DD_FIELD_OFFSET,
       &len);
   pos = mach_read_from_4(field) - 1;
 
   /* Get the is_virtual attribute. */
-  field = (const byte *)rec_get_nth_field(nullptr, rec, offsets, 21, &len);
+  field = rec_get_nth_field(nullptr, rec, offsets, 21, &len);
   is_virtual = mach_read_from_1(field) & 0x01;
 
   /* Get the se_private_data field. */
-  field = (const byte *)rec_get_nth_field(
+  field = rec_get_nth_field(
       nullptr, rec, offsets,
       dd_object_table.field_number("FIELD_SE_PRIVATE_DATA") + DD_FIELD_OFFSET,
       &len);
@@ -5817,7 +5818,7 @@ bool dd_process_dd_virtual_columns_rec(mem_heap_t *heap, const rec_t *rec,
   const dd::Object_table &dd_object_table = dd::get_dd_table<dd::Column>();
 
   /* Get the is_virtual attribute, and skip if it's not a virtual column. */
-  field = (const byte *)rec_get_nth_field(
+  field = rec_get_nth_field(
       nullptr, rec, offsets,
       dd_object_table.field_number("FIELD_IS_VIRTUAL") + DD_FIELD_OFFSET, &len);
   is_virtual = mach_read_from_1(field) & 0x01;
@@ -5827,7 +5828,7 @@ bool dd_process_dd_virtual_columns_rec(mem_heap_t *heap, const rec_t *rec,
   }
 
   /* Get the hidden attribute, and skip if it's a hidden column. */
-  field = (const byte *)rec_get_nth_field(
+  field = rec_get_nth_field(
       nullptr, rec, offsets,
       dd_object_table.field_number("FIELD_HIDDEN") + DD_FIELD_OFFSET, &len);
   hidden = static_cast<dd::Column::enum_hidden_type>(mach_read_from_1(field));
@@ -5837,14 +5838,14 @@ bool dd_process_dd_virtual_columns_rec(mem_heap_t *heap, const rec_t *rec,
   }
 
   /* Get the position. */
-  field = (const byte *)rec_get_nth_field(
+  field = rec_get_nth_field(
       nullptr, rec, offsets,
       dd_object_table.field_number("FIELD_ORDINAL_POSITION") + DD_FIELD_OFFSET,
       &len);
   origin_pos = mach_read_from_4(field) - 1;
 
   /* Get the se_private_data field. */
-  field = (const byte *)rec_get_nth_field(
+  field = rec_get_nth_field(
       nullptr, rec, offsets,
       dd_object_table.field_number("FIELD_SE_PRIVATE_DATA") + DD_FIELD_OFFSET,
       &len);
@@ -5951,7 +5952,7 @@ bool dd_process_dd_indexes_rec(mem_heap_t *heap, const rec_t *rec,
   }
 
   /* Get the se_private_data field. */
-  field = (const byte *)rec_get_nth_field(
+  field = rec_get_nth_field(
       nullptr, rec, offsets,
       dd_object_table.field_number("FIELD_SE_PRIVATE_DATA") + DD_FIELD_OFFSET,
       &len);
@@ -6101,7 +6102,7 @@ bool dd_process_dd_indexes_rec_simple(mem_heap_t *heap, const rec_t *rec,
   }
 
   /* Get the se_private_data field. */
-  field = (const byte *)rec_get_nth_field(
+  field = rec_get_nth_field(
       nullptr, rec, offsets,
       dd_object_table.field_number("FIELD_SE_PRIVATE_DATA") + DD_FIELD_OFFSET,
       &len);
@@ -6175,7 +6176,7 @@ bool dd_process_dd_tablespaces_rec(mem_heap_t *heap, const rec_t *rec,
   memcpy(*name, field, len);
 
   /* Get the options string. */
-  field = (const byte *)rec_get_nth_field(
+  field = rec_get_nth_field(
       nullptr, rec, offsets,
       dd_object_table.field_number("FIELD_OPTIONS") + DD_FIELD_OFFSET, &len);
 
@@ -6219,7 +6220,7 @@ bool dd_process_dd_tablespaces_rec(mem_heap_t *heap, const rec_t *rec,
   delete o;
 
   /* Get the se_private_data field. */
-  field = (const byte *)rec_get_nth_field(
+  field = rec_get_nth_field(
       nullptr, rec, offsets,
       dd_object_table.field_number("FIELD_SE_PRIVATE_DATA") + DD_FIELD_OFFSET,
       &len);

storage/innobase/include/lob0lob.h

@@ -733,8 +733,7 @@ class BtrContext {
     byte *data;
     ulint local_len;
 
-    data = const_cast<byte *>(
-        rec_get_nth_field(m_index, m_rec, m_offsets, i, &local_len));
+    data = rec_get_nth_field(m_index, m_rec, m_offsets, i, &local_len);
     ut_ad(rec_offs_nth_extern(m_index, m_offsets, i));
     ut_a(local_len >= BTR_EXTERN_FIELD_REF_SIZE);
 

storage/innobase/include/log0buf.h

@@ -193,7 +193,9 @@ void log_buffer_flush_to_disk(log_t &log, bool sync = true);
 
 /** Requests flush of the log buffer.
 @param[in]	sync	true: wait until the flush is done */
-void log_buffer_flush_to_disk(bool sync = true);
+inline void log_buffer_flush_to_disk(bool sync = true) {
+  log_buffer_flush_to_disk(*log_sys, sync);
+}
 
 /** Writes the log buffer to the log file. It is intended to be called from
 background master thread periodically. If the log writer threads are active,

storage/innobase/include/log0chkp.h

@@ -108,7 +108,24 @@ However we do the best effort to avoid such situations, and if
 they happen, user threads wait until the space is reclaimed.
 @param[in]	log	redo log
 @return checkpoint age as number of bytes */
-lsn_t log_get_checkpoint_age(const log_t &log);
+inline lsn_t log_get_checkpoint_age(const log_t &log) {
+  const lsn_t last_checkpoint_lsn = log.last_checkpoint_lsn.load();
+
+  const lsn_t current_lsn = log_get_lsn(log);
+
+  if (current_lsn <= last_checkpoint_lsn) {
+    /* Writes or reads have been somehow reordered.
+    Note that this function does not provide any lock,
+    and does not assume any lock existing. Therefore
+    the calculated result is already outdated when the
+    function is finished. Hence, we might assume that
+    this time we calculated age = 0, because checkpoint
+    lsn is close to current lsn if such race happened. */
+    return 0;
+  }
+
+  return current_lsn - last_checkpoint_lsn;
+}
 
 /** Provides opposite checkpoint header number to the given checkpoint
 header number.

storage/innobase/include/rem0lrec.h

@@ -131,43 +131,6 @@ static inline ulint rec_get_nth_field_size_low(const rec_t *rec, ulint n) {
   return (next_os - os);
 }
 
-/** Get an offset to the nth data field in a record.
-@param[in]   offsets  array returned by rec_get_offsets()
-@param[in]   n        index of the field
-@param[out]  len      length of the field; UNIV_SQL_NULL if SQL null;
-                      UNIV_SQL_ADD_COL_DEFAULT if it's default value and no
-                      value inlined
-@return offset from the origin of rec */
-static inline ulint rec_get_nth_field_offs_low(const ulint *offsets, ulint n,
-                                               ulint *len) {
-  ulint offs;
-  ulint length;
-  ut_ad(n < rec_offs_n_fields(offsets));
-  ut_ad(len);
-
-  if (n == 0) {
-    offs = 0;
-  } else {
-    offs = rec_offs_base(offsets)[n] & REC_OFFS_MASK;
-  }
-
-  length = rec_offs_base(offsets)[1 + n];
-
-  if (length & REC_OFFS_SQL_NULL) {
-    length = UNIV_SQL_NULL;
-  } else if (length & REC_OFFS_DEFAULT) {
-    length = UNIV_SQL_ADD_COL_DEFAULT;
-  } else if (length & REC_OFFS_DROP) {
-    length = UNIV_SQL_INSTANT_DROP_COL;
-  } else {
-    length &= REC_OFFS_MASK;
-    length -= offs;
-  }
-
-  *len = length;
-  return (offs);
-}
-
 /** The following function is used to get the offset to the nth
 data field in an old-style record.
 @param[in]   rec  record
@@ -216,14 +179,6 @@ static inline ulint rec_get_nth_field_offs_old_low(const rec_t *rec, ulint n,
   return (os);
 }
 
-/** Returns nonzero if the extern bit is set in nth field of rec.
-@param[in]  offsets  array returned by rec_get_offsets()
-@param[in]  n        index of the field
-@return nonzero if externally stored */
-static inline ulint rec_offs_nth_extern_low(const ulint *offsets, ulint n) {
-  return (rec_offs_base(offsets)[1 + n] & REC_OFFS_EXTERNAL);
-}
-
 /** Mark the nth field as externally stored.
 @param[in]  offsets  array returned by rec_get_offsets()
 @param[in]  n       index of the field */

storage/innobase/include/rem0wrec.h

@@ -40,16 +40,6 @@ this program; if not, write to the Free Software Foundation, Inc.,
 
 #include "rem/rec.h"
 
-/** Gets the value of the specified field in the record.
-@param[in]      index   record descriptor
-@param[in]      rec     physical record
-@param[in]      offsets array returned by rec_get_offsets()
-@param[in]      n       index of the field
-@param[out]     len     length of the field, UNIV_SQL_NULL if SQL null
-@return value of the field */
-byte *rec_get_nth_field(const dict_index_t *index, const rec_t *rec,
-                        const ulint *offsets, ulint n, ulint *len);
-
 const byte *rec_get_nth_field_old(const dict_index_t *index, const rec_t *rec,
                                   ulint n, ulint *len);
 
@@ -71,9 +61,68 @@ record.
 @param[out]     len     length of the field; UNIV_SQL_NULL if SQL null;
                         UNIV_SQL_ADD_COL_DEFAULT if it's default value and no
                         value inlined
+@note This long method is made inline because it is on performance sensitive hot
+path. One must run performance tests if they intend to improve this method.
 @return offset from the origin of rec */
-ulint rec_get_nth_field_offs(const dict_index_t *index, const ulint *offsets,
-                             ulint n, ulint *len);
+inline ulint rec_get_nth_field_offs(const dict_index_t *index,
+                                    const ulint *offsets, ulint n, ulint *len) {
+  if (index && index->has_row_versions()) {
+    n = index->get_field_off_pos(n);
+  }
+
+  ulint offs;
+  ulint length;
+  ut_ad(n < rec_offs_n_fields(offsets));
+  ut_ad(len);
+
+  if (n == 0) {
+    offs = 0;
+  } else {
+    offs = rec_offs_base(offsets)[n] & REC_OFFS_MASK;
+  }
+
+  length = rec_offs_base(offsets)[1 + n];
+
+  if (length & REC_OFFS_SQL_NULL) {
+    length = UNIV_SQL_NULL;
+  } else if (length & REC_OFFS_DEFAULT) {
+    length = UNIV_SQL_ADD_COL_DEFAULT;
+  } else if (length & REC_OFFS_DROP) {
+    length = UNIV_SQL_INSTANT_DROP_COL;
+  } else {
+    length &= REC_OFFS_MASK;
+    length -= offs;
+  }
+
+  *len = length;
+  return (offs);
+}
+
+/** Gets the value of the specified field in the record.
+@param[in]      index   record descriptor
+@param[in]      rec     physical record
+@param[in]      offsets array returned by rec_get_offsets()
+@param[in]      n       index of the field
+@param[out]     len     length of the field, UNIV_SQL_NULL if SQL null
+@return value of the field */
+inline const byte *rec_get_nth_field(const dict_index_t *index,
+                                     const rec_t *rec, const ulint *offsets,
+                                     ulint n, ulint *len) {
+  return rec + rec_get_nth_field_offs(index, offsets, n, len);
+}
+
+/** Gets the value of the specified field in the record.
+@param[in]      index   record descriptor
+@param[in]      rec     physical record
+@param[in]      offsets array returned by rec_get_offsets()
+@param[in]      n       index of the field
+@param[out]     len     length of the field, UNIV_SQL_NULL if SQL null
+@return value of the field */
+inline byte *rec_get_nth_field(const dict_index_t *index, rec_t *rec,
+                               const ulint *offsets, ulint n, ulint *len) {
+  return const_cast<byte *>(rec_get_nth_field(
+      index, const_cast<const rec_t *>(rec), offsets, n, len));
+}
 
 /** The following function is used to get the offset to the nth
 data field in an old-style record.
@@ -85,13 +134,29 @@ data field in an old-style record.
 ulint rec_get_nth_field_offs_old(const dict_index_t *index, const rec_t *rec,
                                  ulint n, ulint *len);
 
+/** Validates offset and field number.
+@param[in]      index   record descriptor
+@param[in]      offsets array returned by rec_get_offsets()
+@param[in]      n       nth field
+@param[in]      L       Line number of calling satement*/
+void validate_rec_offset(const dict_index_t *index, const ulint *offsets,
+                         ulint n, ut::Location L);
+
 /** Returns nonzero if the extern bit is set in nth field of rec.
 @param[in]      index           record descriptor
 @param[in]      offsets         array returned by rec_get_offsets()
 @param[in]      n               nth field
 @return nonzero if externally stored */
-[[nodiscard]] ulint rec_offs_nth_extern(const dict_index_t *index,
-                                        const ulint *offsets, ulint n);
+[[nodiscard]] inline ulint rec_offs_nth_extern(const dict_index_t *index,
+                                               const ulint *offsets, ulint n) {
+  if (index && index->has_row_versions()) {
+    n = index->get_field_off_pos(n);
+  }
+
+  validate_rec_offset(index, offsets, n, UT_LOCATION_HERE);
+  /* Returns nonzero if the extern bit is set in nth field of rec. */
+  return rec_offs_base(offsets)[1 + n] & REC_OFFS_EXTERNAL;
+}
 
 /** Mark the nth field as externally stored.
 @param[in]      index           record descriptor

storage/innobase/lob/lob0lob.cc

@@ -1288,7 +1288,7 @@ bool rec_check_lobref_space_id(dict_index_t *index, const rec_t *rec,
       continue;
     }
 
-    byte *data = rec_get_nth_field(index, rec, offsets, i, &len);
+    const byte *data = rec_get_nth_field(index, rec, offsets, i, &len);
 
     if (len == UNIV_SQL_NULL) {
       continue;
@@ -1298,9 +1298,8 @@ bool rec_check_lobref_space_id(dict_index_t *index, const rec_t *rec,
       ulint local_len = len - BTR_EXTERN_FIELD_REF_SIZE;
       ut_ad(len >= BTR_EXTERN_FIELD_REF_SIZE);
 
-      byte *field_ref = data + local_len;
-      ref_t ref(field_ref);
-      if (!ref.check_space_id(index)) {
+      const byte *field_ref = data + local_len;
+      if (!ref_t{const_cast<byte *>(field_ref)}.check_space_id(index)) {
         return (false);
       }
     }

storage/innobase/log/log0buf.cc

@@ -1201,10 +1201,6 @@ void log_buffer_flush_to_disk(log_t &log, bool sync) {
   log_write_up_to(log, lsn, sync);
 }
 
-void log_buffer_flush_to_disk(bool sync) {
-  log_buffer_flush_to_disk(*log_sys, sync);
-}
-
 void log_buffer_sync_in_background() {
   log_t &log = *log_sys;