From: Chad MILLER Date: March 12 2007 12:36am Subject: bk commit into 5.0 tree (cmiller:1.2424) BUG#24795 List-Archive: http://lists.mysql.com/commits/21694 X-Bug: 24795 Message-Id: <20070312003643.66A138304C@zippy> 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 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 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 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 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 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 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_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 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 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 entries; + Queue 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 history; + Queue history; query_id_t next_profile_id() { return(profile_id_counter++); }