Uploaded image for project: 'MariaDB Server'
  1. MariaDB Server
  2. MDEV-34296

extern thread_local is a CPU waste

Details

    Description

      In MDEV-31558 we wrongly thought that there would be minimal overhead for accessing a thread-local variable:

      extern thread_local ha_handler_stats *mariadb_stats;

      It turns out that in C++11, each access to an extern thread_local variable requires conditionally invoking an initialization function. In fact, we do have a custom initializer of this very variable, so that call is actually unavoidable:

      thread_local ha_handler_stats mariadb_dummy_stats;
      		 
      thread_local ha_handler_stats *mariadb_stats= &mariadb_dummy_stats;

      It turns out that already had a better declared thread local variable that the mariadb_stats would point to when it is not pointing to the dummy object:

      static thread_local THD *THR_THD;
      		 
       
      		 
      MYSQL_THD _current_thd() { return THR_THD; }

      We can access THD::handler_stats directly (well, via an accessor function, because the definition of THD is hidden from the InnoDB storage engine).

      Attachments

        Issue Links

          is caused by

          Task - A task that needs to be done. MDEV-31558 Add InnoDB engine information to the slow query log

          • Critical - Crashes, loss of data, severe memory leak.
          • Closed
          relates to

          Bug - A problem which impairs or prevents the functions of the product. MDEV-34297 get_rnd_value() of ib_counter_t is unnecessarily complex

          • Major - Major loss of function.
          • Closed

          Activity

            Ascending order - Click to sort in descending order
            marko Marko Mäkelä added a comment -

            I see that there also is handler::handler_stats that would be a null pointer or apparently point to THD::handler_stats. If there is no alternative to retaining mariadb_stats, then we could try the following to simulate the C11 _Thread_local:

            typedef IF_WIN(__declspec(thread),__thread) simple_thread_local;
            		 
            extern simple_thread_local ha_handler_stats *mariadb_stats;

            There are a few failing tests (missing r_engine_stats sections in ANALYZE FORMAT=JSON output) for the current implementation:

            10.6-MDEV-34296 5039dee49b7da592a71227e91411254013c3d5b4

            		 
            Failing test(s): main.analyze_engine_stats2 main.analyze_engine_stats main.rowid_filter_innodb

            marko Marko Mäkelä added a comment - I see that there also is handler::handler_stats that would be a null pointer or apparently point to THD::handler_stats . If there is no alternative to retaining mariadb_stats , then we could try the following to simulate the C11 _Thread_local : typedef IF_WIN( __declspec ( thread ),__thread) simple_thread_local; extern simple_thread_local ha_handler_stats *mariadb_stats; There are a few failing tests (missing r_engine_stats sections in ANALYZE FORMAT=JSON output) for the current implementation: 10.6-MDEV-34296 5039dee49b7da592a71227e91411254013c3d5b4 Failing test(s): main.analyze_engine_stats2 main.analyze_engine_stats main.rowid_filter_innodb
            marko Marko Mäkelä added a comment -

            My solution is to retain the mariadb_stats and basically replace &mariadb_dummy_stats with nullptr.

            C++20 introduces constinit thread_local, which roughly corresponds to the C11 _Thread_local, by allowing link-time constant initializer expressions. The address of a regular global or static variable would be fine, but the address of thread_local mariadb_dummy_stats is actually decided at runtime.

            marko Marko Mäkelä added a comment - My solution is to retain the mariadb_stats and basically replace &mariadb_dummy_stats with nullptr . C++20 introduces constinit thread_local , which roughly corresponds to the C11 _Thread_local , by allowing link-time constant initializer expressions. The address of a regular global or static variable would be fine, but the address of thread_local mariadb_dummy_stats is actually decided at runtime.

            People

              marko Marko Mäkelä
              marko Marko Mäkelä
              Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Git Integration

                  Error rendering 'com.xiplink.jira.git.jira_git_plugin:git-issue-webpanel'. Please contact your Jira administrators.