MDEV-22930 Unnecessary contention on rw_lock_list_mutex in ibuf_dummy_index_create()

1. Do not initialize dict_table_t::stats_latch in ibuf
2. Remove overengineering in GenericPolicy to speed up things

dict_mem_table_create(): add new argument init_stats_latch

ibuf_dummy_index_create(): do not initialize dict_table_t::stats_latch

GenericPolicy: add new members m_filename and m_line

sync_file_create_register()
sync_file_created_deregister()
sync_file_created_get()
CreateTracker: remove

rw_lock_t::created: a new debug member

storage/innobase/dict/dict0mem.cc

@@ -124,19 +124,25 @@ bool dict_col_t::same_encoding(uint16_t a, uint16_t b)
   return false;
 }
 
-/**********************************************************************//**
-Creates a table memory object.
+/** Creates a table memory object.
+@param[in]	name		table name
+@param[in]	space		tablespace
+@param[in]	n_cols		total number of columns including virtual and
+				non-virtual columns
+@param[in]	n_v_cols	number of virtual columns
+@param[in]	flags		table flags
+@param[in]	flags2		table flags2
+@param[in]	init_stats_latch	whether to init the stats latch
 @return own: table object */
 dict_table_t*
 dict_mem_table_create(
-/*==================*/
-	const char*	name,	/*!< in: table name */
-	fil_space_t*	space,	/*!< in: tablespace */
-	ulint		n_cols,	/*!< in: total number of columns including
-				virtual and non-virtual columns */
-	ulint		n_v_cols,/*!< in: number of virtual columns */
-	ulint		flags,	/*!< in: table flags */
-	ulint		flags2)	/*!< in: table flags2 */
+	const char*	name,
+	fil_space_t*	space,
+	ulint		n_cols,
+	ulint		n_v_cols,
+	ulint		flags,
+	ulint		flags2,
+	bool		init_stats_latch)
 {
 	dict_table_t*	table;
 	mem_heap_t*	heap;
@@ -204,8 +210,11 @@ dict_mem_table_create(
 	new(&table->foreign_set) dict_foreign_set();
 	new(&table->referenced_set) dict_foreign_set();
 
-	rw_lock_create(dict_table_stats_key, &table->stats_latch,
-		       SYNC_INDEX_TREE);
+	if (init_stats_latch) {
+		rw_lock_create(dict_table_stats_key, &table->stats_latch,
+			       SYNC_INDEX_TREE);
+		table->stats_latch_inited = true;
+	}
 
 	return(table);
 }
@@ -249,7 +258,9 @@ dict_mem_table_free(
 
 	UT_DELETE(table->s_cols);
 
-	rw_lock_free(&table->stats_latch);
+	if (table->stats_latch_inited) {
+		rw_lock_free(&table->stats_latch);
+	}
 
 	mem_heap_free(table->heap);
 }

storage/innobase/ibuf/ibuf0ibuf.cc

@@ -1309,7 +1309,7 @@ ibuf_dummy_index_create(
 	dict_index_t*	index;
 
 	table = dict_mem_table_create("IBUF_DUMMY", NULL, n, 0,
-				      comp ? DICT_TF_COMPACT : 0, 0);
+				      comp ? DICT_TF_COMPACT : 0, 0, false);
 
 	index = dict_mem_index_create(table, "IBUF_DUMMY", 0, n);
 

storage/innobase/include/dict0mem.h

@@ -297,22 +297,21 @@ parent table will fail, and user has to drop excessive foreign constraint
 before proceeds. */
 #define FK_MAX_CASCADE_DEL		15
 
-/**********************************************************************//**
-Creates a table memory object.
+/** Creates a table memory object.
+@param[in]	name		table name
+@param[in]	space		tablespace
+@param[in]	n_cols		total number of columns including virtual and
+				non-virtual columns
+@param[in]	n_v_cols	number of virtual columns
+@param[in]	flags		table flags
+@param[in]	flags2		table flags2
+@param[in]	init_stats_latch	whether to init the stats latch
 @return own: table object */
-dict_table_t*
-dict_mem_table_create(
-/*==================*/
-	const char*	name,		/*!< in: table name */
-	fil_space_t*	space,		/*!< in: tablespace */
-	ulint		n_cols,		/*!< in: total number of columns
-					including virtual and non-virtual
-					columns */
-	ulint		n_v_cols,	/*!< in: number of virtual columns */
-	ulint		flags,		/*!< in: table flags */
-	ulint		flags2);	/*!< in: table flags2 */
-/****************************************************************//**
-Free a table memory object. */
+dict_table_t *dict_mem_table_create(const char *name, fil_space_t *space,
+                                    ulint n_cols, ulint n_v_cols, ulint flags,
+                                    ulint flags2, bool init_stats_latch= true);
+/****************************************************************/ /**
+ Free a table memory object. */
 void
 dict_mem_table_free(
 /*================*/
@@ -2146,6 +2145,8 @@ struct dict_table_t {
 	performance reasons. */
 	rw_lock_t				stats_latch;
 
+  bool stats_latch_inited= false;
+
 	/** TRUE if statistics have been calculated the first time after
 	database startup or table creation. */
 	unsigned				stat_initialized:1;

storage/innobase/include/sync0policy.h

@@ -220,7 +220,8 @@ struct GenericPolicy
 
 		meta.get_counter()->single_register(&m_count);
 
-		sync_file_created_register(this, filename, uint16_t(line));
+		m_filename = filename;
+		m_line = line;
 	}
 
 	/** Called when the mutex is destroyed. */
@@ -230,8 +231,6 @@ struct GenericPolicy
 		latch_meta_t&	meta = sync_latch_get_meta(m_id);
 
 		meta.get_counter()->single_deregister(&m_count);
-
-		sync_file_created_deregister(this);
 	}
 
 	/** Called after a successful mutex acquire.
@@ -272,13 +271,21 @@ struct GenericPolicy
 
   /** @return the string representation */
   std::string to_string() const
-  { return sync_mutex_to_string(get_id(), sync_file_created_get(this)); }
+  {
+    return sync_mutex_to_string(get_id(),
+                                std::string(m_filename)
+                                    .append(":")
+                                    .append(std::to_string(m_line)));
+  }
 
 #ifdef UNIV_DEBUG
   MutexDebug<Mutex> context;
 #endif
 
 private:
+  const char *m_filename;
+  uint32_t m_line;
+
   /** The user visible counters, registered with the meta-data. */
   latch_meta_t::CounterType::Count m_count;
 

storage/innobase/include/sync0rw.h

@@ -567,6 +567,8 @@ struct rw_lock_t :
 #endif /* UNIV_DEBUG */
 	public ilist_node<>
 {
+  ut_d(bool created= false;)
+
   /** Holds the state of the lock. */
   Atomic_relaxed<int32_t> lock_word;
 

storage/innobase/include/sync0types.h

@@ -941,27 +941,6 @@ sync_latch_get_name(latch_level_t level);
 const char*
 sync_basename(const char* filename);
 
-/** Register a latch, called when it is created
-@param[in]	ptr		Latch instance that was created
-@param[in]	filename	Filename where it was created
-@param[in]	line		Line number in filename */
-void
-sync_file_created_register(
-	const void*	ptr,
-	const char*	filename,
-	uint16_t	line);
-
-/** Deregister a latch, called when it is destroyed
-@param[in]	ptr		Latch to be destroyed */
-void
-sync_file_created_deregister(const void* ptr);
-
-/** Get the string where the file was created. Its format is "name:line"
-@param[in]	ptr		Latch instance
-@return created information or "" if can't be found */
-std::string
-sync_file_created_get(const void* ptr);
-
 #ifdef UNIV_DEBUG
 
 /** All (ordered) latches, used in debugging, must derive from this class. */

storage/innobase/sync/sync0debug.cc

@@ -38,7 +38,7 @@ Created 2012-08-21 Sunny Bains
 #include <vector>
 #include <string>
 #include <algorithm>
-#include <iostream>
+#include <map>
 
 #ifdef UNIV_DEBUG
 
@@ -1506,173 +1506,6 @@ sync_latch_meta_destroy()
 	latch_meta.clear();
 }
 
-/** Track mutex file creation name and line number. This is to avoid storing
-{ const char* name; uint16_t line; } in every instance. This results in the
-sizeof(Mutex) > 64. We use a lookup table to store it separately. Fetching
-the values is very rare, only required for diagnostic purposes. And, we
-don't create/destroy mutexes that frequently. */
-struct CreateTracker {
-
-	/** Constructor */
-	CreateTracker()
-		UNIV_NOTHROW
-	{
-		m_mutex.init();
-	}
-
-	/** Destructor */
-	~CreateTracker()
-		UNIV_NOTHROW
-	{
-		ut_ad(m_files.empty());
-
-		m_mutex.destroy();
-	}
-
-	/** Register where the latch was created
-	@param[in]	ptr		Latch instance
-	@param[in]	filename	Where created
-	@param[in]	line		Line number in filename */
-	void register_latch(
-		const void*	ptr,
-		const char*	filename,
-		uint16_t	line)
-		UNIV_NOTHROW
-	{
-		m_mutex.enter();
-
-		Files::iterator	lb = m_files.lower_bound(ptr);
-
-		ut_ad(lb == m_files.end()
-		      || m_files.key_comp()(ptr, lb->first));
-
-		typedef Files::value_type value_type;
-
-		m_files.insert(lb, value_type(ptr, File(filename, line)));
-
-		m_mutex.exit();
-	}
-
-	/** Deregister a latch - when it is destroyed
-	@param[in]	ptr		Latch instance being destroyed */
-	void deregister_latch(const void* ptr)
-		UNIV_NOTHROW
-	{
-		m_mutex.enter();
-
-		Files::iterator	lb = m_files.lower_bound(ptr);
-
-		ut_ad(lb != m_files.end()
-		      && !(m_files.key_comp()(ptr, lb->first)));
-
-		m_files.erase(lb);
-
-		m_mutex.exit();
-	}
-
-	/** Get the create string, format is "name:line"
-	@param[in]	ptr		Latch instance
-	@return the create string or "" if not found */
-	std::string get(const void* ptr)
-		UNIV_NOTHROW
-	{
-		m_mutex.enter();
-
-		std::string	created;
-
-		Files::iterator	lb = m_files.lower_bound(ptr);
-
-		if (lb != m_files.end()
-		    && !(m_files.key_comp()(ptr, lb->first))) {
-
-			std::ostringstream	msg;
-
-			msg << lb->second.m_name << ":" << lb->second.m_line;
-
-			created = msg.str();
-		}
-
-		m_mutex.exit();
-
-		return(created);
-	}
-
-private:
-	/** For tracking the filename and line number */
-	struct File {
-
-		/** Constructor */
-		File() UNIV_NOTHROW : m_name(), m_line() { }
-
-		/** Constructor
-		@param[in]	name		Filename where created
-		@param[in]	line		Line number where created */
-		File(const char*  name, uint16_t line)
-			UNIV_NOTHROW
-			:
-			m_name(sync_basename(name)),
-			m_line(line)
-		{
-			/* No op */
-		}
-
-		/** Filename where created */
-		std::string		m_name;
-
-		/** Line number where created */
-		uint16_t		m_line;
-	};
-
-	/** Map the mutex instance to where it was created */
-	typedef std::map<
-		const void*,
-		File,
-		std::less<const void*>,
-		ut_allocator<std::pair<const void* const, File> > >
-		Files;
-
-	typedef OSMutex	Mutex;
-
-	/** Mutex protecting m_files */
-	Mutex			m_mutex;
-
-	/** Track the latch creation */
-	Files			m_files;
-};
-
-/** Track latch creation location. For reducing the size of the latches */
-static CreateTracker	create_tracker;
-
-/** Register a latch, called when it is created
-@param[in]	ptr		Latch instance that was created
-@param[in]	filename	Filename where it was created
-@param[in]	line		Line number in filename */
-void
-sync_file_created_register(
-	const void*	ptr,
-	const char*	filename,
-	uint16_t	line)
-{
-	create_tracker.register_latch(ptr, filename, line);
-}
-
-/** Deregister a latch, called when it is destroyed
-@param[in]	ptr		Latch to be destroyed */
-void
-sync_file_created_deregister(const void* ptr)
-{
-	create_tracker.deregister_latch(ptr);
-}
-
-/** Get the string where the file was created. Its format is "name:line"
-@param[in]	ptr		Latch instance
-@return created information or "" if can't be found */
-std::string
-sync_file_created_get(const void* ptr)
-{
-	return(create_tracker.get(ptr));
-}
-
 /** Initializes the synchronization data structures. */
 void
 sync_check_init()

storage/innobase/sync/sync0rw.cc

@@ -234,6 +234,8 @@ rw_lock_create_func(
 
 	lock->is_block_lock = 0;
 
+	ut_d(lock->created = true);
+
 	mutex_enter(&rw_lock_list_mutex);
 	rw_lock_list.push_front(*lock);
 	mutex_exit(&rw_lock_list_mutex);
@@ -251,6 +253,8 @@ rw_lock_free_func(
 	ut_ad(rw_lock_validate(lock));
 	ut_a(lock->lock_word == X_LOCK_DECR);
 
+	ut_d(lock->created = false);
+
 	mutex_enter(&rw_lock_list_mutex);
 
 	os_event_destroy(lock->event);
@@ -850,6 +854,8 @@ rw_lock_validate(
 {
 	ut_ad(lock);
 
+	ut_ad(lock->created);
+
 	int32_t lock_word = lock->lock_word;
 
 	ut_ad(lock->waiters < 2);