Bug#17507853 PREVENT FALSE SHARING OF CPU CACHE LINE FOR SENSITIVE COUNTERS

This fix is a code cleanup to avoid risks of performance degradations.

In the performance schema code, several internal counters are highly
sensitive, because they are used very often, and used with atomic
operations.

For these counters, it is critical that the counter is allocated on a CPU
cache line, and that no other variable is allocated in the same memory area,
which can end up in the same CPU cache line.

This fix introduce PFS_cacheline_uint32 and PFS_cacheline_uint64,
to enforce this property.

Al internal counters related to allocation internal objects have been
changed to use PFS_cacheline_uint32/64.

Whether CPU cache line collision did happened in the past,
causing performance degradations, is unknown, so this fix may not improve
current performance.

What this fix does however, is to make sure that cache line collisions are
now impossible, to prevent any regressions when using:
- different compilers
- different compiler options
- different CPUs

Also, the allocation algorithm for PFS_account, PFS_user and PFS_host have
been changed to follow the mainline code.

PFS_scan has been removed, this way of performing allocations is no longer
needed.

Author: marcalff

storage/perfschema/pfs_account.cc

@@ -36,8 +36,9 @@
   @{
 */
 
-ulong account_max;
-ulong account_lost;
+ulong account_max= 0;
+ulong account_lost= 0;
+bool account_full;
 
 PFS_account *account_array= NULL;
 
@@ -59,6 +60,8 @@ int init_account(const PFS_global_param *param)
   uint index;
 
   account_max= param->m_account_sizing;
+  account_lost= 0;
+  account_full= false;
 
   account_array= NULL;
   account_instr_class_waits_array= NULL;
@@ -225,11 +228,7 @@ find_or_create_account(PFS_thread *thread,
                          const char *username, uint username_length,
                          const char *hostname, uint hostname_length)
 {
-  if (account_max == 0)
-  {
-    account_lost++;
-    return NULL;
-  }
+  static PFS_ALIGNED PFS_cacheline_uint32 monotonic;
 
   LF_PINS *pins= get_account_hash_pins(thread);
   if (unlikely(pins == NULL))
@@ -243,16 +242,18 @@ find_or_create_account(PFS_thread *thread,
                     hostname, hostname_length);
 
   PFS_account **entry;
+  PFS_account *pfs;
   uint retry_count= 0;
   const uint retry_max= 3;
+  uint index;
+  uint attempts= 0;
 
 search:
   entry= reinterpret_cast<PFS_account**>
     (lf_hash_search(&account_hash, pins,
                     key.m_hash_key, key.m_key_length));
   if (entry && (entry != MY_ERRPTR))
   {
-    PFS_account *pfs;
     pfs= *entry;
     pfs->inc_refcount();
     lf_hash_search_unpin(pins);
@@ -261,88 +262,88 @@ find_or_create_account(PFS_thread *thread,
 
   lf_hash_search_unpin(pins);
 
-  PFS_scan scan;
-  uint random= randomized_index(username, account_max);
+  if (account_full)
+  {
+    account_lost++;
+    return NULL;
+  }
 
-  for (scan.init(random, account_max);
-       scan.has_pass();
-       scan.next_pass())
+  while (++attempts <= account_max)
   {
-    PFS_account *pfs= account_array + scan.first();
-    PFS_account *pfs_last= account_array + scan.last();
-    for ( ; pfs < pfs_last; pfs++)
+    index= PFS_atomic::add_u32(& monotonic.m_u32, 1) % account_max;
+    pfs= account_array + index;
+
+    if (pfs->m_lock.is_free())
     {
-      if (pfs->m_lock.is_free())
+      if (pfs->m_lock.free_to_dirty())
       {
-        if (pfs->m_lock.free_to_dirty())
+        pfs->m_key= key;
+        if (username_length > 0)
+          pfs->m_username= &pfs->m_key.m_hash_key[0];
+        else
+          pfs->m_username= NULL;
+        pfs->m_username_length= username_length;
+
+        if (hostname_length > 0)
+          pfs->m_hostname= &pfs->m_key.m_hash_key[username_length + 1];
+        else
+          pfs->m_hostname= NULL;
+        pfs->m_hostname_length= hostname_length;
+
+        pfs->m_user= find_or_create_user(thread, username, username_length);
+        pfs->m_host= find_or_create_host(thread, hostname, hostname_length);
+
+        pfs->init_refcount();
+        pfs->reset_stats();
+        pfs->m_disconnected_count= 0;
+
+        if (username_length > 0 && hostname_length > 0)
         {
-          pfs->m_key= key;
-          if (username_length > 0)
-            pfs->m_username= &pfs->m_key.m_hash_key[0];
-          else
-            pfs->m_username= NULL;
-          pfs->m_username_length= username_length;
-
-          if (hostname_length > 0)
-            pfs->m_hostname= &pfs->m_key.m_hash_key[username_length + 1];
-          else
-            pfs->m_hostname= NULL;
-          pfs->m_hostname_length= hostname_length;
-
-          pfs->m_user= find_or_create_user(thread, username, username_length);
-          pfs->m_host= find_or_create_host(thread, hostname, hostname_length);
-
-          pfs->init_refcount();
-          pfs->reset_stats();
-          pfs->m_disconnected_count= 0;
-
-          if (username_length > 0 && hostname_length > 0)
-          {
-            lookup_setup_actor(thread, username, username_length, hostname, hostname_length,
-                               & pfs->m_enabled);
-          }
-          else
-            pfs->m_enabled= true;
+          lookup_setup_actor(thread, username, username_length, hostname, hostname_length,
+                             & pfs->m_enabled);
+        }
+        else
+          pfs->m_enabled= true;
 
-          int res;
-          res= lf_hash_insert(&account_hash, pins, &pfs);
-          if (likely(res == 0))
-          {
-            pfs->m_lock.dirty_to_allocated();
-            return pfs;
-          }
+        int res;
+        res= lf_hash_insert(&account_hash, pins, &pfs);
+        if (likely(res == 0))
+        {
+          pfs->m_lock.dirty_to_allocated();
+          return pfs;
+        }
 
-          if (pfs->m_user)
-          {
-            pfs->m_user->release();
-            pfs->m_user= NULL;
-          }
-          if (pfs->m_host)
-          {
-            pfs->m_host->release();
-            pfs->m_host= NULL;
-          }
+        if (pfs->m_user)
+        {
+          pfs->m_user->release();
+          pfs->m_user= NULL;
+        }
+        if (pfs->m_host)
+        {
+          pfs->m_host->release();
+          pfs->m_host= NULL;
+        }
 
-          pfs->m_lock.dirty_to_free();
+        pfs->m_lock.dirty_to_free();
 
-          if (res > 0)
+        if (res > 0)
+        {
+          if (++retry_count > retry_max)
           {
-            if (++retry_count > retry_max)
-            {
-              account_lost++;
-              return NULL;
-            }
-            goto search;
+            account_lost++;
+            return NULL;
           }
-
-          account_lost++;
-          return NULL;
+          goto search;
         }
+
+        account_lost++;
+        return NULL;
       }
     }
   }
 
   account_lost++;
+  account_full= true;
   return NULL;
 }
 
@@ -651,6 +652,7 @@ void purge_account(PFS_thread *thread, PFS_account *account)
         account->m_host= NULL;
       }
       account->m_lock.allocated_to_free();
+      account_full= false;
     }
   }
 

storage/perfschema/pfs_digest.cc

@@ -69,7 +69,8 @@ bool flag_statements_digest= true;
   Current index in Stat array where new record is to be inserted.
   index 0 is reserved for "all else" case when entire array is full.
 */
-volatile uint32 digest_index= 1;
+static PFS_ALIGNED PFS_cacheline_uint32 digest_index;
+bool digest_full= false;
 
 LF_HASH digest_hash;
 static bool digest_hash_inited= false;
@@ -88,6 +89,8 @@ int init_digest(const PFS_global_param *param)
   */
   digest_max= param->m_digest_sizing;
   digest_lost= 0;
+  digest_index.m_u32= 1;
+  digest_full= false;
 
   if (digest_max == 0)
     return 0;
@@ -228,10 +231,9 @@ find_or_create_digest(PFS_thread *thread,
 
   lf_hash_search_unpin(pins);
 
-  /* Dirty read of digest_index */
-  if (digest_index == 0)
+  if (digest_full)
   {
-    /*  digest_stat array is full. Add stat at index 0 and return. */
+    /* digest_stat array is full. Add stat at index 0 and return. */
     pfs= &statements_digest_stat_array[0];
 
     if (pfs->m_first_seen == 0)
@@ -240,11 +242,11 @@ find_or_create_digest(PFS_thread *thread,
     return & pfs->m_stat;
   }
 
-  safe_index= PFS_atomic::add_u32(& digest_index, 1);
+  safe_index= PFS_atomic::add_u32(& digest_index.m_u32, 1);
   if (safe_index >= digest_max)
   {
     /* The digest array is now full. */
-    digest_index= 0;
+    digest_full= true;
     pfs= &statements_digest_stat_array[0];
 
     if (pfs->m_first_seen == 0)
@@ -352,7 +354,8 @@ void reset_esms_by_digest()
     Reset index which indicates where the next calculated digest information
     to be inserted in statements_digest_stat_array.
   */
-  digest_index= 1;
+  digest_index.m_u32= 1;
+  digest_full= false;
 }
 
 /*

storage/perfschema/pfs_events_stages.cc

@@ -41,7 +41,7 @@ PFS_ALIGNED bool flag_events_stages_history_long= false;
 /** True if EVENTS_STAGES_HISTORY_LONG circular buffer is full. */
 PFS_ALIGNED bool events_stages_history_long_full= false;
 /** Index in EVENTS_STAGES_HISTORY_LONG circular buffer. */
-PFS_ALIGNED volatile uint32 events_stages_history_long_index= 0;
+PFS_ALIGNED PFS_cacheline_uint32 events_stages_history_long_index;
 /** EVENTS_STAGES_HISTORY_LONG circular buffer. */
 PFS_ALIGNED PFS_events_stages *events_stages_history_long_array= NULL;
 
@@ -53,7 +53,7 @@ int init_events_stages_history_long(uint events_stages_history_long_sizing)
 {
   events_stages_history_long_size= events_stages_history_long_sizing;
   events_stages_history_long_full= false;
-  PFS_atomic::store_u32(&events_stages_history_long_index, 0);
+  PFS_atomic::store_u32(&events_stages_history_long_index.m_u32, 0);
 
   if (events_stages_history_long_size == 0)
     return 0;
@@ -122,7 +122,7 @@ void insert_events_stages_history_long(PFS_events_stages *stage)
 
   DBUG_ASSERT(events_stages_history_long_array != NULL);
 
-  uint index= PFS_atomic::add_u32(&events_stages_history_long_index, 1);
+  uint index= PFS_atomic::add_u32(&events_stages_history_long_index.m_u32, 1);
 
   index= index % events_stages_history_long_size;
   if (index == 0)
@@ -165,7 +165,7 @@ void reset_events_stages_history(void)
 /** Reset table EVENTS_STAGES_HISTORY_LONG data. */
 void reset_events_stages_history_long(void)
 {
-  PFS_atomic::store_u32(&events_stages_history_long_index, 0);
+  PFS_atomic::store_u32(&events_stages_history_long_index.m_u32, 0);
   events_stages_history_long_full= false;
 
   PFS_events_stages *pfs= events_stages_history_long_array;

storage/perfschema/pfs_events_stages.h

@@ -1,4 +1,4 @@
-/* Copyright (c) 2010, 2011, Oracle and/or its affiliates. All rights reserved.
+/* Copyright (c) 2010, 2013, Oracle and/or its affiliates. All rights reserved.
 
   This program is free software; you can redistribute it and/or modify
   it under the terms of the GNU General Public License as published by
@@ -42,7 +42,7 @@ extern bool flag_events_stages_history;
 extern bool flag_events_stages_history_long;
 
 extern bool events_stages_history_long_full;
-extern volatile uint32 events_stages_history_long_index;
+extern PFS_ALIGNED PFS_cacheline_uint32 events_stages_history_long_index;
 extern PFS_events_stages *events_stages_history_long_array;
 extern ulong events_stages_history_long_size;
 

storage/perfschema/pfs_events_statements.cc

@@ -41,7 +41,7 @@ PFS_ALIGNED bool flag_events_statements_history_long= false;
 /** True if EVENTS_STATEMENTS_HISTORY_LONG circular buffer is full. */
 PFS_ALIGNED bool events_statements_history_long_full= false;
 /** Index in EVENTS_STATEMENTS_HISTORY_LONG circular buffer. */
-PFS_ALIGNED volatile uint32 events_statements_history_long_index= 0;
+PFS_ALIGNED PFS_cacheline_uint32 events_statements_history_long_index;
 /** EVENTS_STATEMENTS_HISTORY_LONG circular buffer. */
 PFS_ALIGNED PFS_events_statements *events_statements_history_long_array= NULL;
 
@@ -53,7 +53,7 @@ int init_events_statements_history_long(uint events_statements_history_long_sizi
 {
   events_statements_history_long_size= events_statements_history_long_sizing;
   events_statements_history_long_full= false;
-  PFS_atomic::store_u32(&events_statements_history_long_index, 0);
+  PFS_atomic::store_u32(&events_statements_history_long_index.m_u32, 0);
 
   if (events_statements_history_long_size == 0)
     return 0;
@@ -122,7 +122,7 @@ void insert_events_statements_history_long(PFS_events_statements *statement)
 
   DBUG_ASSERT(events_statements_history_long_array != NULL);
 
-  uint index= PFS_atomic::add_u32(&events_statements_history_long_index, 1);
+  uint index= PFS_atomic::add_u32(&events_statements_history_long_index.m_u32, 1);
 
   index= index % events_statements_history_long_size;
   if (index == 0)
@@ -169,7 +169,7 @@ void reset_events_statements_history(void)
 /** Reset table EVENTS_STATEMENTS_HISTORY_LONG data. */
 void reset_events_statements_history_long(void)
 {
-  PFS_atomic::store_u32(&events_statements_history_long_index, 0);
+  PFS_atomic::store_u32(&events_statements_history_long_index.m_u32, 0);
   events_statements_history_long_full= false;
 
   PFS_events_statements *pfs= events_statements_history_long_array;

storage/perfschema/pfs_events_statements.h

@@ -108,7 +108,7 @@ extern bool flag_events_statements_history;
 extern bool flag_events_statements_history_long;
 
 extern bool events_statements_history_long_full;
-extern volatile uint32 events_statements_history_long_index;
+extern PFS_ALIGNED PFS_cacheline_uint32 events_statements_history_long_index;
 extern PFS_events_statements *events_statements_history_long_array;
 extern ulong events_statements_history_long_size;