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 secur...@mariadb.org
_______________________________________________
developers mailing list -- developers@lists.mariadb.org
To unsubscribe send an email to developers-le...@lists.mariadb.org
