Additional changes to MicroLock

Summary:
Changes discussed on the initial MicroLock diff, but that were
accidentally lost before commit.

Reviewed By: luciang, yfeldblum

Differential Revision: D3010789

fbshipit-source-id: 9538ebd1383d1561dd2ce210a2d05e342c6863e6

diff --git a/folly/MicroLock.cpp b/folly/MicroLock.cpp
index 778cd5b91527a75f29e87d7d58221452546bfbf0..541eb711c1f7d8766dd7325270ef398fd839c4f9 100644 (file)
--- a/folly/MicroLock.cpp
+++ b/folly/MicroLock.cpp
@@ -26,7 +26,7 @@ void MicroLockCore::lockSlowPath(uint32_t oldWord,
                                  uint32_t slotHeldBit,
                                  unsigned maxSpins,
                                  unsigned maxYields) {
-  unsigned newWord;
+  uint32_t newWord;
   unsigned spins = 0;
   uint32_t slotWaitBit = slotHeldBit << 1;
 

diff --git a/folly/MicroLock.h b/folly/MicroLock.h
index 15c8d414d69da25937977c8f00b428c0a613c4bb..49a14b141ba89cd104dfc0f6adeebc8ca828eb13 100644 (file)
--- a/folly/MicroLock.h
+++ b/folly/MicroLock.h
@@ -54,10 +54,10 @@ namespace folly {
  * aliasing rules, character types may alias anything.)
  *
  * MicroLock uses a dirty trick: it actually operates on the full
- * word-size, word-aligned bit of memory into which it is embedded.
+ * 32-bit, four-byte-aligned bit of memory into which it is embedded.
  * It never modifies bits outside the ones it's defined to modify, but
- * it _accesses_ all the bits in the word for purposes of
- * futex management.
+ * it _accesses_ all the bits in the 32-bit memory location for
+ * purposes of futex management.
  *
  * The MaxSpins template parameter controls the number of times we
  * spin trying to acquire the lock.  MaxYields controls the number of
@@ -86,9 +86,12 @@ namespace folly {
  *
  * (The virtual dispatch benchmark is provided for scale.)
  *
- * The contended case for MicroLock is likely to be worse compared to
- * std::mutex than the contended case is.  Make sure to benchmark your
- * particular workload.
+ * While the uncontended case for MicroLock is competitive with the
+ * glibc 2.2.0 implementation of std::mutex, std::mutex is likely to be
+ * faster in the contended case, because we need to wake up all waiters
+ * when we release.
+ *
+ * Make sure to benchmark your particular workload.
  *
  */
 
@@ -100,7 +103,7 @@ class MicroLockCore {
 #else
   uint8_t lock_;
 #endif
-  inline detail::Futex<>* word() const;
+  inline detail::Futex<>* word() const; // Well, halfword on 64-bit systems
   inline uint32_t baseShift(unsigned slot) const;
   inline uint32_t heldBit(unsigned slot) const;
   inline uint32_t waitBit(unsigned slot) const;
@@ -128,9 +131,8 @@ inline unsigned MicroLockCore::baseShift(unsigned slot) const {
 
   unsigned offset_bytes = (unsigned)((uintptr_t)&lock_ - (uintptr_t)word());
 
-  return kIsLittleEndian
-             ? offset_bytes * CHAR_BIT + slot * 2
-             : CHAR_BIT * (sizeof(uint32_t) - offset_bytes - 1) + slot * 2;
+  return (
+      unsigned)(kIsLittleEndian ? offset_bytes * CHAR_BIT + slot * 2 : CHAR_BIT * (sizeof(uint32_t) - offset_bytes - 1) + slot * 2);
 }
 
 inline uint32_t MicroLockCore::heldBit(unsigned slot) const {

diff --git a/folly/test/SmallLocksBenchmark.cpp b/folly/test/SmallLocksBenchmark.cpp
index 19830c0f2f000a09e654ba22a04ebdd9a4a08656..7d67883ddc634e84c4ccc4036d5a5f80663fc47e 100644 (file)
--- a/folly/test/SmallLocksBenchmark.cpp
+++ b/folly/test/SmallLocksBenchmark.cpp
@@ -59,7 +59,7 @@ struct VirtualBase {
 };
 
 struct VirtualImpl : VirtualBase {
-  virtual void foo() { /* noop */
+  void foo() override { /* noop */
   }
   virtual ~VirtualImpl() {}
 };