Cleanup: Remove unnecessary trx_i_s_cache_t::last_read_mutex

We can simply use C++11 std::atomic for avoiding undefined behaviour related to concurrent stores to a shared variable. On most if not all ISAs, std::memory_order_relaxed loads and stores will not really differ from non-atomic loads or stores.
MariaDB · Oct 1, 2020 · bd64c2e · bd64c2e
1 parent 9ae608d
commit bd64c2e
Show file tree

Hide file tree

Showing 6 changed files with 4 additions and 31 deletions.
diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc
@@ -588,7 +588,6 @@ static PSI_mutex_info all_innodb_mutexes[] = {
 #  endif /* !PFS_SKIP_BUFFER_MUTEX_RWLOCK */
 	PSI_KEY(buf_pool_mutex),
 	PSI_KEY(buf_pool_zip_mutex),
-	PSI_KEY(cache_last_read_mutex),
 	PSI_KEY(dict_foreign_err_mutex),
 	PSI_KEY(dict_sys_mutex),
 	PSI_KEY(recalc_pool_mutex),

diff --git a/storage/innobase/include/sync0sync.h b/storage/innobase/include/sync0sync.h
@@ -53,7 +53,6 @@ instrumentation due to their large number of instances. */
 extern mysql_pfs_key_t	buffer_block_mutex_key;
 extern mysql_pfs_key_t	buf_pool_mutex_key;
 extern mysql_pfs_key_t	buf_pool_zip_mutex_key;
-extern mysql_pfs_key_t	cache_last_read_mutex_key;
 extern mysql_pfs_key_t	dict_foreign_err_mutex_key;
 extern mysql_pfs_key_t	dict_sys_mutex_key;
 extern mysql_pfs_key_t	fil_system_mutex_key;

diff --git a/storage/innobase/include/sync0types.h b/storage/innobase/include/sync0types.h
@@ -258,8 +258,6 @@ enum latch_level_t {
 
 	SYNC_DICT_OPERATION,
 
-	SYNC_TRX_I_S_LAST_READ,
-
 	SYNC_TRX_I_S_RWLOCK,
 
 	SYNC_RECV_WRITER,
@@ -284,7 +282,6 @@ enum latch_id_t {
 	LATCH_ID_BUF_BLOCK_MUTEX,
 	LATCH_ID_BUF_POOL,
 	LATCH_ID_BUF_POOL_ZIP,
-	LATCH_ID_CACHE_LAST_READ,
 	LATCH_ID_DICT_FOREIGN_ERR,
 	LATCH_ID_DICT_SYS,
 	LATCH_ID_FILE_FORMAT_MAX,

diff --git a/storage/innobase/sync/sync0debug.cc b/storage/innobase/sync/sync0debug.cc
@@ -508,7 +508,6 @@ LatchDebug::LatchDebug()
 	LEVEL_MAP_INSERT(SYNC_DICT);
 	LEVEL_MAP_INSERT(SYNC_FTS_CACHE);
 	LEVEL_MAP_INSERT(SYNC_DICT_OPERATION);
-	LEVEL_MAP_INSERT(SYNC_TRX_I_S_LAST_READ);
 	LEVEL_MAP_INSERT(SYNC_TRX_I_S_RWLOCK);
 	LEVEL_MAP_INSERT(SYNC_RECV_WRITER);
 	LEVEL_MAP_INSERT(SYNC_LEVEL_VARYING);
@@ -767,7 +766,6 @@ LatchDebug::check_order(
 	case SYNC_DICT_OPERATION:
 	case SYNC_DICT_HEADER:
 	case SYNC_TRX_I_S_RWLOCK:
-	case SYNC_TRX_I_S_LAST_READ:
 	case SYNC_IBUF_MUTEX:
 	case SYNC_INDEX_ONLINE_LOG:
 	case SYNC_STATS_AUTO_RECALC:
@@ -1285,9 +1283,6 @@ sync_latch_meta_init()
 
 	LATCH_ADD_MUTEX(BUF_POOL_ZIP, SYNC_BUF_BLOCK, buf_pool_zip_mutex_key);
 
-	LATCH_ADD_MUTEX(CACHE_LAST_READ, SYNC_TRX_I_S_LAST_READ,
-			cache_last_read_mutex_key);
-
 	LATCH_ADD_MUTEX(DICT_FOREIGN_ERR, SYNC_NO_ORDER_CHECK,
 			dict_foreign_err_mutex_key);
 

diff --git a/storage/innobase/sync/sync0sync.cc b/storage/innobase/sync/sync0sync.cc
@@ -38,7 +38,6 @@ Created 9/5/1995 Heikki Tuuri
 mysql_pfs_key_t	buffer_block_mutex_key;
 mysql_pfs_key_t	buf_pool_mutex_key;
 mysql_pfs_key_t	buf_pool_zip_mutex_key;
-mysql_pfs_key_t	cache_last_read_mutex_key;
 mysql_pfs_key_t	dict_foreign_err_mutex_key;
 mysql_pfs_key_t	dict_sys_mutex_key;
 mysql_pfs_key_t	fil_system_mutex_key;

diff --git a/storage/innobase/trx/trx0i_s.cc b/storage/innobase/trx/trx0i_s.cc
@@ -141,12 +141,9 @@ struct i_s_table_cache_t {
 struct trx_i_s_cache_t {
 	rw_lock_t	rw_lock;	/*!< read-write lock protecting
 					the rest of this structure */
-	ulonglong	last_read;	/*!< last time the cache was read;
+	Atomic_relaxed<ulonglong> last_read;
+					/*!< last time the cache was read;
 					measured in nanoseconds */
-	ib_mutex_t		last_read_mutex;/*!< mutex protecting the
-					last_read member - it is updated
-					inside a shared lock of the
-					rw_lock member */
 	i_s_table_cache_t innodb_trx;	/*!< innodb_trx table */
 	i_s_table_cache_t innodb_locks;	/*!< innodb_locks table */
 	i_s_table_cache_t innodb_lock_waits;/*!< innodb_lock_waits table */
@@ -1190,8 +1187,7 @@ Checks if the cache can safely be updated.
 @return whether the cache can be updated */
 static bool can_cache_be_updated(trx_i_s_cache_t* cache)
 {
-	/* Here we read cache->last_read without acquiring its mutex
-	because last_read is only updated when a shared rw lock on the
+	/* cache->last_read is only updated when a shared rw lock on the
 	whole cache is being held (see trx_i_s_cache_end_read()) and
 	we are currently holding an exclusive rw lock on the cache.
 	So it is not possible for last_read to be updated while we are
@@ -1329,17 +1325,13 @@ trx_i_s_cache_init(
 	release lock mutex
 	release trx_i_s_cache_t::rw_lock
 	acquire trx_i_s_cache_t::rw_lock, S
-	acquire trx_i_s_cache_t::last_read_mutex
-	release trx_i_s_cache_t::last_read_mutex
 	release trx_i_s_cache_t::rw_lock */
 
 	rw_lock_create(trx_i_s_cache_lock_key, &cache->rw_lock,
 		       SYNC_TRX_I_S_RWLOCK);
 
 	cache->last_read = 0;
 
-	mutex_create(LATCH_ID_CACHE_LAST_READ, &cache->last_read_mutex);
-
 	table_cache_init(&cache->innodb_trx, sizeof(i_s_trx_row_t));
 	table_cache_init(&cache->innodb_locks, sizeof(i_s_locks_row_t));
 	table_cache_init(&cache->innodb_lock_waits,
@@ -1363,7 +1355,6 @@ trx_i_s_cache_free(
 	trx_i_s_cache_t*	cache)	/*!< in, own: cache to free */
 {
 	rw_lock_free(&cache->rw_lock);
-	mutex_free(&cache->last_read_mutex);
 
 	hash_table_free(cache->locks_hash);
 	ha_storage_free(cache->storage);
@@ -1389,14 +1380,7 @@ trx_i_s_cache_end_read(
 /*===================*/
 	trx_i_s_cache_t*	cache)	/*!< in: cache */
 {
-	ut_ad(rw_lock_own(&cache->rw_lock, RW_LOCK_S));
-
-	/* update cache last read time */
-	const ulonglong now = my_interval_timer();
-	mutex_enter(&cache->last_read_mutex);
-	cache->last_read = now;
-	mutex_exit(&cache->last_read_mutex);
-
+	cache->last_read = my_interval_timer();
 	rw_lock_s_unlock(&cache->rw_lock);
 }