From 9f2adffccae1e155647e48f65aa7932fc833f723 Mon Sep 17 00:00:00 2001 From: Sergei Golubchik Date: Thu, 26 Sep 2024 11:52:05 +0200 Subject: [PATCH] memroot improvement: fix savepoint support savepoint support was added a year ago as get_last_memroot_block() and free_all_new_blocks(), but it didn't work and was disabled. * fix it to work * instead of freeing the memory, only mark blocks free - this feature is supposed to be used inside a loop (otherwise there is no need to free anything, end of statement will do it anyway). And freeing blocks inside a loop is a bad idea, as they'll be all malloc-ed on the next iteration again. So, don't. * fix a bug in mark_blocks_free() - it doesn't change the number of blocks --- include/my_alloc.h | 8 +++++ include/my_sys.h | 4 +-- mysys/my_alloc.c | 85 +++++++++++++++++++++++++++------------------- sql/sql_admin.cc | 6 ++-- 4 files changed, 63 insertions(+), 40 deletions(-) diff --git a/include/my_alloc.h b/include/my_alloc.h index 0b777437dbe..6fd4321c38e 100644 --- a/include/my_alloc.h +++ b/include/my_alloc.h @@ -60,6 +60,14 @@ typedef struct st_mem_root PSI_memory_key psi_key; } MEM_ROOT; +typedef struct st_mem_root_savepoint +{ + MEM_ROOT *root; + USED_MEM *free; + USED_MEM *used; + unsigned short first_block_usage; +} MEM_ROOT_SAVEPOINT; + #ifdef __cplusplus } #endif diff --git a/include/my_sys.h b/include/my_sys.h index bbfaf959163..a9bde7422bb 100644 --- a/include/my_sys.h +++ b/include/my_sys.h @@ -921,8 +921,8 @@ extern void move_root(MEM_ROOT *to, MEM_ROOT *from); extern void set_prealloc_root(MEM_ROOT *root, char *ptr); extern void reset_root_defaults(MEM_ROOT *mem_root, size_t block_size, size_t prealloc_size); -extern USED_MEM *get_last_memroot_block(MEM_ROOT* root); -extern void free_all_new_blocks(MEM_ROOT *root, USED_MEM *last_block); +extern void root_make_savepoint(MEM_ROOT *root, MEM_ROOT_SAVEPOINT *sv); +extern void root_free_to_savepoint(const MEM_ROOT_SAVEPOINT *sv); extern void protect_root(MEM_ROOT *root, int prot); extern char *strdup_root(MEM_ROOT *root,const char *str); static inline char *safe_strdup_root(MEM_ROOT *root, const char *str) diff --git a/mysys/my_alloc.c b/mysys/my_alloc.c index 39c113650da..e6e58a2795f 100644 --- a/mysys/my_alloc.c +++ b/mysys/my_alloc.c @@ -266,8 +266,8 @@ void *alloc_root(MEM_ROOT *mem_root, size_t length) { size_t get_size, block_size; uchar* point; - reg1 USED_MEM *next= 0; - reg2 USED_MEM **prev; + USED_MEM *next= 0; + USED_MEM **prev; size_t original_length __attribute__((unused)) = length; DBUG_ENTER("alloc_root"); DBUG_PRINT("enter",("root: %p", mem_root)); @@ -425,10 +425,10 @@ void *multi_alloc_root(MEM_ROOT *root, ...) #if !(defined(HAVE_valgrind) && defined(EXTRA_DEBUG)) /** Mark all data in blocks free for reusage */ -static inline void mark_blocks_free(MEM_ROOT* root) +static void mark_blocks_free(MEM_ROOT* root) { - reg1 USED_MEM *next; - reg2 USED_MEM **last; + USED_MEM *next; + USED_MEM **last; /* iterate through (partially) free blocks, mark them free */ last= &root->free; @@ -451,7 +451,6 @@ static inline void mark_blocks_free(MEM_ROOT* root) /* Now everything is set; Indicate that nothing is used anymore */ root->used= 0; root->first_block_usage= 0; - root->block_num= 4; } #endif @@ -477,7 +476,7 @@ static inline void mark_blocks_free(MEM_ROOT* root) void free_root(MEM_ROOT *root, myf MyFlags) { - reg1 USED_MEM *next,*old; + USED_MEM *next,*old; DBUG_ENTER("free_root"); DBUG_PRINT("enter",("root: %p flags: %lu", root, MyFlags)); @@ -567,45 +566,61 @@ void move_root(MEM_ROOT *to, MEM_ROOT *from) from->used= 0; } - - /* - Remember last MEM_ROOT block. + Prepare MEM_ROOT to a later truncation. Everything allocated after + that point can be freed while keeping earlier allocations intact. - This allows one to free all new allocated blocks. + For this to work we cannot allow new allocations in partially filled blocks, + so remove all non-empty blocks from the memroot. For simplicity, let's + also remove all used blocks. */ - -USED_MEM *get_last_memroot_block(MEM_ROOT* root) +void root_make_savepoint(MEM_ROOT *root, MEM_ROOT_SAVEPOINT *sv) { - return root->used ? root->used : root->pre_alloc; + USED_MEM **prev= &root->free, *block= *prev; + for ( ; block; prev= &block->next, block= *prev) + if (block->left < block->size - ALIGN_SIZE(sizeof(USED_MEM))) + break; + sv->root= root; + sv->free= block; + sv->used= root->used; + sv->first_block_usage= root->first_block_usage; + *prev= 0; + root->used= 0; } /* - Free all newly allocated blocks + Restore MEM_ROOT to the state before the savepoint was made. + + Restore old free and used lists. + Mark all new (after savepoint) used and partially used blocks free + and put them into the free list. */ - -void free_all_new_blocks(MEM_ROOT *root, USED_MEM *last_block) +void root_free_to_savepoint(const MEM_ROOT_SAVEPOINT *sv) { - USED_MEM *old, *next; - if (!root->used) - return; /* Nothing allocated */ - return; - /* - Free everying allocated up to, but not including, last_block. - However do not go past pre_alloc as we do not want to free - that one. This should not be a problem as in almost all normal - usage pre_alloc is last in the list. - */ + MEM_ROOT *root= sv->root; + USED_MEM **prev= &root->free, *block= *prev; - for (next= root->used ; - next && next != last_block && next != root->pre_alloc ; ) + /* iterate through (partially) free blocks, mark them free */ + for ( ; block; prev= &block->next, block= *prev) { - old= next; next= next->next; - root_free(root, old, old->size); + block->left= block->size - ALIGN_SIZE(sizeof(USED_MEM)); + TRASH_MEM(block); } - root->used= next; - root->block_num= 4; - root->first_block_usage= 0; + + /* Combine the free and the used list */ + *prev= block=root->used; + + /* now go through the used blocks and mark them free */ + for ( ; block; prev= &block->next, block= *prev) + { + block->left= block->size - ALIGN_SIZE(sizeof(USED_MEM)); + TRASH_MEM(block); + } + + /* restore free and used lists from savepoint */ + *prev= sv->free; + root->used= sv->used; + root->first_block_usage= prev == &root->free ? sv->first_block_usage : 0; } /** @@ -615,7 +630,7 @@ void free_all_new_blocks(MEM_ROOT *root, USED_MEM *last_block) #if defined(HAVE_MMAP) && defined(HAVE_MPROTECT) && defined(MAP_ANONYMOUS) void protect_root(MEM_ROOT *root, int prot) { - reg1 USED_MEM *next,*old; + USED_MEM *next,*old; DBUG_ENTER("protect_root"); DBUG_PRINT("enter",("root: %p prot: %d", root, prot)); diff --git a/sql/sql_admin.cc b/sql/sql_admin.cc index 9174166511c..165ac47901c 100644 --- a/sql/sql_admin.cc +++ b/sql/sql_admin.cc @@ -982,7 +982,7 @@ static bool mysql_admin_table(THD* thd, TABLE_LIST* tables, { TABLE *tab= table->table; Field **field_ptr= tab->field; - USED_MEM *memroot_block; + MEM_ROOT_SAVEPOINT memroot_sv; if (!lex->column_list) { @@ -1084,7 +1084,7 @@ static bool mysql_admin_table(THD* thd, TABLE_LIST* tables, } /* Ensure that number of records are updated */ tab->file->info(HA_STATUS_VARIABLE); - memroot_block= get_last_memroot_block(thd->mem_root); + root_make_savepoint(thd->mem_root, &memroot_sv); if (!(compl_result_code= alloc_statistics_for_table(thd, tab, &tab->has_value_set)) && @@ -1092,7 +1092,7 @@ static bool mysql_admin_table(THD* thd, TABLE_LIST* tables, collect_statistics_for_table(thd, tab))) compl_result_code= update_statistics_for_table(thd, tab); free_statistics_for_table(tab); - free_all_new_blocks(thd->mem_root, memroot_block); + root_free_to_savepoint(&memroot_sv); } else compl_result_code= HA_ADMIN_FAILED;