Bug #83574 InnoDB atomics bs: loads and stores
Submitted: 27 Oct 2016 11:29 Modified: 28 Nov 2016 5:31
Reporter: Sergey Vojtovich Email Updates:
Status: Verified Impact on me:
None 
Category:MySQL Server: InnoDB storage engine Severity:S5 (Performance)
Version:8.0 OS:Any
Assigned to: CPU Architecture:ARM

[27 Oct 2016 11:29] Sergey Vojtovich
Description:
InnoDB atomics implementation miss load and store operation. To simulate those it uses heavyweight read-modify-write instructions.

Especially bad is replacement of atomic load by atomic read-modify-write. It may cause scalability bottlenecks where it's not necessary.

Following examples of code that can be replaced with atomic loads and stores...

dict/dict0dict.cc:
...
        /* We use atomics to read index->zip_pad.pad. Here we use zero
        as increment as are not changing the value of the 'pad'. */

        pad = os_atomic_increment_ulint(&index->zip_pad.pad, 0);
...
The above is atomic load.

include/os0once.h:
...
                        const bool      swapped = os_compare_and_swap_uint32(
                                state, IN_PROGRESS, DONE);

                        ut_a(swapped);
...
The above is atomic store.

include/sync0rw.ic:
...
        (void) os_compare_and_swap_ulint(&lock->waiters, 0, 1);
...
        (void) os_compare_and_swap_ulint(&lock->waiters, 1, 0);
...
The above are atomic stores.
...
        /* Prevent Valgrind warnings about writer_thread being
        uninitialized.  It does not matter if writer_thread is
        uninitialized, because we are comparing writer_thread against
        itself, and the operation should always succeed. */
        UNIV_MEM_VALID(&lock->writer_thread, sizeof lock->writer_thread);

        local_thread = lock->writer_thread;
        success = os_compare_and_swap_thread_id(
                &lock->writer_thread, local_thread, curr_thread);
        ut_a(success);
...
The above is my favorite perl. All lines can be replaced with atomic store.

lock/lock0lock.cc:
...
        os_thread_id_t  thread_id = os_thread_get_curr_id();

        cas = os_compare_and_swap_thread_id(&trx->killed_by, 0, thread_id);

        ut_a(cas);
...
The above is atomic store.

trx/trx0purge.cc:
...
  while (!os_compare_and_swap_ulint(
    &purge_sys->n_completed, n_submitted, n_submitted)) {
...
The above is atomic load.

trx/trx0trx.cc:
...
  os_thread_id_t  thread_id = trx->killed_by;
  os_compare_and_swap_thread_id(&trx->killed_by, thread_id, 0);
...
  os_thread_id_t  thread_id = victim_trx->killed_by;
  os_compare_and_swap_thread_id(&victim_trx->killed_by,
                                                         thread_id, 0);
...
The above are atomic stores.

How to repeat:
Code analysis.

Suggested fix:
InnoDB badly needs sane atomics implementation. Follow C11 or gcc atomic builtins API.
[28 Nov 2016 5:31] MySQL Verification Team
Hello Sergey Vojtovich,

Thank you for your bug report. At this time we don't see any correctness issues. Our testing on Intel HW doesn't demonstrate any issues. We will triage and analyse the suggested changes as part of our process.

Thanks,
Umesh