Details
-
Task
-
Status: Open (View Workflow)
-
Major
-
Resolution: Unresolved
-
None
Description
While testing MySQL, however same code still in MariaDB, I came across this performance problem where loading the global variables btr_search_enabled. The load of this memory location was a hot point in the code.
It turns out during compilation that btr_search_enabled was placed on a shared cache line address as ut_rnd_ulint_counter and the system was extensively using random numbers. The result is that a critical path of innodb was slow as this global variable was frequently expelled from the CPU caches and access was only granted to read it to one of the CPUs at a time.
This is a more general problem that SQL global variables are mostly read only and there is a proliferation of statistics counters and other static variables that could be innocently placed beside a C++ variable in the linker stage.
To resolve this, a similar mechanism to the linux kernel is used to use attributes to mark global variables that are read_mostly and put them in the same section. Because they are in the same section they are prevented from false sharing with other static variables.
The attached patch demonstrates this concept, however extending this to every global variable would bit rot and be susceptible to merge conflicts.
As an extension to this, global variables that are copied to session variables could also be in their own section so when the start of the global -> session occurs, they are all the same pre-fetched cache-line.
Attachments
Issue Links
- relates to
-
-
- Open
-
- links to
Activity
MySQL-8.0 also alleviated the ut_rnd_ulint_counter contention by moving it to a c++11 thread_local variable preserving a random mutex spin backoff without contention. I suspect with the above commit it isn't hot in mariadb any more.
Variables are hot because they are written to requiring an exclusive cache line. There may be some cases where 2 or more variables are updated by the same thread (and therefore assumed to be same core) in quick succession and therefore would be better shared. Other hot-hot contention using cache line padding could be good, however I'm more tempted to deal with those on a case by case basis as its also often the case that these are conflicting with themselves (like `buf_pool->stat.n_page_gets++` in buf_page_get_gen that I'll write a separate issue for soon - or at least comment on MDEV-15058).
For the PE variation on this theme, and MSVC see
pragma section, and declspec(allocate("section_name")) in https://docs.microsoft.com/en-us/cpp/preprocessor/section?view=msvc-160
We're less affected by the ut_rnd issue because of https://github.com/MariaDB/server/commit/ce04790
But there're other similar cases of course.
Separating read_mostly from write_mostly variables may have some positive impact. But when it comes to "hot" variables like ut_rnd_ulint_counter, shouldn't these stay on dedicated cache lines? To avoid hot-hot contention.