What
Observe and document all of the remaining instances of DYNAMIC_ARRAY in the codebase. This includes finding where it is allocated, where it is destroyed, and if it has any "interesting" usage. If it is a member variable of a struct or class, that classes lifetime must also be fully understood and documented.
Why
From experiences learned while working on replacing the instances of List, many problems were exposed. Primarily the issue of object lifetime. New contributions to Drizzle are classes with proper construction and destruction. However, legacy and yet to be worked on items often have any of the C style allocations happening to them (memset, memcpy, malloc, mem_root, etc). If an STL container is put in one of these a memory leak can and will occur.
Blueprint
https://blueprints.launchpad.net/drizzle/+spec/code-cleanup-c++-replace-dynamic-array
How
Public Data Members
- unsigned char *buffer
- size_t elements
- Replace with: vector.size()
Public Interface
- my_init_dynamic_array
- Wrapper around init_dynamic_array2, defaults 3rd parameter to NULL
- my_init_dynamic_array_ci
- Wrapper around init_dynamic_array2, defaults 3rd parameter to NULL
- my_init_dynamic_array2
- Wrapper around init_dynamic_array2, passes all parameters as-is
- my_init_dynamic_array2_ci
- Wrapper around init_dynamic_array2, passes all parameters as-is
- init_dynamic_array2
- Sets up internal data (does initial allocation, size of element, and various other bits)
- Returns true on allocation failure, false otherwise
- init_dynamic_array
- Dead code, no implementation
- insert_dynamic
- Returns true if there is an allocation failure (true otherwise)
- Replaced by vector->push_back
- alloc_dynamic
- Returns pointer to internal buffer for where a new element would be placed
- Returns NULL on allocation failure
- No good replacement as it basically is a hand straight into the internal storage
- pop_dynamic
- Returns last element and decrements the size
- If the array is empty, it returns NULL
- Replace with: foo = vector.back(), vector.pop_back()
- set_dynamic
- Sets given index to passed elemetn
- If given index is greater than current size, array is resized to accomidate
- Returns true on allocation failure, false otherwise
- Replace with: vector[index] = foo
- NOTE: Make sure and check if the auto-resize behavior was being used
- allocate_dynamic
- Reserves space in the array for a given number of elements
- Replace with: vector.reserve(num)
- get_dynamic
- Returns given index from the array
- If index is out of bounds, data is set to zero
- Replace with: vector[index]
- Note: May need to check bounds if calling code depends on the zero-ing behavior
- delete_dynamic
- Frees memory and sets element count to zero
- Replace with: (letting the vector destruct)
- Replace with: vector.resize(0) (if the memory is wanted)
- delete_dynamic_element
- Dead code, no implementation
- freeze_size
- Dead code, no implementation
- get_index_dynamic
- Dead code, no implementation
- dynamic_array_ptr
- Access element in the array
- Replace with: vector[index]
- dynamic_element
- Accesses element in the array (casted to the given type)
- Replace with: vector[index]
- push_dynamic
- Just calls insert_dynamic
- int reset_dynamic()
- Sets element count to zero but does not release memory
- Always returns zero (is actually #define)
- Replace with: vector.clear()
- sort_dynamic
- Sorts the array with the given comparison function
- Replace with: std::sort( vector.begin(), vector.end(), cmp )
Remaining Usage
Simple Replacements
- mysys/default.cc:378 - DYNAMIC_ARRAY args
- Usage:
- Holds char*
- Local variable in load_defaults
- Lifetime:
- Declared on 378
- my_init_dynamic_array called on 491
- delete_dynamic called on 461
- drizzled/unique.h:34 - DYNAMIC_ARRAY file_ptrs
- Usage:
- Holds BUFFPEK
- Data member of Unique class
- Lifetime:
- my_init_dynamic_array called in Unique()
- delete_dynamic called in ~Unique()
- reset_dynamic called in Unique::reset()
- Unique Lifetimes:
- QUICK_INDEX_MERGE_SELECT::read_keys_and_merge()
- Instance allocated and destroyed in function
- Item_sum_distinct::tree, Unique* data member
- Allocated in Item_sum_distinct::setup
- Destroyed in Item_sum_distinct::cleanup and destructor
- Item_sum_count_distinct::tree, Unique* data member
- Allocated in Item_sum_count_distinct::setup
- Destroyed in Item_sum_count_distinct::cleanup and destructor
- Item_func_group_concat::unique_filter, Unique* data member
- Allocated in Item_func_group_concat::setup
- Destroyed in Item_func_group_concat::cleanup and destructor
- drizzled/opt_range.h:688 - DYNAMIC_ARRAY min_max_ranges
- Usage:
- Holds QUICK_RANGE*
- Data member of QUICK_GROUP_MIN_MAX_SELECT class
- Lifetime:
- my_init_dynamic called in QUICK_GROUP_MIN_MAX_SELECT::init (opt_range.cc:8960)
- delete_dynamic called in destructor
- QUICK_GROUP_MIN_MAX_SELECT Lifetimes:
- Allocated and returned from TRP_GROUP_MIN_MAX::make_quick
- Like QUICK_RANGE_SELECT above
- Stored in SQL_SELECT::quick member
- SQL_SELECT::cleanup deletes quick member variable
- drizzled/sql_plugin.cc:69 - static DYNAMIC_ARRAY plugin_dl_array
- Usage:
- Holds plugin::Library*
- Static file variable
- Lifetime:
- my_init_dynamic called from plugin_init (sql_plugin.cc:598)
- delete_dynamic called from plugin_shutdown (sql_plugin.cc:801)
- drizzled/sql_plugin.cc:70 static DYNAMIC_ARRAY plugin_array
- Usage:
- Holds plugin::Handle*
- Static file variable
- Lifetime:
- my_init_dynamic called from plugin_init (sql_plugin.cc:600)
- delete_dynamic called from plugin_shutdown (sql_plugin.cc:792)
- tests/resolve_stack_dump.cc - DYNAMIC_ARRAY sym_table
- Usage:
- Holds SYM_ENTRY
- Static file variable
- Lifetime:
- my_init_dynamic called in init_sym_table (tests/resolve_stack_dump.cc:218)
- called from main() (tests/resolve_stack_dump.cc:312)
- delete_dynamic called in clean_up (tests/resolve_stack_dumpe.cc:236)
- called from main() (tests/resolve_stack_dump.cc:314)
- drizzled/set_var.cc:77 - static DYNAMIC_ARRAY fixed_show_vars
- Usage:
- Holds SHOW_VAR
- Static file variable
- Lifetime:
- my_init_dynamic called in set_var_init (set_var.cc:1889)
- delete_dynamic called in set_var_free (set_var.cc:1910)
- There is a memcpy into the fixed_show_vars buffer, but it should be easily replaced by inserting
Non Trivial Replacements
- drizzled/opt_range.h:310 - DYNAMIC_ARRAY ranges
- Usage:
- Holds QUICK_RANGE*
- Data member of QUICK_RANGE_SELECT class
- Lifetime:
- my_init_dynamic_array called in constructor
- delete_dynamic called in destructor
- QUICK_RANGE_SELECT Lifetimes:
- Allocated and returned from get_quick_select (opt_range.cc:6497)
- TABLE_READ_PLAN::make_quick, pure virtual function
- Called and returned from TRP_RANGE::make_quick (opt_range.cc: :1964)
- Called and stored from TRP_ROR_INTERSECT::make_quick (opt_range.cc:3686, 3698)
- Called and stored from TRP_GROUP_MIN_MAX::make_quick (opt_range.cc:8799)
- Stored in SQL_SELECT::quick
- SQL_SELECT::cleanup deletes quick member variable
- Allocated and returned from get_quick_select_for_ref (opt_range.cc:6763)
- Called from create_sort_index, (sql_select.cc:5919), stored in SQL_SELECT::quick
- SQL_SELECT::cleanup deletes quick member variable
- Details:
- The QUICK_RANGE_SELECT copy constructor calls memmove(this, &rhs, sizeof(*this))
- If this is required and/or how it affects the ownership of other data members, I don't know at the moment
- client/drizzletest.cc:6300 - DYNAMIC_ARRAY regex_arr
- Usage:
- Holdsy st_regex
- Data member of st_replace_regex struct
- Lifetime:
- my_init_dynamic called in init_replace_regex (client/drizzletest.cc:6371)
- delete_dynamic called in free_replace_regex (client/drizzletest.cc:6525)
- Details:
- In an effort to "reduce freeing/allocation" (per comments) the st_replace_regex_struct and the regex expression are malloc-ed in one go
- Logic is simple and contained in init_replace_regex
- Multiple calls to malloc would handle this or replacing the char* with std::string
- Could also easily convert the st_replace_regex into a class and move init/free logic to constructor/destructor
- plugin/myisam/sort.cc:110 - DYNAMIC_ARRAY buffpek
- Usage:
- Holds BUFFPEK
- Local variable of _create_index_by_sort function
- Lifetime:
- memset at line 130 for no particular reason?
- my_init_dynamic called at line 162
- delete_dynamic called at line 247
- Details:
- Passed as parameter find_all_keys function (plugin/myisam/sort.cc:257)
- Manual usage of alloc_dynamic and fiddling with the pointer to the last element
- Lifetime is simple, but careful evaluation before change is needed
- mysys/my_lic.cc:99 - DYNAMIC_ARRAY *dir_entries_storage
- Usage:
- Holds FILEINFO
- Local variable of my_dir function
- Lifetime:
- my_init_dynamic called on 103
- delete_dynamic called in my_dirend on 162
- Details:
- Is allocated along with 3 other pieces of data in one call to malloc, with lots of ALIGN_SIZE and offsets
- The internal buffer implicitly changes ownership into a MY_DIR* structure that is returned from my_dir on success
- In my_dirend things are then casted back to what they originally were for freeing
- Basically, anywhere that calls my_dir would need to be looked at and could be affected by a change
- alloc_dynamic is used a number of times in relation to this, which doesn't have a correlation in any std container
- Related: Mysys_replacement which appears to be tackling the larger my_dir problem
- drizzled/join.h:164 - DYNAMIC_ARRAY keyuse
- Usage:
- Data member of JOIN class
- Lifetime:
- my_init_dynamic_array called on join.cc:1241 from
- update_ref_and_keys called on join.cc:5781, which was called by
- make_join_statistics called on join.cc:538 which was called by JOIN::optimize
- delete_dynamic called on join.cc:1694 from JOIN::destroy
- Details:
- JOIN is still treated like a chunk of data
- JOIN::restore_tmp does a memcpy from saved tmp to this (ie, itself)
- JOIN::init_save_join_tab() does a raw session->alloc
- Dubious lifetime (stored in mem_root?)
- mysys/hash.h:50 - DYNAMIC_ARRAY array
- Usage:
- Data member of st_hash/HASH structure
- Lifetime:
- my_init_dynamic_array_ci called in _hash_init (mysys/hash.cc:64)
- delete_dynamic called in hash_free (mysys/hash.cc:120)
- reset_dynamic called in my_hash_reset (mysys/hash.cc:136
- Details:
- Fairly straightforward usage, however it is a public member in the HASH structure
- As of 2009/08/11 there remaing about a dozen usages of HASH in the codebase
- Probably easiest to leave the DYNAMIC_ARRAY usage as it occurs inside of HASH
- HASH will probably also be replaced as well, so inspecting all usages to see if they touch the internal array would be a waste
- If it isn't replaced, it would be putting an STL structure inside of an old mysys data structure - what's the point?
Completed / Refactored