Bug #87350 CACHE_LINE_SIZE in innodb should be 128 on aarch64

• Submitted: 9 Aug 2017 2:31
• Modified: 24 Aug 2017 8:05
• Reporter: jared ZHU
• Status: Verified
• Category: MySQL Server: InnoDB storage engine
• Severity: S5 (Performance)
• Version: 8.0.2,5.6,5.7
• OS: Any
• CPU Architecture: Any
• Tags: CACHE_LINE_SIZE

[9 Aug 2017 2:31] jared ZHU

Description:
Increase the aarch64 standard cacheline size to avoid cacheline competition.

HUAWEI and Cavium aarch64 platform implements cache lines of 128 byte size.

Increasing the size has no negative impact to cache invalidation on
systems with a smaller cache line.

How to repeat:
performance test and the linux kernel also increase the aarch64 cacheline size from 64 to 128 byte.

Suggested fix:
I'll attach the patch:

diff --git a/storage/innobase/include/ut0counter.h b/storage/innobase/include/ut0counter.h
index 1e3008a..6b8f80c 100644
--- a/storage/innobase/include/ut0counter.h
+++ b/storage/innobase/include/ut0counter.h
@@ -32,11 +32,11 @@ Created 2012/04/12 by Sunny Bains
 #include "os0thread.h"

 /** CPU cache line size */
-#ifdef __powerpc__
+#if defined(__powerpc__) || defined(__aarch64__)
 #define CACHE_LINE_SIZE                128
 #else
 #define CACHE_LINE_SIZE                64
-#endif /* __powerpc__ */
+#endif /* __powerpc__ && __aarch64__*/

 /** Default number of slots to use in ib_counter_t */
 #define IB_N_SLOTS             64
diff --git a/storage/ndb/memcache/include/ndbmemcache_global.h b/storage/ndb/memcache/include/ndbmemcache_global.h
index b94da82..9975ad2 100644
--- a/storage/ndb/memcache/include/ndbmemcache_global.h
+++ b/storage/ndb/memcache/include/ndbmemcache_global.h
@@ -33,8 +33,12 @@ Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
 #define END_FUNCTIONS_WITH_C_LINKAGE
 #endif

-/* CPU cache line size; a constant 64 for now.  */
-#define CACHE_LINE_SIZE 64
+/** CPU cache line size */
+#if defined(__powerpc__) || defined(__aarch64__)
+#define CACHE_LINE_SIZE         128
+#else
+#define CACHE_LINE_SIZE         64
+#endif /* __powerpc__ && __aarch64__*/

 /* A memcached constant; also defined in default_engine.h */
 #define POWER_LARGEST  200

[9 Aug 2017 2:32] jared ZHU

attached the patch

Attachment: cacheline_increased.patch (application/octet-stream, text), 1.32 KiB.

[9 Aug 2017 5:24] MySQL Verification Team

Hello Jared ZHU,

Thank you for the report and providing patch.
Please note that in order to submit contributions you must first sign the Oracle Contribution Agreement (OCA). For additional information please check http://www.oracle.com/technetwork/community/oca-486395.html.
If you have any questions, please contact the MySQL community team - https://www.mysql.com/about/contact/?topic=community.

Thanks,
Umesh

[9 Aug 2017 6:44] jared ZHU

I have signed the Oracle Contribution Agreement (OCA), and send it to  [e-mail].

[9 Aug 2017 6:44] jared ZHU

I have signed the Oracle Contribution Agreement (OCA), and send it to  [e-mail].

[9 Aug 2017 6:58] Alexey Kopytov

Is this a duplicate of https://bugs.mysql.com/bug.php?id=79636 ?

An extended version of the patch has also been submitted as https://github.com/mysql/mysql-server/pull/37

[24 Aug 2017 8:05] jared ZHU

Thank you. 
It is the same of the https://bugs.mysql.com/bug.php?id=79636 .

But the patch do not fix all, the trx_sys variable is hard code the pad[64] in the struct trx_sys_t like bellow:

struct trx_sys_t {
        TrxSysMutex     mutex;          /*!< mutex protecting most fields in
                                        this structure except when noted
                                        otherwise */

        MVCC*           mvcc;           /*!< Multi version concurrency control
                                        manager */
        volatile trx_id_t
                        max_trx_id;     /*!< The smallest number not yet
                                        assigned as a transaction id or
                                        transaction number. This is declared
                                        volatile because it can be accessed
                                        without holding any mutex during
                                        AC-NL-RO view creation. */
        trx_ut_list_t   serialisation_list;
                                        /*!< Ordered on trx_t::no of all the
                                        currenrtly active RW transactions */
#ifdef UNIV_DEBUG
        trx_id_t        rw_max_trx_id;  /*!< Max trx id of read-write
                                        transactions which exist or existed */
#endif /* UNIV_DEBUG */

        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[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

        .......
}

and it is should be pad[CACHE_LINE_SIZE] to avoid hotspots from residing on the same memory cache line.

[24 Aug 2017 8:12] Alexey Kopytov

Right, the patch in https://github.com/mysql/mysql-server/pull/37 addressed the problem reported in this bug (and the identical in in bug #79636) + a number of other issues reported separately (including the one in trx_sys_t):

Bug #79637: Hard-coded cache line size
Bug #79638: Reconcile CACHE_LINE_SIZE with CPU_LEVEL1_DCACHE_LINESIZE
Bug #79652: Suspicious padding in srv_conc_t