Hi! On Sun, Mar 28, 2021 at 3:00 PM Sergei Golubchik <serg@mariadb.org> wrote:
Hi, Michael!
On Mar 28, Michael Widenius wrote:
revision-id: dbb62d20f36 (mariadb-10.5.2-518-gdbb62d20f36) parent(s): 0c37211c2a2 author: Michael Widenius <michael.widenius@gmail.com> committer: Michael Widenius <michael.widenius@gmail.com> timestamp: 2021-03-24 14:31:53 +0200 message:
Fixes that enables my_new.cc (new wrapper using my_malloc)
This is not enabled by default as there are leaks in the server that needs to be fixed first. One can compile with -DREALLY_USE_MYSYS_NEW -DSF_REMEMBER_FRAMES=14 to find the memory leaks from 'new'
The change in my_new.cc is ok. The change in my_global.h and CMakeLists.txt is confusing and redundant.
You unconditionaly define USE_MYSYS_NEW then conditionally undefine it again? Everything already worked just fine. By default cxx new is used, if one wants to use mysys version, it's done with
-DHAVE_CXX_NEW=0
this works for me in 10.5 and fails to compile my_new.cc, as it should.
The problem is that one cannot and should never use HAVE_CXX_NEW as this is something that is found by cmake. In the future USE_MYSYS_NEW should be on by default and one would have to use -DUSE_MYSYS_NEW=0 if one does not want to use it. We cannot do that yet, which is why I added REALLY_USE_MYSYS_NEW temporarily to allow one to test, debug and fix MYSYS_NEW. One cannot fix this by reseting HAVE_CXX_NEW, because my_new.cc requires this to be defined if the platform supports it! Not doing that gave me compiler errors.
Please, only keed your my_new.cc changes and say in the commit message "One can compile with -DHAVE_CXX_NEW=0 to find memory leaks from 'new'"
Does not work, see above.
As for -DSF_REMEMBER_FRAMES=14, perhaps it's better to do it automatically? Like
#ifdef USE_MYSYS_NEW #define SF_REMEMBER_FRAMES 14 #else #define SF_REMEMBER_FRAMES 8 #endif
or may be even just always make it 14. It should be easy to enable this feature, right?
yes, this is a good idea, will do.
--- a/mysys/my_static.c +++ b/mysys/my_static.c @@ -48,6 +48,7 @@ PSI_memory_key key_memory_my_err_head; PSI_memory_key key_memory_my_file_info; PSI_memory_key key_memory_pack_frm; PSI_memory_key key_memory_charsets; +PSI_memory_key key_memory_new;
where do you initialize it? I'd suggest statically, like
PSI_memory_key key_memory_new= PSI_INSTRUMENT_MEM;
#ifdef _WIN32 PSI_memory_key key_memory_win_SECURITY_ATTRIBUTES;
Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org