In my investigations into heap allocations during IBD, I discovered that ThreadRename is called many many times, in some cases it does multiple string allocations as well. This modifies ThreadRename to not allocate memory, opting for a static thread-local buffer instead.
I also create a new ThreadRenameWithWorker helper to avoid strprintf allocations.
before:
after:
This is called many times, let's use a buffer on the stack instead
ThreadRename being called so many times? Does it make sense? :)
… ThreadRename is called many many times…
It seems like a more serious bug than memory over-allocation.
hmmm I went into this from a top-down approach. I’m not familiar with the internals of core, but it’s possible the tool I’m using (heaptrack) might have false positives or be flat out wrong. all I know is that doing this PR made the ThreadRename heaptrack logs go away.
There are a bunch of callsites but none of them look too suspicious…
Uhm, if this is that case, the underlying issue must be addressed, not the symptom. It’s supposed to only be called at initial thread creation and bitcoin core’s threads are more or less static. (e.g. the HTTP worker threads are only supposed to be created once, at startup, not all the time? wtf is happening lately)
Could this also explain why thread_local adds unexpected amount of overhead? (see #18678)
It’s somewhat of a mystery. It’s up to the compiler - OS - libc - architecture combination how thread_local is implemented.
Could you try --disable-threadlocal to see if it makes this problem go away?
amazingly, the allocs went away with --disable-threadlocal
make clean just incase to confirm
Ouch. Looks like you’re on to something.
Which platform are you checking on? On x86_64, Ubuntu 20.04, with gcc 9.3.0-10ubuntu2, it looks like ThreadRename is only called once per thread as expected, even with active RPC use (edit: oh and once at shutdown with shutoff). Checked by breakpointing it in gdb.
yup no more ThreadRename allocs on master with --disable-threadlocal after a make clean
gcc (GCC) 9.3.0
going to try with clang
I have the same. Why can’t I reproduce this. This is fucking madness.
Edit: can you please trap the function in gdb and look what the backtrace is, where it keeps being called from for the same threads?
Tested IBD on x86_64, Linux Mint 19.3 with the patched util::ThreadRename() to print a message. The only output is:
0$ ./src/qt/bitcoin-qt -testnet -server
1ThreadRename: name=qt-init
2ThreadRename: name=scriptch.1
3ThreadRename: name=scriptch.0
4ThreadRename: name=scriptch.2
5ThreadRename: name=scheduler
6ThreadRename: name=httpworker.0
7ThreadRename: name=http
8ThreadRename: name=httpworker.3
9ThreadRename: name=httpworker.1
10ThreadRename: name=httpworker.2
11ThreadRename: name=loadblk
12ThreadRename: name=net
13ThreadRename: name=addcon
14ThreadRename: name=msghand
15ThreadRename: name=opencon
16ThreadRename: name=dnsseed
17ThreadRename: name=shutoff
heaptrack tracks allocations, I have no idea how heaptrack attributes mallocs to ThreadRename, but it must be something thread_local related…
here’s with clang with thread_local enabled:
let me try to log the get internal name call
I have the feeling it’s more subtle.
Mind that without thread_local enabled, the SetInternalName function called from ThreadRename is stubbed out
0static void SetInternalName(std::string name) { }
It might still be that it is called many times, it just doesn’t show up in this profile.
(but it means that ::prctl for ex. will still be called a lot)
ok I’m dumb, I think I was reading this pane wrong. It’s showing the top-down allocations. so that means it allocated that many times UP to the point it allocated once in ThreadRename.
so perhaps the thread_local performance thing is just because LogPrintF has to allocate a bit more to display thread info? Seems like 6 minutes is lot though just for a print line…