bzr push into mysql-trunk branch (marc.alff:3414 to 3415) Bug#12993572

[Marc Alff]

 3415 Marc Alff	2011-09-26
      BUG#12993572 - P_S STANDBY MODE CAUSES PERFORMANCE DROP
      
      Backport for mysql-5.6.3-m4-release
      
      This fix is a performance improvement.
      
      Before this fix, the performance schema unbind_table() and rebind_table()
      api were called from sql/sql_base.cc while holding LOCK_open.
      
      As the implementation of these api can involve updating table statistics
      for the current table handle, these calls did cause significant bottlenecks
      for LOCK_open.
      
      This fix:
      
      - moved the call to unbind_table() before adding a table handle to the table
        definition cache (which uses LOCK_open), so the code is outside the
        critical section.
      
      - moved the call to rebind_table() after a free handle has been reused from
        the table definition cache. This is now also outside of the LOCK_open critical
        section.
      
      - The implementation of PFS_table::aggregate(), which is part of
        unbind_table(), has been optimized to be simpler when there are no
        statistics to update. Given that table lock statistics are collected for
        11 different types of lock (see PFS_TL_LOCK_TYPE), and that table io stats
        are collected for 4 types (fetch, insert, update, delete), per table and
        per index, this decreases the overhead when the table instrumentation is
        not used.

    modified:
      sql/sql_base.cc
      storage/perfschema/pfs.cc
      storage/perfschema/pfs_instr.cc
      storage/perfschema/pfs_instr.h
 3414 Guilhem Bichot	2011-09-23
      Fix for Bug#13009176 "USE DATABASE BREAKS OPTIMIZER TRACE IF THERE IS A VIEW IN THE SCHEMA"
      COM_FIELD_LIST opened a view when opt-trace-specific security checks could not be run, which,
      as a security defense, caused tracing to be disabled for the entire session. See sql_parse.cc.
     @ sql/opt_trace2server.cc
        updating comment and assertions.
     @ sql/sql_parse.cc
        See the long comment in opt_trace_disable_if_no_security_context_access().
        No testcase, because to provoke the bug we need the 'mysql' command-line client to
        build its "completion hash". It doesn't do that when 'mysql' is run either with
        option --execute or when stdin is redirected from a file (is not a terminal)
        (both cases causes status.batch to be 1, which shortcuts build_completion_hash()).
        Running 'mysql' in the mtr suite is done with one of these two ways, so we cannot
        add a test. But I tested manually.

    modified:
      sql/opt_trace2server.cc
      sql/sql_parse.cc
=== modified file 'sql/sql_base.cc'
--- a/sql/sql_base.cc	2011-08-19 13:04:28 +0000
+++ b/sql/sql_base.cc	2011-09-26 14:07:09 +0000
@@ -542,8 +542,6 @@ static void table_def_use_table(THD *thd
   DBUG_ASSERT(table->db_stat && table->file);
   /* The children must be detached from the table. */
   DBUG_ASSERT(! table->file->extra(HA_EXTRA_IS_ATTACHED_CHILDREN));
-
-  table->file->rebind_psi();
 }
 
 
@@ -560,7 +558,6 @@ static void table_def_unuse_table(TABLE
   DBUG_ASSERT(! table->s->has_old_version());
 
   table->in_use= 0;
-  table->file->unbind_psi();
 
   /* Remove table from the list of tables used in this share. */
   table->s->used_tables.remove(table);
@@ -1655,6 +1652,10 @@ bool close_thread_table(THD *thd, TABLE
     table->file->ha_reset();
   }
 
+  /* Do this *before* entering the LOCK_open critical section. */
+  if (table->file != NULL)
+    table->file->unbind_psi();
+
   mysql_mutex_lock(&LOCK_open);
 
   if (table->s->has_old_version() || table->needs_reopen() ||
@@ -2731,6 +2732,8 @@ bool open_table(THD *thd, TABLE_LIST *ta
   int error;
   TABLE_SHARE *share;
   my_hash_value_type hash_value;
+  bool recycled_free_table;
+
   DBUG_ENTER("open_table");
 
   /*
@@ -3088,6 +3091,7 @@ retry_share:
   {
     table= share->free_tables.front();
     table_def_use_table(thd, table);
+    recycled_free_table= true;
     /* We need to release share as we have EXTRA reference to it in our hands. */
     release_table_share(share);
   }
@@ -3099,6 +3103,7 @@ retry_share:
 
     mysql_mutex_unlock(&LOCK_open);
 
+    recycled_free_table= false;
     /* make a new table */
     if (!(table=(TABLE*) my_malloc(sizeof(*table),MYF(MY_WME))))
       goto err_lock;
@@ -3140,6 +3145,13 @@ retry_share:
 
   mysql_mutex_unlock(&LOCK_open);
 
+  /* Call rebind_psi outside of the LOCK_open critical section. */
+  if (recycled_free_table)
+  {
+    DBUG_ASSERT(table->file != NULL);
+    table->file->rebind_psi();
+  }
+
   table->mdl_ticket= mdl_ticket;
 
   table->next= thd->open_tables;		/* Link into simple list */

=== modified file 'storage/perfschema/pfs.cc'
--- a/storage/perfschema/pfs.cc	2011-08-18 16:15:02 +0000
+++ b/storage/perfschema/pfs.cc	2011-09-26 14:07:09 +0000
@@ -3710,6 +3710,8 @@ static void end_table_io_wait_v1(PSI_tab
       insert_events_waits_history_long(wait);
     thread->m_events_waits_count--;
   }
+
+  table->m_has_io_stats= true;
 }
 
 /**
@@ -3787,6 +3789,8 @@ static void end_table_lock_wait_v1(PSI_t
       insert_events_waits_history_long(wait);
     thread->m_events_waits_count--;
   }
+
+  table->m_has_lock_stats= true;
 }
 
 static void start_file_wait_v1(PSI_file_locker *locker,

=== modified file 'storage/perfschema/pfs_instr.cc'
--- a/storage/perfschema/pfs_instr.cc	2011-08-18 12:27:21 +0000
+++ b/storage/perfschema/pfs_instr.cc	2011-09-26 14:07:09 +0000
@@ -1306,6 +1306,8 @@ PFS_table* create_table(PFS_table_share
         pfs->m_lock_enabled= share->m_enabled &&
           flag_global_instrumentation && global_table_lock_class.m_enabled;
         pfs->m_lock_timed= share->m_timed && global_table_lock_class.m_timed;
+        pfs->m_has_io_stats= false;
+        pfs->m_has_lock_stats= false;
         share->inc_refcount();
         pfs->m_table_stat.reset();
         pfs->m_thread_owner= opening_thread;
@@ -1327,24 +1329,35 @@ 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)
+  if ((safe_share != NULL && safe_thread != NULL) &&
+      (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;
+  }
 }
 
 void PFS_table::sanitized_aggregate_io(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)
+  if (safe_share != NULL && safe_thread != NULL && m_has_io_stats)
+  {
     safe_aggregate_io(& m_table_stat, safe_share, safe_thread);
+    m_has_io_stats= false;
+  }
 }
 
 void PFS_table::sanitized_aggregate_lock(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)
+  if (safe_share != NULL && safe_thread != NULL && m_has_lock_stats)
+  {
     safe_aggregate_lock(& m_table_stat, safe_share, safe_thread);
+    m_has_lock_stats= false;
+  }
 }
 
 void PFS_table::safe_aggregate(PFS_table_stat *table_stat,
@@ -1361,11 +1374,17 @@ void PFS_table::safe_aggregate(PFS_table
     uint index;
     event_name_array= thread->m_instr_class_waits_stats;
 
-    /* Aggregate to EVENTS_WAITS_SUMMARY_BY_THREAD_BY_EVENT_NAME (for wait/io/table/sql/handler) */
+    /*
+      Aggregate to EVENTS_WAITS_SUMMARY_BY_THREAD_BY_EVENT_NAME
+      (for wait/io/table/sql/handler)
+    */
     index= global_table_io_class.m_event_name_index;
     table_stat->sum_io(& event_name_array[index]);
 
-    /* Aggregate to EVENTS_WAITS_SUMMARY_BY_THREAD_BY_EVENT_NAME (for wait/lock/table/sql/handler) */
+    /*
+      Aggregate to EVENTS_WAITS_SUMMARY_BY_THREAD_BY_EVENT_NAME
+      (for wait/lock/table/sql/handler)
+    */
     index= global_table_lock_class.m_event_name_index;
     table_stat->sum_lock(& event_name_array[index]);
   }
@@ -1389,7 +1408,10 @@ void PFS_table::safe_aggregate_io(PFS_ta
     uint index;
     event_name_array= thread->m_instr_class_waits_stats;
 
-    /* Aggregate to EVENTS_WAITS_SUMMARY_BY_THREAD_BY_EVENT_NAME (for wait/io/table/sql/handler) */
+    /*
+      Aggregate to EVENTS_WAITS_SUMMARY_BY_THREAD_BY_EVENT_NAME
+      (for wait/io/table/sql/handler)
+    */
     index= global_table_io_class.m_event_name_index;
     table_stat->sum_io(& event_name_array[index]);
   }
@@ -1413,7 +1435,10 @@ void PFS_table::safe_aggregate_lock(PFS_
     uint index;
     event_name_array= thread->m_instr_class_waits_stats;
 
-    /* Aggregate to EVENTS_WAITS_SUMMARY_BY_THREAD_BY_EVENT_NAME (for wait/lock/table/sql/handler) */
+    /*
+      Aggregate to EVENTS_WAITS_SUMMARY_BY_THREAD_BY_EVENT_NAME
+      (for wait/lock/table/sql/handler)
+    */
     index= global_table_lock_class.m_event_name_index;
     table_stat->sum_lock(& event_name_array[index]);
   }

=== modified file 'storage/perfschema/pfs_instr.h'
--- a/storage/perfschema/pfs_instr.h	2011-08-18 12:27:21 +0000
+++ b/storage/perfschema/pfs_instr.h	2011-09-26 14:07:09 +0000
@@ -180,6 +180,12 @@ struct PFS_table
   */
   bool m_lock_timed;
 
+  /** True if table io statistics have been collected. */
+  bool m_has_io_stats;
+
+  /** True if table lock statistics have been collected. */
+  bool m_has_lock_stats;
+
 public:
   /**
     Aggregate this table handle statistics to the parents.
@@ -188,8 +194,12 @@ public:
   */
   void aggregate(void)
   {
-    if (likely(m_thread_owner != NULL))
+    if (likely((m_thread_owner != NULL) && (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;
+    }
   }
 
   /**

No bundle (reason: useless for push emails).