List:Commits« Previous MessageNext Message »
From:Chad MILLER Date:March 12 2007 12:36am
Subject:bk commit into 5.0 tree (cmiller:1.2424) BUG#24795
View as plain text  
Below is the list of changes that have just been committed into a local
5.0 repository of cmiller. When cmiller does a push these changes will
be propagated to the main repository and, within 24 hours after the
push, to the public repository.
For information on how to access the public repository
see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html

ChangeSet@stripped, 2007-03-11 20:36:38-04:00, cmiller@stripped +2 -0
  Bug#24795: SHOW PROFILE implementation
  
  Don't use memory roots to store profiling information, because
  memory roots make freeing the data a no-op, and thus long-running
  processes with profiling turned on the whole time could eventually 
  use all available memory.
  
  Instead, use regular heap allocation and deallocation calls to 
  manage profiling data.  Replace the leaky List usage with a similar-
  behaving structure named "Queue".

  sql/sql_profile.cc@stripped, 2007-03-11 20:36:37-04:00, cmiller@stripped +39 -72
    Don't use C++ iterators on our simple Queue implementation.  They're
    not implemented and we don't really need them.
    
    Rip out idea of swapping out the thd's mem_root.

  sql/sql_profile.h@stripped, 2007-03-11 20:36:37-04:00, cmiller@stripped +108 -3
    Rip out idea of needing a mem_root.
    
    Implement a Queue that looks and behaves very similarly to memroot-
    using List.

# This is a BitKeeper patch.  What follows are the unified diffs for the
# set of deltas contained in the patch.  The rest of the patch, the part
# that BitKeeper cares about, is below these diffs.
# User:	cmiller
# Host:	zippy.cornsilk.net
# Root:	/home/cmiller/work/mysql/mysql-5.0-comeng--bug24795

--- 1.4/sql/sql_profile.cc	2007-03-11 20:36:43 -04:00
+++ 1.5/sql/sql_profile.cc	2007-03-11 20:36:43 -04:00
@@ -175,11 +175,8 @@ void QUERY_PROFILE::set_query_source(cha
 
 QUERY_PROFILE::~QUERY_PROFILE()
 {
-  PROFILE_ENTRY *entry;
-  List_iterator<PROFILE_ENTRY> it(entries);
-  while ((entry= it++) != NULL)
-    delete entry;
-  entries.empty();
+  while (! entries.is_empty())
+    delete entries.pop();
 
   if (query_source != NULL)
     my_free(query_source, MYF(0));
@@ -191,7 +188,6 @@ void QUERY_PROFILE::status(const char *s
 {
   THD *thd= profiling->thd;
   PROFILE_ENTRY *prof;
-  MEM_ROOT *saved_mem_root;
   DBUG_ENTER("QUERY_PROFILE::status");
 
   /* Blank status.  Just return, and thd->proc_info will be set blank later. */
@@ -210,22 +206,6 @@ void QUERY_PROFILE::status(const char *s
   if (unlikely((thd->query_id != server_query_id) && !thd->spcont))
     reset();
     
-  /*
-    In order to keep the profile information between queries (i.e. from
-    SELECT to the following SHOW PROFILE command) the following code is
-    necessary to keep the profile from getting freed automatically when
-    mysqld cleans up after the query.
-
-    The "entries" list allocates is memory from the current thd's mem_root.
-    We substitute our mem_root temporarily so that we intercept those
-    allocations into our own mem_root.
-    
-    The thd->mem_root structure is freed after each query is completed,
-    so temporarily override it.
-  */
-  saved_mem_root= thd->mem_root;
-  thd->mem_root= &profiling->mem_root;
-
   if (function_arg && file_arg) 
   {
     if ((profile_end= prof= new PROFILE_ENTRY(this, status_arg, function_arg, 
@@ -238,9 +218,6 @@ void QUERY_PROFILE::status(const char *s
       entries.push_back(prof);
   }
 
-  /* Restore mem_root */
-  thd->mem_root= saved_mem_root;
-
   DBUG_VOID_RETURN;
 }
 
@@ -252,11 +229,8 @@ void QUERY_PROFILE::reset()
     server_query_id= profiling->thd->query_id; /* despite name, is global */
     profile_start.collect();
 
-    PROFILE_ENTRY *entry;
-    List_iterator<PROFILE_ENTRY> it(entries);
-    while ((entry= it++) != NULL)
-      delete entry;
-    entries.empty();
+    while (! entries.is_empty())
+      delete entries.pop();
   }
   DBUG_VOID_RETURN;
 }
@@ -344,10 +318,14 @@ bool QUERY_PROFILE::show(uint options)
   struct rusage *last_rusage= &(profile_start.rusage);
 #endif
 
-  List_iterator<PROFILE_ENTRY> it(entries);
   PROFILE_ENTRY *entry;
-  while ((entry= it++) != NULL)
+  void *iterator;
+  for (iterator= entries.new_iterator(); 
+       iterator != NULL; 
+       iterator= entries.iterator_next(iterator))
   {
+    entry= entries.iterator_value(iterator);
+
 #ifdef HAVE_GETRUSAGE
     struct rusage *rusage= &(entry->rusage);
 #endif
@@ -463,24 +441,15 @@ bool QUERY_PROFILE::show(uint options)
 PROFILING::PROFILING()
   :profile_id_counter(0), keeping(1), current(NULL), last(NULL)
 {
-  init_sql_alloc(&mem_root,
-                 PROFILE_ALLOC_BLOCK_SIZE,
-                 PROFILE_ALLOC_PREALLOC_SIZE);
 }
 
 PROFILING::~PROFILING()
 {
-  QUERY_PROFILE *prof;
-
-  List_iterator<QUERY_PROFILE> it(history);
-  while ((prof= it++) != NULL)
-    delete prof;
-  history.empty();
+  while (! history.is_empty())
+    delete history.pop();
 
   if (current != NULL)
     delete current;
-
-  free_root(&mem_root, MYF(0));
 }
 
 void PROFILING::status_change(const char *status_arg,
@@ -505,7 +474,6 @@ void PROFILING::status_change(const char
 
 void PROFILING::store()
 {
-  MEM_ROOT *saved_mem_root;
   DBUG_ENTER("PROFILING::store");
 
   /* Already stored */
@@ -517,22 +485,8 @@ void PROFILING::store()
   }
 
   while (history.elements > thd->variables.profiling_history_size)
-  {
-    QUERY_PROFILE *tmp= history.pop();
-    delete tmp;
-  }
-
-  /* 
-    Switch out memory roots so that we're sure that we keep what we need 
-    
-    The "history" list implementation allocates its memory in the current 
-    thd's mem_root.  We substitute our mem_root temporarily so that we
-    intercept those allocations into our own mem_root.
-  */
+    delete history.pop();
 
-  saved_mem_root= thd->mem_root;
-  thd->mem_root= &mem_root;
-  
   if (current != NULL)
   {
     if (keeping && 
@@ -556,9 +510,6 @@ void PROFILING::store()
   DBUG_ASSERT(current == NULL);
   current= new QUERY_PROFILE(this, thd->query, thd->query_length);
 
-  /* Restore memory root */
-  thd->mem_root= saved_mem_root;
-
   DBUG_VOID_RETURN;
 }
 
@@ -598,9 +549,13 @@ bool PROFILING::show_profiles()
 
   unit->set_limit(sel);
   
-  List_iterator<QUERY_PROFILE> it(history);
-  while ((prof= it++) != NULL)
+  void *iterator;
+  for (iterator= history.new_iterator(); 
+       iterator != NULL; 
+       iterator= history.iterator_next(iterator))
   {
+    prof= history.iterator_value(iterator);
+
     String elapsed;
 
     PROFILE_ENTRY *ps= &prof->profile_start;
@@ -651,9 +606,13 @@ bool PROFILING::show(uint options, uint 
   DBUG_ENTER("PROFILING::show");
   QUERY_PROFILE *prof;
 
-  List_iterator<QUERY_PROFILE> it(history);
-  while ((prof= it++) != NULL)
+  void *iterator;
+  for (iterator= history.new_iterator(); 
+       iterator != NULL; 
+       iterator= history.iterator_next(iterator))
   {
+    prof= history.iterator_value(iterator);
+
     if(prof->profiling_query_id == profiling_query_id)
       DBUG_RETURN(prof->show(options));
   }
@@ -681,11 +640,15 @@ int PROFILING::fill_statistics_info(THD 
   TABLE *table= tables->table;
   ulonglong row_number= 0;
 
-  List_iterator<QUERY_PROFILE> query_it(history);
   QUERY_PROFILE *query;
   /* Go through each query in this thread's stored history... */
-  while ((query= query_it++) != NULL)
+  void *history_iterator;
+  for (history_iterator= history.new_iterator(); 
+       history_iterator != NULL; 
+       history_iterator= history.iterator_next(history_iterator))
   {
+    query= history.iterator_value(history_iterator);
+
     PROFILE_ENTRY *ps= &(query->profile_start);
     double last_time= ps->time_usecs;
 #ifdef HAVE_GETRUSAGE
@@ -699,16 +662,20 @@ int PROFILING::fill_statistics_info(THD 
     */
     ulonglong seq;
 
-    List_iterator<PROFILE_ENTRY> step_it(query->entries);
+    void *entry_iterator;
     PROFILE_ENTRY *entry;
     /* ...and for each query, go through all its state-change steps. */
-    for (seq= 0, entry= step_it++; 
-         entry != NULL; 
+    for (seq= 0, entry_iterator= query->entries.new_iterator(); 
+         entry_iterator != NULL; 
+         entry_iterator= query->entries.iterator_next(entry_iterator),
 #ifdef HAVE_GETRUSAGE
          last_rusage= &(entry->rusage),
 #endif
-         seq++, last_time= entry->time_usecs, entry= step_it++, row_number++)
+         seq++, last_time= entry->time_usecs, row_number++)
     {
+      entry= query->entries.iterator_value(entry_iterator);
+
+
       /* Set default values for this row. */
       restore_record(table, s->default_values);
 

--- 1.3/sql/sql_profile.h	2007-03-11 20:36:43 -04:00
+++ 1.4/sql/sql_profile.h	2007-03-11 20:36:43 -04:00
@@ -84,6 +84,112 @@ class PROFILING;
 
 
 /**
+  Implements a persistent FIFO using server List method names.  Not
+  thread-safe.  Intended to be used on thread-local data only.  
+*/
+template <class T> class Queue
+{
+private:
+
+  struct queue_item
+  {
+    T *payload;
+    struct queue_item *next, *previous;
+  };
+
+  struct queue_item *first, *last;
+
+public:
+  Queue()
+  {
+    elements= 0;
+    first= last= NULL;
+  }
+
+  void empty()
+  {
+    struct queue_item *i, *after_i;
+    for (i= first; i != NULL; i= after_i)
+    {
+      after_i= i->next;
+      my_free((char *) i, MYF(0));
+    }
+    elements= 0;
+  }
+
+  ulong elements;                       /* The count of items in the Queue */
+
+  void push_back(T *payload)
+  {
+    struct queue_item *new_item;
+
+    new_item= (struct queue_item *) my_malloc(sizeof(struct queue_item), MYF(0));
+
+    new_item->payload= payload;
+
+    if (first == NULL)
+      first= new_item;
+    if (last != NULL)
+    {
+      DBUG_ASSERT(last->next == NULL);
+      last->next= new_item;
+    }
+    new_item->previous= last;
+    new_item->next= NULL;
+    last= new_item;
+
+    elements++;
+  }
+
+  T *pop()
+  {
+    struct queue_item *old_item= first;
+    T *ret= NULL;
+
+    if (first == NULL)
+    {
+      DBUG_PRINT("warning", ("tried to pop nonexistent item from Queue"));
+      return NULL;
+    }
+
+    ret= old_item->payload;
+    if (first->next != NULL)
+      first->next->previous= NULL;
+    else
+      last= NULL;
+    first= first->next;
+
+    my_free((char *)old_item, MYF(0));
+    elements--;
+
+    return ret;
+  }
+
+  bool is_empty()
+  {
+    DBUG_ASSERT(((elements > 0) && (first != NULL)) || ((elements == 0) || (first == NULL)));
+    return (elements == 0);
+  }
+
+  void *new_iterator()
+  {
+    return first;
+  }
+
+  void *iterator_next(void *current)
+  {
+    return ((struct queue_item *) current)->next;
+  }
+
+  T *iterator_value(void *current)
+  {
+    return ((struct queue_item *) current)->payload;
+  }
+
+};
+
+
+/**
   A single entry in a single profile.
 */
 class PROFILE_ENTRY
@@ -135,7 +241,7 @@ private:
   char *query_source;
   PROFILE_ENTRY profile_start;
   PROFILE_ENTRY *profile_end;
-  List<PROFILE_ENTRY> entries;
+  Queue<PROFILE_ENTRY> entries;
 
 
   QUERY_PROFILE(PROFILING *profiling_arg, char *query_source_arg, uint query_length_arg);
@@ -169,13 +275,12 @@ private:
     Not the system query_id, but a counter unique to profiling. 
   */
   query_id_t profile_id_counter;     
-  MEM_ROOT mem_root;
   THD *thd;
   bool keeping;
 
   QUERY_PROFILE *current;
   QUERY_PROFILE *last;
-  List<QUERY_PROFILE> history;
+  Queue<QUERY_PROFILE> history;
  
   query_id_t next_profile_id() { return(profile_id_counter++); }
 
Thread
bk commit into 5.0 tree (cmiller:1.2424) BUG#24795Chad MILLER12 Mar