Hi, Yuchen, ok to push, after making one change (const) below. On Sep 13, Yuchen Pei wrote:
revision-id: 14a12f98eb8 (mariadb-11.0.1-232-g14a12f98eb8) parent(s): e987b9350cb author: Yuchen Pei committer: Yuchen Pei timestamp: 2023-09-13 11:50:41 +1000 message:
MDEV-15935 Clean up string sys_var types.
Merge sys_var_charptr with sys_var_charptr_base, as well as merge Sys_var_session_lexstring into Sys_var_lexstring. Also refactored update methods of sys_var_charptr accordingly.
Because the class is more generic, session_update() calls sys_var_charptr::session_update() which does not assume a buffer field associated with THD, but instead call strdup/free, we get rid of THD::default_master_connection_buff accordingly
You could've mentioned here "this also makes THD smaller by 192 bytes, and there can be many thousands of concurrent THDs" I mean, it's a good result, one of the benefits of your cleanup.
diff --git a/sql/sys_vars.inl b/sql/sys_vars.inl index 31ebbae01ca..5bac91a0db3 100644 --- a/sql/sys_vars.inl +++ b/sql/sys_vars.inl @@ -492,14 +492,18 @@ class Sys_var_mybool: public Sys_var_typelib Backing store: char*
@note - This class supports only GLOBAL variables, because THD on destruction - does not destroy individual members of SV, there's no way to free - allocated string variables for every thread. + + Note that the memory management for SESSION_VAR's is manual, the + value must be strdup'ed in THD::init() and freed in + plugin_thdvar_cleanup(). TODO: it should be done automatically when + we'll have more session string variables to justify it. Maybe some + kind of a loop over all variables, like sys_var_end() in set_var.cc? */ -class Sys_var_charptr_base: public sys_var +class Sys_var_charptr: public sys_var { + size_t max_length= 2000;
Should be const, right? No need to store max_length in every Sys_var_charptr instance, if you aren't going to modify it per instance.
public: - Sys_var_charptr_base(const char *name_arg, + Sys_var_charptr(const char *name_arg, const char *comment, int flag_args, ptrdiff_t off, size_t size, CMD_LINE getopt, const char *def_val, PolyLock *lock=0,
Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org