From 91180cb8ab4924fc9907ee16ba3479f88c309bc2 Mon Sep 17 00:00:00 2001 From: unknown Date: Thu, 23 Jun 2005 20:22:08 +0400 Subject: [PATCH] - implement inheritance of sp_instr: public Query_arena. We need every instruction to have its own arena, because we want to track instruction's state (INITIALIZED_FOR_SP -> EXECUTED). Because of `if' statements and other conditional instructions used in stored procedures, not every instruction of a stored procedure gets executed during the first (or even subsequent) execution of the procedure. So it's better if we track the execution state of every instruction independently. All instructions of a given procedure now also share sp_head's mem_root, but keep their own free_list. This simplifies juggling with free Item lists in sp_head::execute. - free_items() moved to be a member of Query_arena. - logic of 'backup_arena' debug member of Query_arena has been changed to support multi-backups. Until now, TRUE 'backup_arena' meant that there is exactly one active backup of the THD arena. Now it means simply that the arena is used for backup, so that we can't accidentally overwrite an existing backup. This allows doing multiple backups, e.g. in sp_head::execute and Cursor::fetch, when THD arena is already backed up but we want to set yet another arena (usually the 'permanent' arena, to save permanent transformations/optimizations of a parsed tree). sql/sp_head.cc: - use Query_arena support in sp_head::execute() as now sp_instr inherites from it. sql/sp_head.h: - inherite sp_instr from Query_arena sql/sql_class.cc: - changed the principle of Query_arena::backup_arena; free_items is now a member of Query_arena. sql/sql_class.h: - changed the principle of Query_arena::backup_arena; free_items is now a member of Query_arena. sql/sql_prepare.cc: free_items() is now a member of Query_arena. sql/sql_select.cc: free_items() now automatically sets free_list to zero. --- sql/sp_head.cc | 71 ++++++++++++++++++++-------------------------- sql/sp_head.h | 7 ++--- sql/sql_class.cc | 27 +++++++++++++----- sql/sql_class.h | 13 +++++++-- sql/sql_prepare.cc | 2 +- sql/sql_select.cc | 3 +- 6 files changed, 65 insertions(+), 58 deletions(-) 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)