Bug#13862186 - PERFORMANCE SCHEMA OVERHEAD FOR TABLE AGGREGATES

Before this fix, the performance schema would use a lot of CPU
to execute PFS_table aggregate functions in general.

With this fix, execution has been improved, per the following changes:

1)

In PFS_table::aggregate(),
the code used to call safe_aggregate() if any stat was present.

When both m_has_io_stats and m_has_lock_stats are true, this is correct.

When only m_has_io_stats or m_has_lock_stats is true, this is overkill.
Calling safe_aggregate_io() or safe_aggregate_lock() is better in this case.

2)

Likewise for PFS_table::sanitized_aggregate()

3)

In PFS_table_stat::aggregate_io() and PFS_table_stat::sum_io(),
the code did loop on every possible index in the [0, MAX_INDEXES-1] range,
regardless on whether the table actually had that many index used.

Now the code only use index statistics for index that do exist,
using the [0, key_count] range.

Calling code has been adjusted accordingly.

4)

For a given index statistic, PFS_table_io_stat::m_has_data was added.

This boolean indicates of data has been collected for this index.

Using this flag saves a lot, because even if a table can effectively have
many indexes, a thread opening the table, performing a select, and closing
the table is more likely to use one index only (or a few), instead of all the indexes
available.

When aggregating the statistics for this thread to the table statistics,
testing this flag saves on CPU.

5)

For code clarity only (no performance change),
MAX_KEY has been replaced by MAX_INDEXES, which is more accurate.
These two constants are synonym.

Author: marcalff

storage/perfschema/pfs.cc

@@ -3682,23 +3682,27 @@ static void end_table_io_wait_v1(PSI_table_locker* locker)
   DBUG_ASSERT(table != NULL);
 
   PFS_single_stat *stat;
+  PFS_table_io_stat *table_io_stat;
 
   DBUG_ASSERT((state->m_index < table->m_share->m_key_count) ||
-              (state->m_index == MAX_KEY));
+              (state->m_index == MAX_INDEXES));
+
+  table_io_stat= & table->m_table_stat.m_index_stat[state->m_index];
+  table_io_stat->m_has_data= true;
 
   switch (state->m_io_operation)
   {
   case PSI_TABLE_FETCH_ROW:
-    stat= & table->m_table_stat.m_index_stat[state->m_index].m_fetch;
+    stat= & table_io_stat->m_fetch;
     break;
   case PSI_TABLE_WRITE_ROW:
-    stat= & table->m_table_stat.m_index_stat[state->m_index].m_insert;
+    stat= & table_io_stat->m_insert;
     break;
   case PSI_TABLE_UPDATE_ROW:
-    stat= & table->m_table_stat.m_index_stat[state->m_index].m_update;
+    stat= & table_io_stat->m_update;
     break;
   case PSI_TABLE_DELETE_ROW:
-    stat= & table->m_table_stat.m_index_stat[state->m_index].m_delete;
+    stat= & table_io_stat->m_delete;
     break;
   default:
     DBUG_ASSERT(false);

storage/perfschema/pfs_instr.cc

@@ -1329,12 +1329,24 @@ void PFS_table::sanitized_aggregate(void)
   */
   PFS_table_share *safe_share= sanitize_table_share(m_share);
   PFS_thread *safe_thread= sanitize_thread(m_thread_owner);
-  if ((safe_share != NULL && safe_thread != NULL) &&
-      (m_has_io_stats || m_has_lock_stats))
+  if ((safe_share != NULL && safe_thread != NULL))
   {
-    safe_aggregate(& m_table_stat, safe_share, safe_thread);
-    m_has_io_stats= false;
-    m_has_lock_stats= false;
+    if (m_has_io_stats && m_has_lock_stats)
+    {
+      safe_aggregate(& m_table_stat, safe_share, safe_thread);
+      m_has_io_stats= false;
+      m_has_lock_stats= false;
+    }
+    else if (m_has_io_stats)
+    {
+      safe_aggregate_io(& m_table_stat, safe_share, safe_thread);
+      m_has_io_stats= false;
+    }
+    else if (m_has_lock_stats)
+    {
+      safe_aggregate_lock(& m_table_stat, safe_share, safe_thread);
+      m_has_lock_stats= false;
+    }
   }
 }
 
@@ -1368,6 +1380,8 @@ void PFS_table::safe_aggregate(PFS_table_stat *table_stat,
   DBUG_ASSERT(table_share != NULL);
   DBUG_ASSERT(thread != NULL);
 
+  uint key_count= sanitize_index_count(table_share->m_key_count);
+
   if (flag_thread_instrumentation && thread->m_enabled)
   {
     PFS_single_stat *event_name_array;
@@ -1379,7 +1393,7 @@ void PFS_table::safe_aggregate(PFS_table_stat *table_stat,
       (for wait/io/table/sql/handler)
     */
     index= global_table_io_class.m_event_name_index;
-    table_stat->sum_io(& event_name_array[index]);
+    table_stat->sum_io(& event_name_array[index], key_count);
 
     /*
       Aggregate to EVENTS_WAITS_SUMMARY_BY_THREAD_BY_EVENT_NAME
@@ -1390,7 +1404,7 @@ void PFS_table::safe_aggregate(PFS_table_stat *table_stat,
   }
 
   /* Aggregate to TABLE_IO_SUMMARY, TABLE_LOCK_SUMMARY */
-  table_share->m_table_stat.aggregate(table_stat);
+  table_share->m_table_stat.aggregate(table_stat, key_count);
   table_stat->fast_reset();
 }
 
@@ -1402,6 +1416,8 @@ void PFS_table::safe_aggregate_io(PFS_table_stat *table_stat,
   DBUG_ASSERT(table_share != NULL);
   DBUG_ASSERT(thread != NULL);
 
+  uint key_count= sanitize_index_count(table_share->m_key_count);
+
   if (flag_thread_instrumentation && thread->m_enabled)
   {
     PFS_single_stat *event_name_array;
@@ -1413,11 +1429,11 @@ void PFS_table::safe_aggregate_io(PFS_table_stat *table_stat,
       (for wait/io/table/sql/handler)
     */
     index= global_table_io_class.m_event_name_index;
-    table_stat->sum_io(& event_name_array[index]);
+    table_stat->sum_io(& event_name_array[index], key_count);
   }
 
   /* Aggregate to TABLE_IO_SUMMARY */
-  table_share->m_table_stat.aggregate_io(table_stat);
+  table_share->m_table_stat.aggregate_io(table_stat, key_count);
   table_stat->fast_reset_io();
 }
 

storage/perfschema/pfs_instr.h

@@ -196,11 +196,24 @@ struct PFS_table
   */
   void aggregate(void)
   {
-    if (likely((m_thread_owner != NULL) && (m_has_io_stats || m_has_lock_stats)))
+    if (likely(m_thread_owner != NULL))
     {
-      safe_aggregate(& m_table_stat, m_share, m_thread_owner);
-      m_has_io_stats= false;
-      m_has_lock_stats= false;
+      if (m_has_io_stats && m_has_lock_stats)
+      {
+        safe_aggregate(& m_table_stat, m_share, m_thread_owner);
+        m_has_io_stats= false;
+        m_has_lock_stats= false;
+      }
+      else if (m_has_io_stats)
+      {
+        safe_aggregate_io(& m_table_stat, m_share, m_thread_owner);
+        m_has_io_stats= false;
+      }
+      else if (m_has_lock_stats)
+      {
+        safe_aggregate_lock(& m_table_stat, m_share, m_thread_owner);
+        m_has_lock_stats= false;
+      }
     }
   }
 

storage/perfschema/pfs_instr_class.cc

@@ -1193,7 +1193,7 @@ static void set_keys(PFS_table_share *pfs, const TABLE_SHARE *share)
     pfs_key->m_name_length= len;
   }
 
-  pfs_key_last= pfs->m_keys + MAX_KEY;
+  pfs_key_last= pfs->m_keys + MAX_INDEXES;
   for ( ; pfs_key < pfs_key_last; pfs_key++)
     pfs_key->m_name_length= 0;
 }
@@ -1353,9 +1353,10 @@ PFS_table_share* find_or_create_table_share(PFS_thread *thread,
 
 void PFS_table_share::aggregate_io(void)
 {
+  uint safe_key_count= sanitize_index_count(m_key_count);
   uint index= global_table_io_class.m_event_name_index;
   PFS_single_stat *table_io_total= & global_instr_class_waits_array[index];
-  m_table_stat.sum_io(table_io_total);
+  m_table_stat.sum_io(table_io_total, safe_key_count);
   m_table_stat.fast_reset_io();
 }
 

storage/perfschema/pfs_instr_class.h

@@ -316,13 +316,20 @@ struct PFS_table_share
   /** Table statistics. */
   PFS_table_stat m_table_stat;
   /** Index names. */
-  PFS_table_key m_keys[MAX_KEY];
+  PFS_table_key m_keys[MAX_INDEXES];
 
 private:
   /** Number of opened table handles. */
   int m_refcount;
 };
 
+inline uint sanitize_index_count(uint count)
+{
+  if (likely(count <= MAX_INDEXES))
+    return count;
+  return 0;
+}
+
 /**
   Instrument controlling all table io.
   This instrument is used with table SETUP_OBJECTS.

storage/perfschema/pfs_stat.h

@@ -329,6 +329,7 @@ struct PFS_statement_stat
 /** Single table io statistic. */
 struct PFS_table_io_stat
 {
+  bool m_has_data;
   /** FETCH statistics */
   PFS_single_stat m_fetch;
   /** INSERT statistics */
@@ -338,8 +339,14 @@ struct PFS_table_io_stat
   /** DELETE statistics */
   PFS_single_stat m_delete;
 
+  PFS_table_io_stat()
+  {
+    m_has_data= false;
+  }
+
   inline void reset(void)
   {
+    m_has_data= false;
     m_fetch.reset();
     m_insert.reset();
     m_update.reset();
@@ -348,18 +355,25 @@ struct PFS_table_io_stat
 
   inline void aggregate(const PFS_table_io_stat *stat)
   {
-    m_fetch.aggregate(&stat->m_fetch);
-    m_insert.aggregate(&stat->m_insert);
-    m_update.aggregate(&stat->m_update);
-    m_delete.aggregate(&stat->m_delete);
+    if (stat->m_has_data)
+    {
+      m_has_data= true;
+      m_fetch.aggregate(&stat->m_fetch);
+      m_insert.aggregate(&stat->m_insert);
+      m_update.aggregate(&stat->m_update);
+      m_delete.aggregate(&stat->m_delete);
+    }
   }
 
   inline void sum(PFS_single_stat *result)
   {
-    result->aggregate(& m_fetch);
-    result->aggregate(& m_insert);
-    result->aggregate(& m_update);
-    result->aggregate(& m_delete);
+    if (m_has_data)
+    {
+      result->aggregate(& m_fetch);
+      result->aggregate(& m_insert);
+      result->aggregate(& m_update);
+      result->aggregate(& m_delete);
+    }
   }
 };
 
@@ -419,10 +433,10 @@ struct PFS_table_stat
 {
   /**
     Statistics, per index.
-    Each index stat is in [0, MAX_KEY-1],
-    stats when using no index are in [MAX_KEY].
+    Each index stat is in [0, MAX_INDEXES-1],
+    stats when using no index are in [MAX_INDEXES].
   */
-  PFS_table_io_stat m_index_stat[MAX_KEY + 1];
+  PFS_table_io_stat m_index_stat[MAX_INDEXES + 1];
 
   /**
     Statistics, per lock type.
@@ -433,7 +447,7 @@ struct PFS_table_stat
   inline void reset_io(void)
   {
     PFS_table_io_stat *stat= & m_index_stat[0];
-    PFS_table_io_stat *stat_last= & m_index_stat[MAX_KEY + 1];
+    PFS_table_io_stat *stat_last= & m_index_stat[MAX_INDEXES + 1];
     for ( ; stat < stat_last ; stat++)
       stat->reset();
   }
@@ -466,42 +480,63 @@ struct PFS_table_stat
     memcpy(this, & g_reset_template, sizeof(*this));
   }
 
-  inline void aggregate_io(const PFS_table_stat *stat)
+  inline void aggregate_io(const PFS_table_stat *stat, uint key_count)
   {
-    PFS_table_io_stat *to_stat= & m_index_stat[0];
-    PFS_table_io_stat *to_stat_last= & m_index_stat[MAX_KEY + 1];
-    const PFS_table_io_stat *from_stat= & stat->m_index_stat[0];
+    PFS_table_io_stat *to_stat;
+    PFS_table_io_stat *to_stat_last;
+    const PFS_table_io_stat *from_stat;
+
+    DBUG_ASSERT(key_count <= MAX_INDEXES);
+
+    /* Aggregate stats for each index, if any */
+    to_stat= & m_index_stat[0];
+    to_stat_last= to_stat + key_count;
+    from_stat= & stat->m_index_stat[0];
     for ( ; to_stat < to_stat_last ; from_stat++, to_stat++)
       to_stat->aggregate(from_stat);
+
+    /* Aggregate stats for the table */
+    to_stat= & m_index_stat[MAX_INDEXES];
+    from_stat= & stat->m_index_stat[MAX_INDEXES];
+    to_stat->aggregate(from_stat);
   }
 
   inline void aggregate_lock(const PFS_table_stat *stat)
   {
     m_lock_stat.aggregate(& stat->m_lock_stat);
   }
 
-  inline void aggregate(const PFS_table_stat *stat)
+  inline void aggregate(const PFS_table_stat *stat, uint key_count)
   {
-    aggregate_io(stat);
+    aggregate_io(stat, key_count);
     aggregate_lock(stat);
   }
 
-  inline void sum_io(PFS_single_stat *result)
+  inline void sum_io(PFS_single_stat *result, uint key_count)
   {
-    PFS_table_io_stat *stat= & m_index_stat[0];
-    PFS_table_io_stat *stat_last= & m_index_stat[MAX_KEY + 1];
+    PFS_table_io_stat *stat;
+    PFS_table_io_stat *stat_last;
+
+    DBUG_ASSERT(key_count <= MAX_INDEXES);
+
+    /* Sum stats for each index, if any */
+    stat= & m_index_stat[0];
+    stat_last= stat + key_count;
     for ( ; stat < stat_last ; stat++)
       stat->sum(result);
+
+    /* Sum stats for the table */
+    m_index_stat[MAX_INDEXES].sum(result);
   }
 
   inline void sum_lock(PFS_single_stat *result)
   {
     m_lock_stat.sum(result);
   }
 
-  inline void sum(PFS_single_stat *result)
+  inline void sum(PFS_single_stat *result, uint key_count)
   {
-    sum_io(result);
+    sum_io(result, key_count);
     sum_lock(result);
   }