MDEV-8684 ut relax cpu isnt relaxing #168
Conversation
UT_RELAX_CPU(): Use a compiler barrier. ut_delay(): Remove the dummy global variable ut_always_false. RB: 11399 Reviewed-by: Jimmy Yang <jimmy.yang@oracle.com> Backported from MySQL-5.7 - patch mysql/mysql-server@5e3efb0 Suggestion by Stewart Smith
Bug#18842925 : SET THREAD PRIORITY IN INNODB MUTEX SPINLOOP Like "pause" instruction for hyper-threading at Intel CPUs, POWER has special instructions only for hinting priority of hardware-threads. Approved by Sunny in rb#6256 Backport of the 5.7 fix - mysql/mysql-server@c92102a (excluded cache line size patch) Suggestion by Stewart Smith
Also introduce compiler barrier properly on all architectures.
Using __ppc_get_timebase will translate to mfspr instruction The mfspr instruction will block FXU1 until complete but the other Pipelines are available for execution of instructions from other SMT threads on the same core. The latency time to read the timebase SPR is ~10 cycles. So any impact on other threads is limited other FXU1 only instructions (basically other mfspr/mtspr ops). Suggested by Steven J. Munroe, Linux on Power Toolchain Architect, Linux Technology Center IBM Corporation
|
Hi, can you explain the change and have you done some performance testing on this ? |
| @@ -88,15 +88,25 @@ struct ut_when_dtor { | |||
| the YieldProcessor macro defined in WinNT.h. It is a CPU architecture- | |||
| independent way by using YieldProcessor. */ | |||
| # define UT_RELAX_CPU() YieldProcessor() | |||
| # elif defined(HAVE_ATOMIC_BUILTINS) | |||
There was a problem hiding this comment.
Here you have replaced HAVE_ATOMIC_BUILTINS to powerpc is HAVE_ATOMIC_BUILTINS defined only on powerpc or can this be executed on other platforms before the change ?
There was a problem hiding this comment.
Atomic operations aren't relaxing on any CPU and have effects beyond the CPU which is counter to what ut_delay wants to achieve.
@akopytov do you have a suggestion as to what ARM should do to relax a processor?
@akamat-ibm any suggestion on what Z should do?
There was a problem hiding this comment.
@grooverdan I'm still evaluating possible options for ARM. I'll submit separate PRs for MySQL and likely MariaDB.
|
I put the performance testing in https://jira.mariadb.org/browse/MDEV-8684 |
i.e. __ppc_set_ppr_low rather than 'or 1,1,1'
grooverdan
changed the title
| # define UT_RELAX_CPU() do { \ | ||
| volatile lint volatile_var; \ | ||
| os_compare_and_swap_lint(&volatile_var, 0, 1); \ |
There was a problem hiding this comment.
But when HAVE_ATOMIC_BUILTINS was defined it did these two lines also when CPU != powerpc now not sure what happens.
There was a problem hiding this comment.
Because of UT_COMPILER_BARRIER the loop won't be optimised away and there will just be an empty loop.
I've no objection to leaving the HAVE_ATOMIC_BUILTINS implementation for non x86/powerpc architectures until they can come up with something better. I was just following the MySQL upstream patch in this respect.
|
thanks @janlindstrom |
|
Naturally, it does not compile on Windows... |
| } while (0) | ||
| # else | ||
| # define UT_RELAX_CPU() ((void)0) /* avoid warning for an empty statement */ | ||
| # endif | ||
|
|
||
| #define UT_COMPILER_BARRIER() __asm__ __volatile__ ("":::"memory") |
There was a problem hiding this comment.
This line does not compile on WIndows, is this portable outside gcc/clang ?
There was a problem hiding this comment.
Looks gcc-ish The correspording intrinsic for Microsoft compiler is called MemoryBarrier()
|
thanks @vaintroub, sorry for missing that. |
Combination of fixes. Details in JIRA.
My contributions here are under the MCA.