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

Add a startup message about PAUSE instruction timing

Details

    • Bug
    • Status: In Review (View Workflow)
    • Major
    • Resolution: Unresolved
    • 10.3(EOL), 10.4(EOL), 10.5
    • 10.5
    • Server
    • None

    Description

      It would be useful to log the outcome of the startup check that was implemented as part of MDEV-19845.

      The following patch technically works and does not cause an extra message to the log that is displayed by mysql-test-run.pl startup, but it is not nice in that it is scattering the logic to two source code modules:

      diff --git a/sql/mysqld.cc b/sql/mysqld.cc
      index f98f51c73d0..5b439856e84 100644
      --- a/sql/mysqld.cc
      +++ b/sql/mysqld.cc
      @@ -6198,6 +6198,10 @@ int mysqld_main(int argc, char **argv)
                                 mysqld_port, MYSQL_COMPILATION_COMMENT);
         }
       
      +#ifdef HAVE_PAUSE_INSTRUCTION
      +  sql_print_information("Using PAUSE multiplier %u", my_cpu_relax_multiplier);
      +#endif
      +
       #ifndef _WIN32
         // try to keep fd=0 busy
         if (please_close_stdin && !freopen("/dev/null", "r", stdin))
      

      Attachments

        Issue Links

          Activity

            monty Michael Widenius added a comment - - edited

            We can't use defines for having pause as there is no guarantee that the computer we run the program has the same CPU as the one where we compiled.

            Any detection of cpu instructions and the usage of them has to be done runtime

            monty Michael Widenius added a comment - - edited We can't use defines for having pause as there is no guarantee that the computer we run the program has the same CPU as the one where we compiled. Any detection of cpu instructions and the usage of them has to be done runtime

            monty, I do not understand your comment. We do not have any run-time check for the PAUSE instruction. It is unnecessary, because that instruction is defined as REPE NOP. As far as I understand, the REPE a.k.a. REPZ a.k.a. REP prefix is documented to be mostly no-op, except when followed by a specific instruction opcode. Both REPE (0xf3) and NOP (0x90) should exist since the dawn of the Intel 8086 microprocessor.

            Long time ago, REPE RET was employed to work around a hardware bug in AMD CPUs. https://gcc.gnu.org/legacy-ml/gcc-patches/2003-05/msg02117.html

            So, basically, HAVE_PAUSE_INSTRUCTION can be always enabled on the IA-32 and AMD64 ISA. The reason for any compile-time check in cmake is that it did take a while for the compiler and linker toolchains to implement the PAUSE alias for REPE NOP.

            marko Marko Mäkelä added a comment - monty , I do not understand your comment. We do not have any run-time check for the PAUSE instruction. It is unnecessary, because that instruction is defined as REPE NOP . As far as I understand, the REPE a.k.a. REPZ a.k.a. REP prefix is documented to be mostly no-op, except when followed by a specific instruction opcode. Both REPE (0xf3) and NOP (0x90) should exist since the dawn of the Intel 8086 microprocessor. Long time ago, REPE RET was employed to work around a hardware bug in AMD CPUs. https://gcc.gnu.org/legacy-ml/gcc-patches/2003-05/msg02117.html So, basically, HAVE_PAUSE_INSTRUCTION can be always enabled on the IA-32 and AMD64 ISA. The reason for any compile-time check in cmake is that it did take a while for the compiler and linker toolchains to implement the PAUSE alias for REPE NOP .

            monty said that we could make the message output dependent on global_system_variables.log_warnings > 2 or similar.

            Measurements on two Intel Cascade Lake based systems suggest that the PAUSE instruction latency was reverted from the Skylake microarchitecture’s roughly 120 clock cycles to pre-Skylake levels (about 12 clock cycles). To compensate for that, it seems that a larger value of my_cpu_relax_multiplier would be useful. I think that it would be best to address that as part of this bug fix.

            marko Marko Mäkelä added a comment - monty said that we could make the message output dependent on global_system_variables.log_warnings > 2 or similar. Measurements on two Intel Cascade Lake based systems suggest that the PAUSE instruction latency was reverted from the Skylake microarchitecture’s roughly 120 clock cycles to pre-Skylake levels (about 12 clock cycles). To compensate for that, it seems that a larger value of my_cpu_relax_multiplier would be useful. I think that it would be best to address that as part of this bug fix.

            For the record, Intel documentation claims the following:

            Architecture Pause Latency Cycles
            Broadwell 12
            Skylake 140
            Cascade Lake 44

            I believe that before it makes sense to perform any tweaks to the timing measurement in my_cpu_init(), we must address MDEV-22871 and run extensive benchmarks. Then we can say better what the my_cpu_relax_multiplier should be.

            marko Marko Mäkelä added a comment - For the record, Intel documentation claims the following: Architecture Pause Latency Cycles Broadwell 12 Skylake 140 Cascade Lake 44 I believe that before it makes sense to perform any tweaks to the timing measurement in my_cpu_init() , we must address MDEV-22871 and run extensive benchmarks. Then we can say better what the my_cpu_relax_multiplier should be.

            People

              monty Michael Widenius
              marko Marko Mäkelä
              Votes:
              1 Vote for this issue
              Watchers:
              7 Start watching this issue

              Dates

                Created:
                Updated:

                Git Integration

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