MySQL Bugs: #98499: Improvement about CPU cache line size

Bug #98499	Improvement about CPU cache line size
Submitted:	6 Feb 2020 11:35	Modified:	11 Feb 2020 13:34
Reporter:	Bo Zhao	Email Updates:
Status:	Verified	Impact on me:	None
Category:	MySQL Server: Compiling	Severity:	S3 (Non-critical)
Version:		OS:	Any
Assigned to:		CPU Architecture:	ARM

Description:
Currently Mysql is all aligned to 64 byte cacheline, but for Arm, the cacheline is 128B, so we need to find the 64 byte change to 128B in the code of mysql to fit on arm.

Also this must need a check when the change happen. And this check must be based on the ARCH info.

How to repeat:
not involve

Hi Mr. Zhao,

Thank you very much for your bug report.

This report seems very interesting to me. However, we need some additional info from you.

First of all, how have you come to that conclusion ??? What tools should be used and in which manner ???

Second, do you have any idea of how can we mend that ???

Third, do you have any patch prepared ????

hi
1.In mysql8 source code,we find that INNOBASE_CACHE_LINE_SIZE is set to 64,and set to 128 on powerpc.just like this:
#ifdef __powerpc__
 #define INNOBASE_CACHE_LINE_SIZE 128
#else
 #define INNOBASE_CACHE_LINE_SIZE 64
#endif /* __powerpc__ */
and in some files,the source code add a pad  variable to void false sharing,just like this :
 char pad3[64]; /*!< To avoid false sharing */
the 64 is hard code .

2.we plan to add aarch64 when define the INNOBASE_CACHE_LINE_SIZE，and use the INNOBASE_CACHE_LINE_SIZE to instead of 64.
and also we will add aarch64 branch if it cannot use INNOBASE_CACHE_LINE_SIZE

3.we have a patch，git diff like this，based on mysql-8.0.17：

diff --git a/mysql_src/include/lf.h b/mysql_src/include/lf.h
index 78686ae8..53898f42 100644
--- a/mysql_src/include/lf.h
+++ b/mysql_src/include/lf.h
@@ -86,16 +86,10 @@ struct LF_PINS {
   void *purgatory;
   uint32 purgatory_count;
   std::atomic<uint32> link;
-  /* we want sizeof(LF_PINS) to be 64 or 128 to avoid false sharing */
-#ifdef __aarch64__
-#if SIZEOF_INT * 2 + SIZEOF_CHARP * (LF_PINBOX_PINS + 2) != 128
-  char pad[128 - sizeof(uint32) * 2 - sizeof(void *) * (LF_PINBOX_PINS + 2)];
-#endif
-#else
+  /* we want sizeof(LF_PINS) to be 64 to avoid false sharing */
 #if SIZEOF_INT * 2 + SIZEOF_CHARP * (LF_PINBOX_PINS + 2) != 64
   char pad[64 - sizeof(uint32) * 2 - sizeof(void *) * (LF_PINBOX_PINS + 2)];
 #endif
-#endif /* __aarch64__ */
 };
 
 /*
diff --git a/mysql_src/mysys/lf_alloc-pin.cc b/mysql_src/mysys/lf_alloc-pin.cc
index d04e44da..02d6c366 100644
--- a/mysql_src/mysys/lf_alloc-pin.cc
+++ b/mysql_src/mysys/lf_alloc-pin.cc
@@ -138,11 +138,7 @@ static void lf_pinbox_real_free(LF_PINS *pins);
 void lf_pinbox_init(LF_PINBOX *pinbox, uint free_ptr_offset,
                     lf_pinbox_free_func *free_func, void *free_func_arg) {
   DBUG_ASSERT(free_ptr_offset % sizeof(void *) == 0);
-#ifdef __aarch64__
-  static_assert(sizeof(LF_PINS) == 128, "");
-#else
   static_assert(sizeof(LF_PINS) == 64, "");
-#endif /* __aarch64__ */  
   lf_dynarray_init(&pinbox->pinarray, sizeof(LF_PINS));
   pinbox->pinstack_top_ver = 0;
   pinbox->pins_in_array = 0;
diff --git a/mysql_src/storage/innobase/btr/btr0sea.cc b/mysql_src/storage/innobase/btr/btr0sea.cc
index b736310c..0436e4ac 100644
--- a/mysql_src/storage/innobase/btr/btr0sea.cc
+++ b/mysql_src/storage/innobase/btr/btr0sea.cc
@@ -69,7 +69,7 @@ ulint btr_search_n_hash_fail = 0;
 /** padding to prevent other memory update
 hotspots from residing on the same memory
 cache line as btr_search_latches */
-byte btr_sea_pad1[INNOBASE_CACHE_LINE_SIZE];
+byte btr_sea_pad1[64];
 
 /** The latches protecting the adaptive search system: this latches protects the
 (1) positions of records on those pages where a hash index has been built.
@@ -81,7 +81,7 @@ rw_lock_t **btr_search_latches;
 
 /** padding to prevent other memory update hotspots from residing on
 the same memory cache line */
-byte btr_sea_pad2[INNOBASE_CACHE_LINE_SIZE];
+byte btr_sea_pad2[64];
 
 /** The adaptive hash index */
 btr_search_sys_t *btr_search_sys;
diff --git a/mysql_src/storage/innobase/include/read0types.h b/mysql_src/storage/innobase/include/read0types.h
index 4bf1d85e..973f1018 100644
--- a/mysql_src/storage/innobase/include/read0types.h
+++ b/mysql_src/storage/innobase/include/read0types.h
@@ -316,7 +316,7 @@ class ReadView {
   typedef UT_LIST_NODE_T(ReadView) node_t;
 
   /** List of read views in trx_sys */
-  byte pad1[INNOBASE_CACHE_LINE_SIZE - sizeof(node_t)];
+  byte pad1[64 - sizeof(node_t)];
   node_t m_view_list;
 };
 
diff --git a/mysql_src/storage/innobase/include/trx0sys.h b/mysql_src/storage/innobase/include/trx0sys.h
index 163756ee..d7cc4edf 100644
--- a/mysql_src/storage/innobase/include/trx0sys.h
+++ b/mysql_src/storage/innobase/include/trx0sys.h
@@ -445,13 +445,13 @@ struct trx_sys_t {
                           transactions added for purge. */
 #endif                    /* UNIV_DEBUG */
 
-  char pad1[INNOBASE_CACHE_LINE_SIZE];             /*!< To avoid false sharing */
+  char pad1[64];             /*!< To avoid false sharing */
   trx_ut_list_t rw_trx_list; /*!< List of active and committed in
                              memory read-write transactions, sorted
                              on trx id, biggest first. Recovered
                              transactions are always on this list. */
 
-  char pad2[INNOBASE_CACHE_LINE_SIZE];                /*!< To avoid false sharing */
+  char pad2[64];                /*!< To avoid false sharing */
   trx_ut_list_t mysql_trx_list; /*!< List of transactions created
                                 for MySQL. All user transactions are
                                 on mysql_trx_list. The rw_trx_list
@@ -471,7 +471,7 @@ struct trx_sys_t {
                         to ensure right order of removal and
                         consistent snapshot. */
 
-  char pad3[INNOBASE_CACHE_LINE_SIZE]; /*!< To avoid false sharing */
+  char pad3[64]; /*!< To avoid false sharing */
 
   Rsegs rsegs; /*!< Vector of pointers to rollback
                segments. These rsegs are iterated
diff --git a/mysql_src/storage/innobase/include/ut0counter.h b/mysql_src/storage/innobase/include/ut0counter.h
index 1945243a..3cc484ee 100644
--- a/mysql_src/storage/innobase/include/ut0counter.h
+++ b/mysql_src/storage/innobase/include/ut0counter.h
@@ -46,11 +46,11 @@ this program; if not, write to the Free Software Foundation, Inc.,
 #include <functional>
 
 /** CPU cache line size */
-#if defined(__powerpc__) || defined(__aarch64__)
+#ifdef __powerpc__
 #define INNOBASE_CACHE_LINE_SIZE 128
 #else
 #define INNOBASE_CACHE_LINE_SIZE 64
-#endif /* __powerpc__ || __aarch64__ */
+#endif /* __powerpc__ */
 
 /** Default number of slots to use in ib_counter_t */
 #define IB_N_SLOTS 64
diff --git a/mysql_src/storage/innobase/srv/srv0conc.cc b/mysql_src/storage/innobase/srv/srv0conc.cc
index b00d213b..abe85641 100644
--- a/mysql_src/storage/innobase/srv/srv0conc.cc
+++ b/mysql_src/storage/innobase/srv/srv0conc.cc
@@ -71,7 +71,7 @@ ulong srv_thread_concurrency = 0;
 
 /** Variables tracking the active and waiting threads. */
 struct srv_conc_t {
-  char pad[INNOBASE_CACHE_LINE_SIZE - (sizeof(ulint) + sizeof(lint))];
+  char pad[64 - (sizeof(ulint) + sizeof(lint))];
 
   /** Number of transactions that have declared_to_be_inside_innodb set.
   It used to be a non-error for this value to drop below zero temporarily.

Hello Mr. Zhao,

Thank you, very much, for your contribution. Both with this bug and with your patch.

Verified as reported.

In the interests of simplicity and not treading on powerpc changes, for innodb just changing INNOBASE_CACHE_LINE_SIZE in storage/innobase/include/ut0counter.h is sufficient for the other changes to have the same effect.

Likewise a single definition in lf.h could be reused in mysys/lf_alloc-pin.cc.

Thank you, Mr. Black.