Post-push fix for Bug#36302624 performance_schema.data_lock_waits is generated with O(N^3) complexity

Problem:

On 32-bit systems alignof(uint64_t) might be 4.
This in turn permits alignof(lock_t) == 4 and sizeof(lock_t)%8 == 4.
Taken together, this means that the bitmap which is immediately after
the lock_t struct can have an address which is not a multiple of 8.
This is suboptimal as Bitmap::find_set(..) handles such case by entering
a slow path. There is a debug-only assert which checks that the address
of bitmap is a multiple of 8 to ensure high performance.
This assertion is failing on 32-bit builds.

Solution:

Added alignas(8) to definition of lock_t to enforce proper alignment.
Added a static assert that alignof(lock_t) is at least 8, to prevent
regression.
Added a static assert that heap allocator respects alignof(lock_t).
Added an assert to dict_mem_table_create that the lock_t allocated there
for autoinc is also properly aligned.
As sizeof(T) must be at least alignof(T), this also ensures that
sizeof(lock_t) is a multiple of 8.

Note: the instances of lock_t are obtained in various ways:

a) By calling ut::malloc_withkey(...,REC_LOCK_SIZE * REC_LOCK_CACHE) to
allocate one big chunk, and then slicing it into REC_LOCK_CACHE items
of REC_LOCK_SIZE bytes. This is fine, because allocator ensures
alignof(std::max_align_t) which must be at least that of double. So, the
address of the big buffer is at least divisible by 8. And because
REC_LOCK_SIZE is defined as sizeof(ib_lock_t) + 256, so as long as
alignof(lock_t) is at least 8, then adding 256 to it will not spoil the
property that each smaller buffer has an address divisible by 8.

b) Similar to a) but with TABLE_LOCK_SIZE and TABLE_LOCK_CACHE. Here the
difference is that TABLE_LOCK_SIZE doesn't include 256 bytes of padding
for the bitmap, which makes the proof even simpler. Also we don't ever
use bitmaps for table locks, so this shouldn't matter in practice.

c) Calls to mem_heap_alloc in RecLock::lock_alloc, lock_table_create and
dict_mem_table_create. Each of them should give an address which has at
least UNIV_MEM_ALIGNMENT==8, so should be fine. There are asserts in
each of these places (this patch adds the missing one) that the
allocated lock object has at least alignof(lock_t).

This is why we expect all the lock_t structs to have at least
alignof(lock_t) and why it is sufficient to ensure alignof(lock_t) is at
least 8.

Change-Id: I7d4892d28baefd11aa1c0fa4ac2dba502f270bd2

Author: Jakub Łopuszański

storage/innobase/dict/mem.cc

@@ -233,8 +233,7 @@ dict_table_t *dict_mem_table_create(const char *name, space_id_t space,
   dict_table_stats_lock() will not be noop. */
   dict_table_stats_latch_create(table, true);
 
-  table->autoinc_lock =
-      static_cast<ib_lock_t *>(mem_heap_alloc(heap, lock_get_size()));
+  table->autoinc_lock = lock_alloc_from_heap(heap);
 
   /* lazy creation of table autoinc latch */
   dict_table_autoinc_create_lazy(table);

storage/innobase/include/lock0lock.h

@@ -248,9 +248,19 @@ class ReadView;
 
 extern bool innobase_deadlock_detect;
 
-/** Gets the size of a lock struct.
- @return size in bytes */
-ulint lock_get_size(void);
+/** Allocates memory suitable for holding a lock_t from specified heap.
+The allocated memory has additional bitmap_bytes right after the returned
+lock_t instance for holding the bitmap used by LOCK_REC type.
+@param[in]     heap
+                 The heap to allocate the memory from
+@param[in]     bitmap_bytes
+                 The number of bytes to reserve right after the lock_t struct
+                 for the bitmap. Defaults to 0, which is ok for LOCK_TABLE.
+@return A pointer to the memory allocated from the heap, aligned as lock_t,
+and of size sizeof(lock_t)+bitmap_bytes. Note that it can contain garbage,
+so it is caller's responsibility to initialize lock_t and the bitmap. */
+lock_t *lock_alloc_from_heap(mem_heap_t *heap, size_t bitmap_bytes = 0);
+
 /** Creates the lock system at database start. */
 void lock_sys_create(
     ulint n_cells); /*!< in: number of slots in lock hash table */

storage/innobase/include/lock0priv.h

@@ -134,7 +134,7 @@ static inline bool lock_mode_is_next_key_lock(ulint mode) {
 static inline bool lock_rec_get_nth_bit(const lock_t *lock, ulint i);
 
 /** Lock struct; protected by lock_sys latches */
-struct lock_t {
+struct alignas(8 /* For efficient Bitmap::find_set */) lock_t {
   /** transaction owning the lock */
   trx_t *trx;
 
@@ -256,7 +256,9 @@ struct lock_t {
   /** @overload */
   Bitset<const byte> bitset() const {
     ut_ad(is_record_lock());
-    static_assert(alignof(uint64_t) <= alignof(lock_t),
+    /* On a 32-bit system alignof(uint64_t) might be 4. Still, the
+    Bitmap::find_set goes into slow path if address is not a multiple of 8. */
+    static_assert(8 <= alignof(lock_t),
                   "lock_t and thus the bitmap after lock_t should be aligned "
                   "for efficient 64-bit access");
     const byte *bitmap = (const byte *)&this[1];

storage/innobase/lock/lock0lock.cc

@@ -474,10 +474,6 @@ void lock_sys_close(void) {
   lock_sys = nullptr;
 }
 
-/** Gets the size of a lock struct.
- @return size in bytes */
-ulint lock_get_size(void) { return ((ulint)sizeof(lock_t)); }
-
 bool lock_is_waiting(const lock_t &lock) {
   ut_ad(locksys::owns_lock_shard(&lock));
   return lock.is_waiting();
@@ -1108,6 +1104,15 @@ ulint lock_number_of_tables_locked(const trx_t *trx) {
   return count;
 }
 
+lock_t *lock_alloc_from_heap(mem_heap_t *heap, size_t bitmap_bytes) {
+  const size_t n_bytes = sizeof(lock_t) + bitmap_bytes;
+  static_assert(alignof(lock_t) <= UNIV_MEM_ALIGNMENT,
+                "heap allocator must ensure lock_t is properly aligned");
+  auto ptr = mem_heap_alloc(heap, n_bytes);
+  ut_a(ut::is_aligned_as<lock_t>(ptr));
+  return reinterpret_cast<lock_t *>(ptr);
+}
+
 /*============== RECORD LOCK CREATION AND QUEUE MANAGEMENT =============*/
 
 /**
@@ -1158,11 +1163,7 @@ lock_t *RecLock::lock_alloc(trx_t *trx, dict_index_t *index, ulint mode,
 
   if (trx->lock.rec_cached >= trx->lock.rec_pool.size() ||
       sizeof(*lock) + size > REC_LOCK_SIZE) {
-    ulint n_bytes = size + sizeof(*lock);
-    mem_heap_t *heap = trx->lock.lock_heap;
-    auto ptr = mem_heap_alloc(heap, n_bytes);
-    ut_a(ut::is_aligned_as<lock_t>(ptr));
-    lock = reinterpret_cast<lock_t *>(ptr);
+    lock = lock_alloc_from_heap(trx->lock.lock_heap, size);
   } else {
     lock = trx->lock.rec_pool[trx->lock.rec_cached];
     ++trx->lock.rec_cached;
@@ -3293,9 +3294,7 @@ static inline lock_t *lock_table_create(
   } else if (trx->lock.table_cached < trx->lock.table_pool.size()) {
     lock = trx->lock.table_pool[trx->lock.table_cached++];
   } else {
-    auto ptr = mem_heap_alloc(trx->lock.lock_heap, sizeof(*lock));
-    ut_a(ut::is_aligned_as<lock_t>(ptr));
-    lock = static_cast<lock_t *>(ptr);
+    lock = lock_alloc_from_heap(trx->lock.lock_heap);
   }
   lock->type_mode = uint32_t(type_mode | LOCK_TABLE);
   lock->trx = trx;