MDEV-24167 fixup: Improve the PERFORMANCE_SCHEMA instrumentation

Let us try to avoid code bloat for the common case that
performance_schema is disabled at runtime, and use
ATTRIBUTE_NOINLINE member functions for instrumented latch acquisition.

Also, let us distinguish lock waits from non-contended lock requests
by using write_lock,read_lock for the requests that lead to waits,
and try_write_lock,try_read_lock for the wait-free lock acquisitions.
Actual 'try' operations are not being instrumented at all.

mysql-test/suite/perfschema/r/sxlock_func.result

 UPDATE performance_schema.setup_instruments SET enabled = 'NO', timed = 'YES';
 UPDATE performance_schema.setup_instruments SET enabled = 'YES'
 WHERE name like 'wait/synch/sxlock/%';
+UPDATE performance_schema.setup_instruments SET enabled = 'YES'
+WHERE name like 'wait/synch/rwlock/innodb/%';
 SELECT DISTINCT name FROM performance_schema.setup_instruments
-WHERE name LIKE 'wait/synch/rwlock/innodb%'
+WHERE name LIKE 'wait/synch/rwlock/innodb/%'
 AND name!='wait/synch/rwlock/innodb/btr_search_latch' ORDER BY name;
 name
 wait/synch/rwlock/innodb/dict_operation_lock
@@ -28,6 +30,14 @@ insert into t1 values (1), (2), (3);
 insert into t1 values (1), (2), (3);
 commit;
 drop table t1;
+SELECT DISTINCT event_name FROM performance_schema.events_waits_history_long
+WHERE event_name LIKE 'wait/synch/rwlock/innodb/%'
+AND event_name!='wait/synch/rwlock/innodb/btr_search_latch'
+ORDER BY event_name;
+event_name
+wait/synch/rwlock/innodb/dict_operation_lock
+wait/synch/rwlock/innodb/fil_space_latch
+wait/synch/rwlock/innodb/trx_purge_latch
 select operation from performance_schema.events_waits_history_long
 where event_name like "wait/synch/sxlock/%"
   and operation = "shared_lock" limit 1;

mysql-test/suite/perfschema/t/sxlock_func.test

@@ -11,8 +11,11 @@ UPDATE performance_schema.setup_instruments SET enabled = 'NO', timed = 'YES';
 UPDATE performance_schema.setup_instruments SET enabled = 'YES'
 WHERE name like 'wait/synch/sxlock/%';
 
+UPDATE performance_schema.setup_instruments SET enabled = 'YES'
+WHERE name like 'wait/synch/rwlock/innodb/%';
+
 SELECT DISTINCT name FROM performance_schema.setup_instruments
-WHERE name LIKE 'wait/synch/rwlock/innodb%'
+WHERE name LIKE 'wait/synch/rwlock/innodb/%'
 AND name!='wait/synch/rwlock/innodb/btr_search_latch' ORDER BY name;
 
 TRUNCATE TABLE performance_schema.events_waits_history_long;
@@ -38,6 +41,13 @@ insert into t1 values (1), (2), (3);
 commit;
 drop table t1;
 
+# Make sure some rw-lock operations have been executed
+
+SELECT DISTINCT event_name FROM performance_schema.events_waits_history_long
+WHERE event_name LIKE 'wait/synch/rwlock/innodb/%'
+AND event_name!='wait/synch/rwlock/innodb/btr_search_latch'
+ORDER BY event_name;
+
 # Make sure some shared_lock operations have been executed
 
 select operation from performance_schema.events_waits_history_long
@@ -63,6 +73,4 @@ select operation from performance_schema.events_waits_history_long
 
 # Cleanup
 
-
 UPDATE performance_schema.setup_instruments SET enabled = 'YES', timed = 'YES';
-

storage/innobase/btr/btr0cur.cc

@@ -2501,7 +2501,7 @@ btr_cur_search_to_nth_level_func(
 
 #ifdef BTR_CUR_HASH_ADAPT
 	if (ahi_latch) {
-		ahi_latch->rd_lock();
+		ahi_latch->rd_lock(SRW_LOCK_CALL);
 	}
 #endif /* BTR_CUR_HASH_ADAPT */
 
@@ -4315,7 +4315,7 @@ btr_cur_update_in_place(
 				btr_search_update_hash_on_delete(cursor);
 			}
 
-			ahi_latch->wr_lock();
+			ahi_latch->wr_lock(SRW_LOCK_CALL);
 		}
 
 		assert_block_ahi_valid(block);

storage/innobase/btr/btr0sea.cc

@@ -181,7 +181,7 @@ static void btr_search_check_free_space_in_heap(const dict_index_t *index)
   buf_block_t *block= buf_block_alloc();
   auto part= btr_search_sys.get_part(*index);
 
-  part->latch.wr_lock();
+  part->latch.wr_lock(SRW_LOCK_CALL);
 
   if (!btr_search_enabled || part->heap->free_block)
     buf_block_free(block);
@@ -711,7 +711,7 @@ btr_search_update_hash_ref(
 	ut_ad(index == cursor->index);
 	ut_ad(!dict_index_is_ibuf(index));
 	auto part = btr_search_sys.get_part(*index);
-	part->latch.wr_lock();
+	part->latch.wr_lock(SRW_LOCK_CALL);
 	ut_ad(!block->index || block->index == index);
 
 	if (block->index
@@ -1054,7 +1054,7 @@ btr_search_guess_on_hash(
 	const rec_t* rec;
 
 	if (!ahi_latch) {
-		part->latch.rd_lock();
+		part->latch.rd_lock(SRW_LOCK_CALL);
 
 		if (!btr_search_enabled) {
 			goto fail;
@@ -1266,7 +1266,7 @@ void btr_search_drop_page_hash_index(buf_block_t* block)
 	auto part = btr_search_sys.get_part(index_id,
 					    block->page.id().space());
 
-	part->latch.rd_lock();
+	part->latch.rd_lock(SRW_LOCK_CALL);
 	assert_block_ahi_valid(block);
 
 	if (!block->index || !btr_search_enabled) {
@@ -1341,7 +1341,7 @@ void btr_search_drop_page_hash_index(buf_block_t* block)
 		mem_heap_free(heap);
 	}
 
-	part->latch.wr_lock();
+	part->latch.wr_lock(SRW_LOCK_CALL);
 
 	if (UNIV_UNLIKELY(!block->index)) {
 		/* Someone else has meanwhile dropped the hash index */
@@ -1482,7 +1482,7 @@ btr_search_build_page_hash_index(
 				  RW_LOCK_FLAG_X | RW_LOCK_FLAG_S));
 	ut_ad(block->page.id().page_no() >= 3);
 
-	ahi_latch->rd_lock();
+	ahi_latch->rd_lock(SRW_LOCK_CALL);
 
 	const bool enabled = btr_search_enabled;
 	const bool rebuild = enabled && block->index
@@ -1596,7 +1596,7 @@ btr_search_build_page_hash_index(
 
 	btr_search_check_free_space_in_heap(index);
 
-	ahi_latch->wr_lock();
+	ahi_latch->wr_lock(SRW_LOCK_CALL);
 
 	if (!btr_search_enabled) {
 		goto exit_func;
@@ -1730,7 +1730,7 @@ btr_search_move_or_delete_hash_entries(
 		return;
 	}
 
-	ahi_latch->rd_lock();
+	ahi_latch->rd_lock(SRW_LOCK_CALL);
 
 	if (block->index) {
 		uint16_t n_fields = block->curr_n_fields;
@@ -1807,7 +1807,7 @@ void btr_search_update_hash_on_delete(btr_cur_t* cursor)
 
 	auto part = btr_search_sys.get_part(*index);
 
-	part->latch.wr_lock();
+	part->latch.wr_lock(SRW_LOCK_CALL);
 	assert_block_ahi_valid(block);
 
 	if (block->index && btr_search_enabled) {
@@ -1861,7 +1861,7 @@ void btr_search_update_hash_node_on_insert(btr_cur_t *cursor,
 
 	ut_a(cursor->index == index);
 	ut_ad(!dict_index_is_ibuf(index));
-	ahi_latch->wr_lock();
+	ahi_latch->wr_lock(SRW_LOCK_CALL);
 
 	if (!block->index || !btr_search_enabled) {
 
@@ -1979,7 +1979,7 @@ void btr_search_update_hash_on_insert(btr_cur_t *cursor,
 	} else {
 		if (left_side) {
 			locked = true;
-			ahi_latch->wr_lock();
+			ahi_latch->wr_lock(SRW_LOCK_CALL);
 
 			if (!btr_search_enabled || !block->index) {
 				goto function_exit;
@@ -1998,7 +1998,7 @@ void btr_search_update_hash_on_insert(btr_cur_t *cursor,
 
 		if (!locked) {
 			locked = true;
-			ahi_latch->wr_lock();
+			ahi_latch->wr_lock(SRW_LOCK_CALL);
 
 			if (!btr_search_enabled || !block->index) {
 				goto function_exit;
@@ -2023,7 +2023,7 @@ void btr_search_update_hash_on_insert(btr_cur_t *cursor,
 		if (!left_side) {
 			if (!locked) {
 				locked = true;
-				ahi_latch->wr_lock();
+				ahi_latch->wr_lock(SRW_LOCK_CALL);
 
 				if (!btr_search_enabled || !block->index) {
 					goto function_exit;
@@ -2043,7 +2043,7 @@ void btr_search_update_hash_on_insert(btr_cur_t *cursor,
 	if (ins_fold != next_fold) {
 		if (!locked) {
 			locked = true;
-			ahi_latch->wr_lock();
+			ahi_latch->wr_lock(SRW_LOCK_CALL);
 
 			if (!btr_search_enabled || !block->index) {
 				goto function_exit;

storage/innobase/include/btr0sea.h

@@ -342,7 +342,7 @@ inline ulint dict_index_t::n_ahi_pages() const
   if (!btr_search_enabled)
     return 0;
   srw_lock *latch= &btr_search_sys.get_part(*this)->latch;
-  latch->rd_lock();
+  latch->rd_lock(SRW_LOCK_CALL);
   ulint ref_count= search_info->ref_count;
   latch->rd_unlock();
   return ref_count;

storage/innobase/include/btr0sea.ic

@@ -85,7 +85,7 @@ btr_search_info_update(
 static inline void btr_search_x_lock_all()
 {
 	for (ulint i = 0; i < btr_ahi_parts; ++i) {
-		btr_search_sys.parts[i].latch.wr_lock();
+		btr_search_sys.parts[i].latch.wr_lock(SRW_LOCK_CALL);
 	}
 }
 
@@ -101,7 +101,7 @@ static inline void btr_search_x_unlock_all()
 static inline void btr_search_s_lock_all()
 {
 	for (ulint i = 0; i < btr_ahi_parts; ++i) {
-		btr_search_sys.parts[i].latch.rd_lock();
+		btr_search_sys.parts[i].latch.rd_lock(SRW_LOCK_CALL);
 	}
 }
 

storage/innobase/include/dict0dict.h

@@ -1562,7 +1562,7 @@ class dict_sys_t
   /** Lock the data dictionary cache. */
   void lock(const char* file, unsigned line)
   {
-    latch.wr_lock();
+    latch.wr_lock(SRW_LOCK_ARGS(file, line));
     ut_ad(!latch_ex);
     ut_d(latch_ex= true);
     mutex_enter_loc(&mutex, file, line);
@@ -1578,7 +1578,7 @@ class dict_sys_t
   }
 
   /** Prevent modifications of the data dictionary */
-  void freeze() { latch.rd_lock(); ut_ad(!latch_ex); }
+  void freeze() { latch.rd_lock(SRW_LOCK_CALL); ut_ad(!latch_ex); }
   /** Allow modifications of the data dictionary */
   void unfreeze() { ut_ad(!latch_ex); latch.rd_unlock(); }
 

storage/innobase/include/fil0fil.h

@@ -1000,7 +1000,7 @@ struct fil_space_t final
   /** Acquire the allocation latch in exclusive mode */
   void x_lock()
   {
-    latch.wr_lock();
+    latch.wr_lock(SRW_LOCK_CALL);
     ut_ad(!latch_owner);
     ut_d(latch_owner= os_thread_get_curr_id());
     ut_ad(!latch_count.fetch_add(1));
@@ -1016,7 +1016,7 @@ struct fil_space_t final
   /** Acquire the allocation latch in shared mode */
   void s_lock()
   {
-    latch.rd_lock();
+    latch.rd_lock(SRW_LOCK_CALL);
     ut_ad(!latch_owner);
     ut_d(latch_count.fetch_add(1));
   }

storage/innobase/include/srw_lock.h

@@ -73,9 +73,13 @@ class srw_lock_low final : private rw_lock
 
 #ifndef UNIV_PFS_RWLOCK
 # define SRW_LOCK_INIT(key) init()
+# define SRW_LOCK_ARGS(file, line) /* nothing */
+# define SRW_LOCK_CALL /* nothing */
 typedef srw_lock_low srw_lock;
 #else
 # define SRW_LOCK_INIT(key) init(key)
+# define SRW_LOCK_ARGS(file, line) file, line
+# define SRW_LOCK_CALL __FILE__, __LINE__
 
 /** Slim reader-writer lock with PERFORMANCE_SCHEMA instrumentation */
 class srw_lock
@@ -83,6 +87,8 @@ class srw_lock
   srw_lock_low lock;
   PSI_rwlock *pfs_psi;
 
+  ATTRIBUTE_NOINLINE void psi_rd_lock(const char *file, unsigned line);
+  ATTRIBUTE_NOINLINE void psi_wr_lock(const char *file, unsigned line);
 public:
   void init(mysql_pfs_key_t key)
   {
@@ -98,22 +104,12 @@ class srw_lock
     }
     lock.destroy();
   }
-  void rd_lock()
+  void rd_lock(const char *file, unsigned line)
   {
-    uint32_t l;
-    if (lock.read_trylock(l))
-      return;
-    if (pfs_psi)
-    {
-      PSI_rwlock_locker_state state;
-      PSI_rwlock_locker *locker= PSI_RWLOCK_CALL(start_rwlock_rdwait)
-        (&state, pfs_psi, PSI_RWLOCK_READLOCK, __FILE__, __LINE__);
-      lock.read_lock(l);
-      if (locker)
-        PSI_RWLOCK_CALL(end_rwlock_rdwait)(locker, 0);
-      return;
-    }
-    lock.read_lock(l);
+    if (psi_likely(pfs_psi != nullptr))
+      psi_rd_lock(file, line);
+    else
+      lock.rd_lock();
   }
   void rd_unlock()
   {
@@ -121,21 +117,12 @@ class srw_lock
       PSI_RWLOCK_CALL(unlock_rwlock)(pfs_psi);
     lock.rd_unlock();
   }
-  void wr_lock()
+  void wr_lock(const char *file, unsigned line)
   {
-    if (lock.write_trylock())
-      return;
-    if (pfs_psi)
-    {
-      PSI_rwlock_locker_state state;
-      PSI_rwlock_locker *locker= PSI_RWLOCK_CALL(start_rwlock_wrwait)
-        (&state, pfs_psi, PSI_RWLOCK_WRITELOCK, __FILE__, __LINE__);
-      lock.write_lock();
-      if (locker)
-        PSI_RWLOCK_CALL(end_rwlock_rdwait)(locker, 0);
-      return;
-    }
-    lock.write_lock();
+    if (psi_likely(pfs_psi != nullptr))
+      psi_wr_lock(file, line);
+    else
+      lock.wr_lock();
   }
   void wr_unlock()
   {

storage/innobase/include/trx0purge.h

@@ -259,7 +259,7 @@ class purge_sys_t
   /** A wrapper around trx_sys_t::clone_oldest_view(). */
   void clone_oldest_view()
   {
-    latch.wr_lock();
+    latch.wr_lock(SRW_LOCK_CALL);
     trx_sys.clone_oldest_view(&view);
     latch.wr_unlock();
   }

storage/innobase/row/row0sel.cc

@@ -3858,7 +3858,7 @@ row_sel_try_search_shortcut_for_mysql(
 	ut_ad(!prebuilt->templ_contains_blob);
 
 	srw_lock* ahi_latch = btr_search_sys.get_latch(*index);
-	ahi_latch->rd_lock();
+	ahi_latch->rd_lock(SRW_LOCK_CALL);
 	btr_pcur_open_with_no_init(index, search_tuple, PAGE_CUR_GE,
 				   BTR_SEARCH_LEAF, pcur, ahi_latch, mtr);
 	rec = btr_pcur_get_rec(pcur);

storage/innobase/row/row0umod.cc

@@ -375,7 +375,7 @@ row_undo_mod_clust(
 		} else {
 			index->set_modified(mtr);
 			have_latch = true;
-			purge_sys.latch.rd_lock();
+			purge_sys.latch.rd_lock(SRW_LOCK_CALL);
 			if (!row_undo_mod_must_purge(node, &mtr)) {
 				goto mtr_commit_exit;
 			}
@@ -402,7 +402,7 @@ row_undo_mod_clust(
 			mtr.set_log_mode(MTR_LOG_NO_REDO);
 		} else {
 			have_latch = true;
-			purge_sys.latch.rd_lock();
+			purge_sys.latch.rd_lock(SRW_LOCK_CALL);
 			if (!row_undo_mod_must_purge(node, &mtr)) {
 				goto mtr_commit_exit;
 			}
@@ -429,7 +429,7 @@ row_undo_mod_clust(
 		}
 		rec_t* rec = btr_pcur_get_rec(pcur);
 		have_latch = true;
-		purge_sys.latch.rd_lock();
+		purge_sys.latch.rd_lock(SRW_LOCK_CALL);
 		if (!purge_sys.changes_visible(node->new_trx_id,
 					       node->table->name)) {
 			goto mtr_commit_exit;

storage/innobase/srv/srv0srv.cc

@@ -918,7 +918,7 @@ srv_printf_innodb_monitor(
 #ifdef BTR_CUR_HASH_ADAPT
 	for (ulint i = 0; i < btr_ahi_parts && btr_search_enabled; ++i) {
 		const auto part= &btr_search_sys.parts[i];
-		part->latch.rd_lock();
+		part->latch.rd_lock(SRW_LOCK_CALL);
 		ut_ad(part->heap->type == MEM_HEAP_FOR_BTR_SEARCH);
 		fprintf(file, "Hash table size " ULINTPF
 			", node heap has " ULINTPF " buffer(s)\n",
@@ -1059,7 +1059,7 @@ srv_export_innodb_status(void)
 	ulint mem_adaptive_hash = 0;
 	for (ulong i = 0; i < btr_ahi_parts; i++) {
 		const auto part= &btr_search_sys.parts[i];
-		part->latch.rd_lock();
+		part->latch.rd_lock(SRW_LOCK_CALL);
 		if (part->heap) {
 			ut_ad(part->heap->type == MEM_HEAP_FOR_BTR_SEARCH);
 
@@ -1477,7 +1477,7 @@ bool purge_sys_t::running() const
 /** Stop purge during FLUSH TABLES FOR EXPORT */
 void purge_sys_t::stop()
 {
-  latch.wr_lock();
+  latch.wr_lock(SRW_LOCK_CALL);
 
   if (!enabled())
   {
@@ -1514,7 +1514,7 @@ void purge_sys_t::resume()
    ut_ad(srv_force_recovery < SRV_FORCE_NO_BACKGROUND);
    ut_ad(!sync_check_iterate(sync_check()));
    purge_coordinator_task.enable();
-   latch.wr_lock();
+   latch.wr_lock(SRW_LOCK_CALL);
    int32_t paused= m_paused--;
    ut_a(paused);
 

storage/innobase/sync/srw_lock.cc

@@ -176,3 +176,39 @@ void srw_lock_low::write_lock()
 void srw_lock_low::rd_unlock() { if (read_unlock()) writer_wake(); }
 
 void srw_lock_low::wr_unlock() { write_unlock(); readers_wake(); }
+
+#ifdef UNIV_PFS_RWLOCK
+void srw_lock::psi_rd_lock(const char *file, unsigned line)
+{
+  PSI_rwlock_locker_state state;
+  uint32_t l;
+  const bool nowait= lock.read_trylock(l);
+  if (PSI_rwlock_locker *locker= PSI_RWLOCK_CALL(start_rwlock_rdwait)
+      (&state, pfs_psi,
+       nowait ? PSI_RWLOCK_TRYREADLOCK : PSI_RWLOCK_READLOCK, file, line))
+  {
+    if (!nowait)
+      lock.read_lock(l);
+    PSI_RWLOCK_CALL(end_rwlock_rdwait)(locker, 0);
+  }
+  else if (!nowait)
+    lock.read_lock(l);
+}
+
+void srw_lock::psi_wr_lock(const char *file, unsigned line)
+{
+  PSI_rwlock_locker_state state;
+  const bool nowait= lock.write_trylock();
+  if (PSI_rwlock_locker *locker= PSI_RWLOCK_CALL(start_rwlock_wrwait)
+      (&state, pfs_psi,
+       nowait ? PSI_RWLOCK_TRYWRITELOCK : PSI_RWLOCK_WRITELOCK,
+       file, line))
+  {
+    if (!nowait)
+      lock.wr_lock();
+    PSI_RWLOCK_CALL(end_rwlock_rdwait)(locker, 0);
+  }
+  else if (!nowait)
+    lock.wr_lock();
+}
+#endif /* UNIV_PFS_RWLOCK */

storage/innobase/trx/trx0i_s.cc

@@ -1305,7 +1305,7 @@ trx_i_s_cache_start_read(
 /*=====================*/
 	trx_i_s_cache_t*	cache)	/*!< in: cache */
 {
-	cache->rw_lock.rd_lock();
+	cache->rw_lock.rd_lock(SRW_LOCK_CALL);
 }
 
 /*******************************************************************//**
@@ -1326,7 +1326,7 @@ trx_i_s_cache_start_write(
 /*======================*/
 	trx_i_s_cache_t*	cache)	/*!< in: cache */
 {
-	cache->rw_lock.wr_lock();
+	cache->rw_lock.wr_lock(SRW_LOCK_CALL);
 }
 
 /*******************************************************************//**

storage/innobase/trx/trx0rec.cc

@@ -2201,7 +2201,7 @@ trx_undo_get_undo_rec(
 	const table_name_t&	name,
 	trx_undo_rec_t**	undo_rec)
 {
-	purge_sys.latch.rd_lock();
+	purge_sys.latch.rd_lock(SRW_LOCK_CALL);
 
 	bool missing_history = purge_sys.changes_visible(trx_id, name);
 	if (!missing_history) {
@@ -2364,7 +2364,7 @@ trx_undo_prev_version_build(
 
 		if ((update->info_bits & REC_INFO_DELETED_FLAG)
 		    && row_upd_changes_disowned_external(update)) {
-			purge_sys.latch.rd_lock();
+			purge_sys.latch.rd_lock(SRW_LOCK_CALL);
 
 			bool missing_extern = purge_sys.changes_visible(
 				trx_id,	index->table->name);