diff --git a/sql/sp_head.cc b/sql/sp_head.cc index 29cee6da4d3..825ec34e410 100644 --- a/sql/sp_head.cc +++ b/sql/sp_head.cc @@ -509,7 +509,7 @@ sp_head::destroy() delete i; delete_dynamic(&m_instr); m_pcont->destroy(); - free_items(free_list); + free_items(); /* If we have non-empty LEX stack then we just came out of parser with @@ -596,7 +596,6 @@ sp_head::execute(THD *thd) ctx->clear_handler(); thd->query_error= 0; old_arena= thd->current_arena; - thd->current_arena= this; /* We have to save/restore this info when we are changing call level to @@ -636,23 +635,18 @@ sp_head::execute(THD *thd) break; DBUG_PRINT("execute", ("Instruction %u", ip)); thd->set_time(); // Make current_time() et al work - { - /* - We have to substitute free_list of executing statement to - current_arena to store there all new items created during execution - (for example '*' expanding, or items made during permanent subquery - transformation) - Note: Every statement have to have all its items listed in free_list - for correct cleaning them up - */ - Item *save_free_list= thd->current_arena->free_list; - thd->current_arena->free_list= i->free_list; - ret= i->execute(thd, &ip); - i->free_list= thd->current_arena->free_list; - thd->current_arena->free_list= save_free_list; - } + /* + We have to set thd->current_arena before executing the instruction + to store in the instruction free_list all new items, created + during the first execution (for example expanding of '*' or the + items made during other permanent subquery transformations). + */ + thd->current_arena= i; + ret= i->execute(thd, &ip); if (i->free_list) cleanup_items(i->free_list); + i->state= Query_arena::EXECUTED; + // Check if an exception has occurred and a handler has been found // Note: We havo to check even if ret==0, since warnings (and some // errors don't return a non-zero value. @@ -694,7 +688,6 @@ sp_head::execute(THD *thd) DBUG_ASSERT(!thd->derived_tables); thd->derived_tables= old_derived_tables; - cleanup_items(thd->current_arena->free_list); thd->current_arena= old_arena; state= EXECUTED; @@ -728,8 +721,8 @@ sp_head::execute_function(THD *thd, Item **argp, uint argcount, Item **resp) sp_rcontext *nctx = NULL; uint i; int ret; - MEM_ROOT *old_mem_root, call_mem_root; - Item *old_free_list, *call_free_list; + MEM_ROOT call_mem_root; + Query_arena call_arena(&call_mem_root, INITIALIZED_FOR_SP), backup_arena; if (argcount != params) { @@ -741,14 +734,12 @@ sp_head::execute_function(THD *thd, Item **argp, uint argcount, Item **resp) } init_alloc_root(&call_mem_root, MEM_ROOT_BLOCK_SIZE, 0); - old_mem_root= thd->mem_root; - thd->mem_root= &call_mem_root; - old_free_list= thd->free_list; // Keep the old list - thd->free_list= NULL; // Start a new one + + thd->set_n_backup_item_arena(&call_arena, &backup_arena); // QQ Should have some error checking here? (types, etc...) nctx= new sp_rcontext(csize, hmax, cmax); - nctx->callers_mem_root= old_mem_root; + nctx->callers_mem_root= backup_arena.mem_root; for (i= 0 ; i < argcount ; i++) { sp_pvar_t *pvar = m_pcont->find_pvar(i); @@ -780,9 +771,7 @@ sp_head::execute_function(THD *thd, Item **argp, uint argcount, Item **resp) // Partially restore context now. // We still need the call mem root and free list for processing // of the result. - call_free_list= thd->free_list; - thd->free_list= old_free_list; - thd->mem_root= old_mem_root; + thd->restore_backup_item_arena(&call_arena, &backup_arena); if (m_type == TYPE_ENUM_FUNCTION && ret == 0) { @@ -802,8 +791,7 @@ sp_head::execute_function(THD *thd, Item **argp, uint argcount, Item **resp) thd->spcont= octx; // Now get rid of the rest of the callee context - cleanup_items(call_free_list); - free_items(call_free_list); + call_arena.free_items(); free_root(&call_mem_root, MYF(0)); DBUG_RETURN(ret); @@ -835,8 +823,8 @@ sp_head::execute_procedure(THD *thd, List *args) sp_rcontext *octx = thd->spcont; sp_rcontext *nctx = NULL; my_bool tmp_octx = FALSE; // True if we have allocated a temporary octx - MEM_ROOT *old_mem_root, call_mem_root; - Item *old_free_list, *call_free_list; + MEM_ROOT call_mem_root; + Query_arena call_arena(&call_mem_root, INITIALIZED_FOR_SP), backup_arena; if (args->elements != params) { @@ -846,10 +834,7 @@ sp_head::execute_procedure(THD *thd, List *args) } init_alloc_root(&call_mem_root, MEM_ROOT_BLOCK_SIZE, 0); - old_mem_root= thd->mem_root; - thd->mem_root= &call_mem_root; - old_free_list= thd->free_list; // Keep the old list - thd->free_list= NULL; // Start a new one + thd->set_n_backup_item_arena(&call_arena, &backup_arena); if (csize > 0 || hmax > 0 || cmax > 0) { @@ -919,9 +904,7 @@ sp_head::execute_procedure(THD *thd, List *args) // Partially restore context now. // We still need the call mem root and free list for processing // of out parameters. - call_free_list= thd->free_list; - thd->free_list= old_free_list; - thd->mem_root= old_mem_root; + thd->restore_backup_item_arena(&call_arena, &backup_arena); if (!ret && csize > 0) { @@ -996,8 +979,7 @@ sp_head::execute_procedure(THD *thd, List *args) thd->spcont= octx; // Now get rid of the rest of the callee context - cleanup_items(call_free_list); - free_items(call_free_list); + call_arena.free_items(); thd->lex->unit.cleanup(); free_root(&call_mem_root, MYF(0)); @@ -1291,6 +1273,13 @@ void sp_head::add_instr(sp_instr *instr) { instr->free_list= m_thd->free_list; m_thd->free_list= 0; + /* + Memory root of every instruction is designated for permanent + transformations (optimizations) made on the parsed tree during + the first execution. It points to the memory root of the + entire stored procedure, as their life span is equal. + */ + instr->mem_root= &main_mem_root; insert_dynamic(&m_instr, (gptr)&instr); } diff --git a/sql/sp_head.h b/sql/sp_head.h index 2c75a320f30..aaef5a3d50e 100644 --- a/sql/sp_head.h +++ b/sql/sp_head.h @@ -274,7 +274,7 @@ private: // "Instructions"... // -class sp_instr : public Sql_alloc +class sp_instr :public Query_arena, public Sql_alloc { sp_instr(const sp_instr &); /* Prevent use of these */ void operator=(sp_instr &); @@ -282,17 +282,16 @@ class sp_instr : public Sql_alloc public: uint marked; - Item *free_list; // My Items uint m_ip; // My index sp_pcontext *m_ctx; // My parse context // Should give each a name or type code for debugging purposes? sp_instr(uint ip, sp_pcontext *ctx) - :Sql_alloc(), marked(0), free_list(0), m_ip(ip), m_ctx(ctx) + :Query_arena(0, INITIALIZED_FOR_SP), marked(0), m_ip(ip), m_ctx(ctx) {} virtual ~sp_instr() - { free_items(free_list); } + { free_items(); } // Execute this instrution. '*nextp' will be set to the index of the next // instruction to execute. (For most instruction this will be the diff --git a/sql/sql_class.cc b/sql/sql_class.cc index 8abd7cbbe7d..20f48da9283 100644 --- a/sql/sql_class.cc +++ b/sql/sql_class.cc @@ -171,9 +171,6 @@ THD::THD() spcont(NULL) { current_arena= this; -#ifndef DBUG_OFF - backup_arena= 0; -#endif host= user= priv_user= db= ip= 0; catalog= (char*)"std"; // the only catalog we have for now host_or_ip= "connecting host"; @@ -528,7 +525,7 @@ void THD::cleanup_after_query() next_insert_id= 0; } /* Free Items that were created during this execution */ - free_items(free_list); + free_items(); /* In the rest of code we assume that free_list never points to garbage: Keep this predicate true. @@ -1485,6 +1482,21 @@ Query_arena::Type Query_arena::type() const } +void Query_arena::free_items() +{ + Item *next; + DBUG_ENTER("Query_arena::free_items"); + /* This works because items are allocated with sql_alloc() */ + for (; free_list; free_list= next) + { + next= free_list->next; + free_list->delete_self(); + } + /* Postcondition: free_list is 0 */ + DBUG_VOID_RETURN; +} + + /* Statement functions */ @@ -1556,11 +1568,11 @@ void THD::end_statement() void Query_arena::set_n_backup_item_arena(Query_arena *set, Query_arena *backup) { DBUG_ENTER("Query_arena::set_n_backup_item_arena"); - DBUG_ASSERT(backup_arena == 0); + DBUG_ASSERT(backup->is_backup_arena == FALSE); backup->set_item_arena(this); set_item_arena(set); #ifndef DBUG_OFF - backup_arena= 1; + backup->is_backup_arena= TRUE; #endif DBUG_VOID_RETURN; } @@ -1569,10 +1581,11 @@ void Query_arena::set_n_backup_item_arena(Query_arena *set, Query_arena *backup) void Query_arena::restore_backup_item_arena(Query_arena *set, Query_arena *backup) { DBUG_ENTER("Query_arena::restore_backup_item_arena"); + DBUG_ASSERT(backup->is_backup_arena); set->set_item_arena(this); set_item_arena(backup); #ifndef DBUG_OFF - backup_arena= 0; + backup->is_backup_arena= FALSE; #endif DBUG_VOID_RETURN; } diff --git a/sql/sql_class.h b/sql/sql_class.h index a635a126f84..adc164085f9 100644 --- a/sql/sql_class.h +++ b/sql/sql_class.h @@ -663,7 +663,10 @@ public: Item *free_list; MEM_ROOT *mem_root; // Pointer to current memroot #ifndef DBUG_OFF - bool backup_arena; + bool is_backup_arena; /* True if this arena is used for backup. */ +#define INIT_ARENA_DBUG_INFO is_backup_arena= 0 +#else +#define INIT_ARENA_DBUG_INFO #endif enum enum_state { @@ -681,12 +684,14 @@ public: Query_arena(MEM_ROOT *mem_root_arg, enum enum_state state_arg) : free_list(0), mem_root(mem_root_arg), state(state_arg) - {} + { INIT_ARENA_DBUG_INFO; } /* This constructor is used only when Query_arena is created as backup storage for another instance of Query_arena. */ - Query_arena() {}; + Query_arena() { INIT_ARENA_DBUG_INFO; } + +#undef INIT_ARENA_DBUG_INFO virtual Type type() const; virtual ~Query_arena() {}; @@ -726,6 +731,8 @@ public: void set_n_backup_item_arena(Query_arena *set, Query_arena *backup); void restore_backup_item_arena(Query_arena *set, Query_arena *backup); void set_item_arena(Query_arena *set); + + void free_items(); }; diff --git a/sql/sql_prepare.cc b/sql/sql_prepare.cc index eeea493d868..c97cb037f15 100644 --- a/sql/sql_prepare.cc +++ b/sql/sql_prepare.cc @@ -2429,7 +2429,7 @@ Prepared_statement::~Prepared_statement() { if (cursor) cursor->Cursor::~Cursor(); - free_items(free_list); + free_items(); delete lex->result; } diff --git a/sql/sql_select.cc b/sql/sql_select.cc index 1487dfbb436..96a25c7919b 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -1914,8 +1914,7 @@ Cursor::close() } join= 0; unit= 0; - free_items(free_list); - free_list= 0; + free_items(); /* Must be last, as some memory might be allocated for free purposes, like in free_tmp_table() (TODO: fix this issue)