Sergey Vojtovich added a comment - 2017-12-01 13:12 There're 7 patches now, top 2 are not of interested, the rest 5 are to be reviewed.

Sergey Vojtovich added a comment - 2017-12-07 12:45 marko , sorry now there're even more revisions. Please look at those that have MDEV-14529 in their title.

Marko Mäkelä added a comment - 2017-12-07 14:27 svoj , the patches look generally OK. I posted some minor comments. OK to push after addressing those.

Sergey Vojtovich added a comment - 2017-12-08 13:58 Pushed commit c60095a818dce92838940525899a13a05633d148 Author: Sergey Vojtovich <svoj@mariadb.org> Date: Thu Dec 7 18:36:01 2017 +0400   Faster atomic loads and stores on windows   commit b04f2a0f01902d11e21afb690e14d12d2c464184 Author: Sergey Vojtovich <svoj@mariadb.org> Date: Fri Dec 1 15:48:03 2017 +0400   MDEV-14529 - InnoDB rw-locks: optimize memory barriers   Relax memory barrier for lock_word.   rw_lock_lock_word_decr() - used to acquire rw-lock, thus we only need to issue ACQUIRE when we succeed locking.   rw_lock_x_lock_func_nowait() - same as above, but used to attempt to acquire X-lock.   rw_lock_s_unlock_func() - used to release S-lock, RELEASE is what we need here.   rw_lock_x_unlock_func() - used to release X-lock. Ideally we'd need only RELEASE here, but due to mess with waiters (they must be loaded after lock_word is stored) we have to issue both ACQUIRE and RELEASE.   rw_lock_sx_unlock_func() - same as above, but used to release SX-lock.   rw_lock_s_lock_spin(), rw_lock_x_lock_func(), rw_lock_sx_lock_func() - fetch-and-store to waiters has to issue only ACQUIRE memory barrier, so that waiters are stored before lock_word is loaded.   Note that there is violation of RELEASE-ACQUIRE protocol here, because we do on lock:   my_atomic_fas32_explicit((int32*) &lock->waiters, 1, MY_MEMORY_ORDER_ACQUIRE); my_atomic_load32_explicit(&lock->lock_word, MY_MEMORY_ORDER_RELAXED);   on unlock   my_atomic_add32_explicit(&lock->lock_word, X_LOCK_DECR, MY_MEMORY_ORDER_ACQ_REL); my_atomic_load32_explicit((int32*) &lock->waiters, MY_MEMORY_ORDER_RELAXED);   That is we kind of synchronize ACQUIRE on lock_word with ACQUIRE on waiters. It was there before this patch. Simple fix may have negative performance impact. Proper fix requires refactoring of lock_word.   commit 51bb18f989d198aa648b2481865abd0734520d35 Author: Sergey Vojtovich <svoj@mariadb.org> Date: Fri Dec 1 13:52:24 2017 +0400   MDEV-14529 - InnoDB rw-locks: optimize memory barriers   Relax memory barrier for waiters: these 2 stores must be completed before os_event_set() finishes. This is guaranteed by RELEASE barrier issued by mutex.exit() of os_event_set().   commit 5b624f00fc0e6fa0a5a676d3ec445f62c0fb75ec Author: Sergey Vojtovich <svoj@mariadb.org> Date: Fri Dec 1 13:37:07 2017 +0400   MDEV-14529 - InnoDB rw-locks: optimize memory barriers   Remove volatile modifier from waiters: it's not supposed for inter-thread communication, use appropriate atomic operations instead.   Changed waiters to int32_t, my_atomic friendly type.   commit 57d20f1132df71e3b9aca998bcc31dfd62c942b3 Author: Sergey Vojtovich <svoj@mariadb.org> Date: Fri Dec 1 13:14:42 2017 +0400   MDEV-14529 - InnoDB rw-locks: optimize memory barriers   Remove volatile modifier from lock_word: it's not supposed for inter-thread communication, use appropriate atomic operations instead.   commit c73e77da0fa0fbdb4be5bfa1f4e441b06d1d91f9 Author: Sergey Vojtovich <svoj@mariadb.org> Date: Fri Dec 1 12:02:51 2017 +0400   MDEV-14529 - InnoDB rw-locks: optimize memory barriers   Change lock_word from lint to int32_t: the latter is my_atomic_* friendly type.

Sergey Vojtovich added a comment - 2017-12-08 14:07 - edited ssethia , rw-locks are now fixed in 10.3. If you still see performance improvement from your patch, then we should analyse other places where SEQ_CST is being issued.