1. 20 Jul, 2018 4 commits
    • Matthieu Martin's avatar
      Revert Folly NestedRequestContextGuard · 20eea8f0
      Matthieu Martin authored
      Summary:
      This attempted to provide a deep copy guard for folly request context.
      
      Per brainstorming (see task), we don't think that deep copying all metadata has legitimate use cases. And in fact, this has no usage in our codebase currently. Because it default to reset (when createChild is not overriden), the behaviour is also very confusing.
      
      Guards's goal is usually to copy 1 metadata. The solution for that is to provide a shallow copy guard. Something that I will do on top of this revert.
      I'll wait for both changes to be discussed and accepted before landing this one.
      
      Reviewed By: yfeldblum, LeeHowes
      
      Differential Revision: D8906912
      
      fbshipit-source-id: e8b9eed04cbe539009037ff75d51e28941502d88
      20eea8f0
    • Matthieu Martin's avatar
      Fix set/unset for default folly RequestContext · dec334ce
      Matthieu Martin authored
      Summary:
      My guess is that if we allow to set data in the default context, we also expect set/unset to work.
      It currently doesn't. This fixes it (at least, the test repro passes), but not sure it's the right fix.
      
      Reviewed By: djwatson
      
      Differential Revision: D8910506
      
      fbshipit-source-id: cedf1dd0ee91761d210137949da7477ed69fce31
      dec334ce
    • Yedidya Feldblum's avatar
      Comments to document explicit conversions from Range · 09c55d0a
      Yedidya Feldblum authored
      Summary: [Folly] Comments to document explicit conversions from `Range`.
      
      Reviewed By: terrelln
      
      Differential Revision: D8921990
      
      fbshipit-source-id: 0728e0096738f9a86eecfaa5b05d4dfbcb0533b6
      09c55d0a
    • Yedidya Feldblum's avatar
      Cut non-const conversion overloads from Range · b5f590c2
      Yedidya Feldblum authored
      Summary: [Folly] Cut non-`const` conversion overloads from `Range`, for both `operator T` and `to<T>`: let them be `const` member functions so that the converted-to object cannot update `Range` internal state.
      
      Reviewed By: terrelln, vitaut
      
      Differential Revision: D8921428
      
      fbshipit-source-id: eefc14b5fe2033cac43ebc274b27bdea95388488
      b5f590c2
  2. 19 Jul, 2018 2 commits
  3. 18 Jul, 2018 8 commits
    • Yang Zhang's avatar
      Add ThreadPoolExecutor::numActiveThreads() · ac6b1e28
      Yang Zhang authored
      Summary: ThreadPoolExecutor could dynamically adjust number of threads according to workload. Add numActiveThreads() so we can check how many active threads are actually there, while numThreads() returns the upper bound of threads.
      
      Reviewed By: djwatson
      
      Differential Revision: D8683795
      
      fbshipit-source-id: 09f3b4ee8570e2f2f9f97e939061693f3e0639af
      ac6b1e28
    • Maged Michael's avatar
      hazptr: Add function to reclaim linked objects without checking hazard pointers. · bc809fb4
      Maged Michael authored
      Summary:
      Add unlink_and_reclaim member function to hazptr_obj_base_linked, as an alternative to unlink() that reclaims the object if its link count is zero without checking hazard pointers.
      It is useful in destructors of data structures when it is guaranteed that objects are not protected by hazard pointers (e,.g., destructor of ConcurrentHashMap).
      
      Reviewed By: yfeldblum
      
      Differential Revision: D8864019
      
      fbshipit-source-id: 618eeded45a1f8b9503569d1ceca9994ca6e317f
      bc809fb4
    • Doron Roberts-Kedes's avatar
      DeterministicSchedule: Deschedule parent threads while they wait to join child threads. · 49a83b77
      Doron Roberts-Kedes authored
      Summary:
      Eliminate while loop behavior in DeterministicSchedule::join
      
      Depends on D8789304
      
      Reviewed By: djwatson
      
      Differential Revision: D8840276
      
      fbshipit-source-id: f0b0db60f7bec344d021a49cec408a2604d4a1c8
      49a83b77
    • Doron Roberts-Kedes's avatar
      DeterministicSchedule: Deschedule threads waiting to acquire DeterministicMutex · 566f0478
      Doron Roberts-Kedes authored
      Summary: Eliminate spinlock behavior from DeterministicMutex::lock by descheduling threads waiting to acquire the mutex, and placing the thread local semaphore in a waitqueue for the mutex. The unlocking thread reschedules a single waiting thread if the workqueue is non empty.
      
      Reviewed By: djwatson
      
      Differential Revision: D8789304
      
      fbshipit-source-id: 8ffe3e289c9abfe7515b678ff98f0cefef2461c0
      566f0478
    • Chad Austin's avatar
      Drain ManualExecutor on destruction · a72a920c
      Chad Austin authored
      Summary:
      For use in tests where the executor has a nontrivial lifetime but any
      queued jobs should be completed, have ManualExecutor drain itself
      before it's destroyed.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D8845457
      
      fbshipit-source-id: 7c2aa65aa27a7850ff73a93cfbe34c2248b62d26
      a72a920c
    • Aaryaman Sagar's avatar
      Remove const wlock acquisition · 37c011d8
      Aaryaman Sagar authored
      Summary:
      Change the write acquire interface to only acquire write locks on non-const
      `Synchronized` instances.  This reflects the intention of mutation associated
      with a write lock.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D8851208
      
      fbshipit-source-id: 5be9d2e4a38d25632430a9b6c981b6e7f8412daa
      37c011d8
    • Dan Melnic's avatar
      Fix broken opt build due to "error: unused variable 'rv'" · 9793a0e7
      Dan Melnic authored
      Summary: Fix broken opt build due to "error: unused variable 'rv'"
      
      Reviewed By: yfeldblum
      
      Differential Revision: D8883701
      
      fbshipit-source-id: 2a832cfea0bd9cbca63a8d5c2c8d7127593728af
      9793a0e7
    • Aaryaman Sagar's avatar
      Remove const folly::Synchronized upgrade lock acquire · ea29a7a6
      Aaryaman Sagar authored
      Summary: Users acquiring upgrade locks should only be allowed to lock the synchronized object when it is non-const.  Because an upgrade locks are permitted to transition to write locks when mutation is required.  At which point non-const access is often required on the Synchronized object.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D8654360
      
      fbshipit-source-id: c2a6574bd880db0d8e4c90166e288f5b83542ad6
      ea29a7a6
  4. 17 Jul, 2018 2 commits
    • Nathan Bronson's avatar
      mark uses of folly::assume used to optimize placement new · 0aa15e94
      Nathan Bronson authored
      Summary:
      F14 currently has numerous places where folly::assume is used to
      help optimization on GCC < 6.  These are all things the optimizer should
      be able to deduce itself, so it would be nice to eventually remove them.
      This diff marks them with TODO(T31574848), as well as removing two
      occurrences in F14Table that duplicated an assume in F14Policy.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D8849832
      
      fbshipit-source-id: e88143b4d29f1a633b51467206c6fb6afd1cd296
      0aa15e94
    • Nick Cooper's avatar
      Remove the ' ' prior to : in folly::json's pretty output. · 001320e2
      Nick Cooper authored
      Summary: Remove the ' ' prior to : in folly::json's pretty output - this improves consistency with other JSON formatters.
      
      Reviewed By: yfeldblum, luciang
      
      Differential Revision: D8843977
      
      fbshipit-source-id: 1d5c84d7d3806ad8752b619ffe23101f7f103683
      001320e2
  5. 16 Jul, 2018 2 commits
    • Maged Michael's avatar
      UnboundedQueue: Add cleanup of remaining items at destruction. · f96d7622
      Maged Michael authored
      Summary:
      - Add cleanup of remaining items, if any, at destruction.
      - Add test.
      
      Reviewed By: davidtgoldblatt
      
      Differential Revision: D8860966
      
      fbshipit-source-id: e3ab2e6ff31e08d91aa20c8c058471823c722a38
      f96d7622
    • Maged Michael's avatar
      hazptr: Add test to detect no reclamation, without calling cleanup. · c5f46707
      Maged Michael authored
      Summary: Add test to detect no reclamation (without calling hazptr_cleanup). The test retires a number of objects that would trigger bulk reclamation. One or more objects are expected be reclaimed. The number of retired objects must be greater than or equal to hazptr_domain::kThreshold to expect reclamation to happen.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D8849957
      
      fbshipit-source-id: ef590af21a55348ed0ed72be3637853eceb21cbc
      c5f46707
  6. 14 Jul, 2018 5 commits
    • Marshall Cline's avatar
      remove lvalue-qual Future::get(...) · 0b1134d2
      Marshall Cline authored
      Summary:
      This is part of "the great r-valuification of folly::Future":
      
      * This is something we should do for safety in general.
      * Using lvalue-qualified `Future::get(...)` has caused some failures around D7840699 since lvalue-qualification hides that operation's move-out semantics - leads to some use of future operations that are really not correct, but are not obviously incorrect.
      * Problems with `Future::get(...) &`: it moves-out the result but doesn't invalidate the Future - the Future remains (technically) valid even though it actually is partially moved-out. Callers can subsequently access that moved-out result via things like `future.get()`, `future.result()`, `future.value()`, etc. - these access an already-moved-out result which is/can be surprising.
      * Reasons `Future::get(...) &&` is better: its semantics are more obvious and user-testable. It moves-out the Future, leaving it with `future.valid() == false`.
      * Note: `get(...)` refers to `get()` and `get(Duration)`.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D8710296
      
      fbshipit-source-id: ae201af1928eb2f6a2897c9b7db884393b36b910
      0b1134d2
    • Maged Michael's avatar
      hazptr: Fix hazptr_priv::push_all_to_domain to try reclamation when not in dtor · 41ed7dd9
      Maged Michael authored
      Summary: The most recent change had a bug that prevents all calls to hazptr_priv::push_all_to_domain from trying reclamation, instead of preventing that only when called from the destructor of hazptr_priv and during reclamation.
      
      Reviewed By: davidtgoldblatt
      
      Differential Revision: D8849738
      
      fbshipit-source-id: 15a27e8d4a88288179609e8cf179bc8c10c96b90
      41ed7dd9
    • Nathan Bronson's avatar
      optimize first insertion into F14 hash table · cf17895f
      Nathan Bronson authored
      Summary:
      This diff performs several micro-optimizations that improve
      the lifecycle of a single-element F14FastMap or F14FastSet by about 10%
      (in a single-threaded microbenchmark).  Lifecycle here means construction,
      insertion of one entry, and then destruction, so it includes one malloc
      and one free.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D8771446
      
      fbshipit-source-id: 95d59f32de5a450b16ecdcb0e39b7f566ce797da
      cf17895f
    • Adam Simpkins's avatar
      logging: add XCHECK() and XDCHECK() macros · 5c438fac
      Adam Simpkins authored
      Summary:
      This adds `XCHECK()` and `XDCHECK()` macros to the folly logging library.
      These are similar to glog's `CHECK()` and `DCHECK()` macros, and should make
      it easier for users to convert from glog to folly logging.
      
      `XCHECK(condition)` is basically an alias for `XLOG_IF(FATAL, condition)`
      
      `XDCHECK(condition)` is like `XCHECK(condition)` in non-debug builds, but is
      compiled out entirely in debug builds.  Note that this is *not* like
      `XLOG_IF(DFATAL, condition)`, which still evaluates the condition but avoids
      crashing (logging the message only) in release builds.
      
      Reviewed By: mnv104
      
      Differential Revision: D8817270
      
      fbshipit-source-id: 86c4575e11af37219b30eda4e7e30273e1e32ab1
      5c438fac
    • Adam Simpkins's avatar
      logging: fix the behavior of XLOG_IF(FATAL, condition) · e8c31188
      Adam Simpkins authored
      Summary:
      Previously `XLOG_IF(FATAL, condition)` always crashed regardless of the
      condition check.  When `XLOG_IF()` was added it did not update the checks used
      to mark the statement as `[noreturn]` based on the log level.  As a result
      `XLOG_IF(FATAL, ...)` always used the `[noreturn]` APIs, even though this code
      can return if the condition is not true.
      
      This splits the `XLOG()` and `XLOG_IF()` implementations so that `XLOG(FATAL)`
      can still be marked as `noreturn` but `XLOG_IF(FATAL, ...)` is no `noreturn`.
      
      Reviewed By: yfeldblum, mnv104
      
      Differential Revision: D8817269
      
      fbshipit-source-id: 47a493eaaac69c563cff07da0888dd423f7dc07d
      e8c31188
  7. 13 Jul, 2018 7 commits
    • Chad Austin's avatar
      clang-format folly/executors/ · 056e121f
      Chad Austin authored
      Summary:
      Run clang-format across folly/executors/.
      
      ```
      find . \( -iname '*.cpp' -o -iname '*.h' \) -exec clang-format -i {} \;
      ```
      
      Reviewed By: yfeldblum
      
      Differential Revision: D8843064
      
      fbshipit-source-id: 0a3c82083eebf2c684a4ab2e12067f0f742bf1d4
      056e121f
    • Louis Brandy's avatar
      make UtilityTest compile with c++17. · 1d2790bc
      Louis Brandy authored
      Summary:
      C++17 actually removes this overload of `std::addressof` to avoid taking the address of constants. It's not clear to me that the second test here is actually adding much value here.
      
      See (2) at: https://en.cppreference.com/w/cpp/memory/addressof
      
      Reviewed By: yfeldblum, elsteveogrande
      
      Differential Revision: D8775544
      
      fbshipit-source-id: a42209484574509f4d032ebbdf05430f0ed372c4
      1d2790bc
    • Adam Simpkins's avatar
      logging: make IntervalRateLimiter constexpr-constructible · 1e877625
      Adam Simpkins authored
      Summary:
      This changes `IntervalRateLimiter` to allow it to be `constexpr`-constructible.
      
      We now always initialize the `timestamp_` field to 0.  The very first call to
      `check()` will now always call `checkSlow()` which will then initialize
      `timestamp_` properly.
      
      This also removes the pure virtual `RateLimiter` interface class.  At the
      moment `IntervalRateLimiter` is the only implementation, and all call sites
      use this class directly.  We can always add the `RateLimiter` interface back
      in the future if we need it later.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D8798167
      
      fbshipit-source-id: 80885a16506a8daa67653bd0a92accae7a973289
      1e877625
    • Adam Simpkins's avatar
      logging: reformat fatal_test.py with black · 16ae8fd4
      Adam Simpkins authored
      Summary: Reformat fatal_test.py with black (https://github.com/ambv/black)
      
      Reviewed By: mnv104
      
      Differential Revision: D8817268
      
      fbshipit-source-id: b642496ac61e3b2120b76d9b234c29bf51603651
      16ae8fd4
    • Adam Simpkins's avatar
      logging: update some static lambda variable names · 7af069c3
      Adam Simpkins authored
      Summary:
      Update the static local variable names used in `XLOG_IMPL()` and
      `XLOG_IS_ON_IMPL()` to match the naming style used in D8218663.
      
      The current version of clang-format also appears to be able to format
      `XLOG_IMPL()` correctly now, so remove the comments disabling it around this
      macro body.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D8816625
      
      fbshipit-source-id: 155b759953de5d5db0b350f27870edf9f5516914
      7af069c3
    • Adam Simpkins's avatar
      logging: Add rate limiting log macros · 130fca21
      Adam Simpkins authored
      Summary:
      This adds several macros for explicitly rate-limited log messages.
      
      - `XLOG_EVERY_N()` logs only 1 of every N times it is called.
        This is similar to glog's `LOG_EVERY_N()` and `VLOG_EVERY_N()` macros.
      
      - `XLOG_EVERY_MS()` logs only once per specified time interval.
        This is similar to the `LOG_EVERY_MS()` and `LOG_EVERY_MS_ATOMIC()` macros
        that Facebook has defined internally on top of glog.
      
      - `XLOG_N_PER_MS()` logs the first N messages per specified time interval.
      
      These should make it easier for Facebook programs to migrate from glog to
      xlog.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D8218663
      
      fbshipit-source-id: a1e71265ace41fea95e5dbb79bc4381962b11297
      130fca21
    • Dave Watson's avatar
      Revert D8397670 D8797823 · 3d61f8ac
      Dave Watson authored
      Summary:
      Delayed hazptr destruction is causing destruction ordering issues.
      
      P59812860$445
      
      buck test //titan/cachius/test:CachiusServiceHandlerTest
      
      Reviewed By: phil-lopreiato
      
      Differential Revision: D8825716
      
      fbshipit-source-id: 808341489fd80b8edb63eb7c12377a8ffadfd1eb
      3d61f8ac
  8. 12 Jul, 2018 5 commits
    • Andrii Grynenko's avatar
      Make fibers::Baton awaitable · 2c76542e
      Andrii Grynenko authored
      Summary:
      This extends fibers::Baton to support any awaiter (not just folly::Fiber) which makes it possible to integrate it with coroutines.
      This will make it easy to integrate existing synchronization primitives that use fibers::Baton with coroutines.
      
      Reviewed By: lewissbaker
      
      Differential Revision: D8580775
      
      fbshipit-source-id: 8f8593793d3012e0470b8f9b3bcf2713145d3573
      2c76542e
    • Maged Michael's avatar
      hazptr: Prevent destructor of hazptr_priv from getting reference to itself · 4bfcf7b4
      Maged Michael authored
      Summary: Prevent destructor of hazptr_priv which is a SingltonThreadLocal from making call to get a reference to the singlton. This may be a problem on some platforms.
      
      Reviewed By: djwatson
      
      Differential Revision: D8711918
      
      fbshipit-source-id: fb319f640cff74e46ce27ab28ae5fbf93961b262
      4bfcf7b4
    • Nathan Bronson's avatar
      IsAvalanchingHasher improvements and fixes · fb02412a
      Nathan Bronson authored
      Summary:
      Specializing folly::IsAvalanchingHasher can be bulky and
      mistake-prone, such as constructing a new hash functor by subclassing.
      For example, folly::IsAvalanchingHasher<folly::transparent<H>, K>
      was incorrectly false even when H is avalanching.  This diff adds the
      ability to use a member type is_avalanching to accomplish the same
      effect, and also fixes the computation of folly::IsAvalanchingHasher
      for folly::transparent.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D8772423
      
      fbshipit-source-id: 49e4e33b2981efd8c302d9a217dc9df0dcb290cc
      fb02412a
    • Sven Over's avatar
      also test Function move construction from empty Function · 39ef9832
      Sven Over authored
      Summary: The `Emptiness_T` test already tests the emptiness of a default-constructed Function as well as one that was assigned an empty Function. This diff adds a test for a Function that was move-constructed from an empty Function.
      
      Reviewed By: shixiao
      
      Differential Revision: D8818468
      
      fbshipit-source-id: 3f0e04b9f0f44df2de45fc972a2d26e12adc7362
      39ef9832
    • Nathan Bronson's avatar
      remove unnecessary folly:: qualification in F14 · 787dc636
      Nathan Bronson authored
      Summary: F14 code inside namespace folly doesn't need folly::
      
      Reviewed By: yfeldblum
      
      Differential Revision: D8772565
      
      fbshipit-source-id: 720fa683b7ec9be9a096559191240ef7e672fe3e
      787dc636
  9. 11 Jul, 2018 5 commits
    • Yedidya Feldblum's avatar
      Reorder includes in folly/io/async/HHWheelTimer.cpp · c1c59c5b
      Yedidya Feldblum authored
      Summary: [Folly] Reorder `#include`s in `folly/io/async/HHWheelTimer.cpp`.
      
      Reviewed By: djwatson
      
      Differential Revision: D8806020
      
      fbshipit-source-id: 43496acdaac3b8a79169b00913881d17d160906c
      c1c59c5b
    • Dave Watson's avatar
      Summary: · f8e888a0
      Dave Watson authored
      Fix requestcontext destruction before globals are destroyed.
      Bad stacktrace P59804825
      
      Reviewed By: yfeldblum
      
      Differential Revision: D8797823
      
      fbshipit-source-id: a9cbff159e5a0ab2577cb7412817be5d554aa32f
      f8e888a0
    • Marshall Cline's avatar
      deprecate rvalue-qual Future::get(...) · acd11b42
      Marshall Cline authored
      Summary:
      This is part of "the great r-valuification of folly::Future":
      
      * This is something we should do for safety in general.
      * Context: `Future::get(...)` means both `Future::get()` and `Future::get(Duration)`
      * Using lvalue-qualified `Future::get(...)` has caused some failures around D7840699 since lvalue-qualification hides that operation's move-out semantics - leads to some use of future operations that are really not correct, but are not obviously incorrect.
      * Problems with `Future::get(...) &`: it moves-out the result but doesn't invalidate the Future - the Future remains (technically) valid even though it actually is partially moved-out. Callers can subsequently access that moved-out result via things like `future.get(...)`, `future.result()`, `future.value()`, etc. - these access an already-moved-out result which is/can be surprising.
      * Reasons `Future::get(...) &&` is better: its semantics are more obvious and user-testable. It moves-out the Future, leaving it with `future.valid() == false`.
      
      Reviewed By: LeeHowes
      
      Differential Revision: D8799081
      
      fbshipit-source-id: 890a221c84ba4847abfaf6e564b0eceb0176fd54
      acd11b42
    • Jack Montgomery's avatar
      Add data() method to folly sorted vector types · c8eeea45
      Jack Montgomery authored
      Summary: Add a data() method to folly::sorted_vector_map and folly::sorted_vector_set which returns a const pointer to the underlying container's data
      
      Reviewed By: yfeldblum
      
      Differential Revision: D8750840
      
      fbshipit-source-id: d361fe21ac741b31b60f551935a3ed8c9549f11a
      c8eeea45
    • Yao Han's avatar
      fix zstd macro (#885) · 16b84942
      Yao Han authored
      Summary:
      detect FOLLY_HAVE_LIBZSTD by true/false
      Pull Request resolved: https://github.com/facebook/folly/pull/885
      
      Reviewed By: terrelln
      
      Differential Revision: D8807276
      
      Pulled By: yfeldblum
      
      fbshipit-source-id: 4372b925a487a31b26cc4c59d7f513639d864f00
      16b84942