1. 13 Apr, 2021 7 commits
    • Rob Sherwood's avatar
      Mechanism to fire callbacks on new object construction · d31902b9
      Rob Sherwood authored
      Summary:
      Create a new class, ConstructorCallback, that when included as a member in
      another class (e.g., Foo) allows other functions to register and receive
      callbacks every time any of Foo's constructors are called.
      
      Main implementation concern is to be extremely lightweight (e.g., avoid locks)
      assuming that most code calling this will be performance optimized (both
      for CPU and mem).
      
      Implement as a static array of functions:
      
      1) Static array instead of dynamic (e.g., vector):
        vector occasionally resizes so avoid locking during resize
      2) Don't allow removing callback functions:
        simplify the calling of funcitons, again to avoid locks and races
      3) Use function callbacks instead of objects with an observer pattern
        (e.g., like AsyncSocket::LifecycleObserver) to ensure that there
        won't be races on shutdown.  Functions are static where classes can
        de-allocate at shutdown and since we don't allow un-registering the
        call back, it's not easy to remove the race for a class.
      
      Reviewed By: simpkins
      
      Differential Revision: D27056139
      
      fbshipit-source-id: 0846e0d55124cfde2e15acbe18e01ca5e327e7df
      d31902b9
    • Giuseppe Ottaviano's avatar
      Improve cpu id caching in DigestBuilder · 6f9321c4
      Giuseppe Ottaviano authored
      Summary:
      `DigestBuilder` caches the cpu id and invalidates it when it detects contention, but the critical section is small enough that it is possible for two thread to end up on a conflicting slot but not detect it for some time, causing cache-line pingponging in the meantime.
      
      Switch to `cachedCurrent()` instead, which uses the number of accesses to invalidate the cache, and is successfully adopted by other high-concurrency primitives.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D27726518
      
      fbshipit-source-id: 05d70ad9f101df4a8797e44cc3cbe94cbd1cb7b8
      6f9321c4
    • Andrii Grynenko's avatar
      Patch unit tests to work with GCC · d5bf7f5a
      Andrii Grynenko authored
      Summary:
      GmockHelpersTest.cpp is the only one that fails to build with GCC after this change.
      Also only 5 tests are failing at this point.
      
      Reviewed By: pixelb
      
      Differential Revision: D27728248
      
      fbshipit-source-id: ded994f466347a8826ba90429a13237c21d3af5e
      d5bf7f5a
    • Nathan G Bronson's avatar
      avoid operator!= ambiguity in c++20 (#1544) · b471fe2a
      Nathan G Bronson authored
      Summary:
      F14 map `iterator` doesn't define `operator==` or `operator!=` to itself.
      Those member functions take `const_iterator` as the right-hand side,
      relying on the ability to convert implicitly from `iterator` to
      `const_iterator`. This interacts poorly with c++20's default equality
      operators, triggering clang's `-Wambiguous-reversed-operator`.
      
      This diff replaces the equality member functions with friends that use
      the same left-hand and right-hand types.  iterator != const_iterator
      will now compile by implicitly converting the left-hand-side and then
      calling `operator!=(const_iterator const&, const_iterator const&)`,
      rather than calling `iterator::operator!=(const_iterator const&)`.
      This resolves the ambiguity in c++20, and should work fine on earlier
      versions, although I have not tested it on anything except c++17.
      
      Pull Request resolved: https://github.com/facebook/folly/pull/1544
      
      Test Plan: does it build?
      
      Reviewed By: yfeldblum
      
      Differential Revision: D27479086
      
      Pulled By: shixiao
      
      fbshipit-source-id: 0d42dbdc056cec58a36de3a816883736a761d0f6
      b471fe2a
    • Pranjal Raihan's avatar
      Refer to type names consistently · 692d08fb
      Pranjal Raihan authored
      Summary: Consistently use `Observer` and `Snapshot` like everywhere else in these files instead of `folly::Observer::{Observer,Snapshot}`.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D27685699
      
      fbshipit-source-id: 7eca35b9d113d8d9cd8ca631be2ad54c827431ea
      692d08fb
    • Chad Austin's avatar
      remove unused LockFreeRingBuffer functionality · 89383c3a
      Chad Austin authored
      Summary:
      Nothing uses the skipFraction in LockFreeRingBuffer::currentTail, and
      floating point math for discrete fractions can be hard to reason
      about, so remove this dead code.
      
      Differential Revision: D27717906
      
      fbshipit-source-id: b25e2bea25ab66c59e8c76a80d75140db474c46d
      89383c3a
    • Aleksandr Sasha Sergeev's avatar
      Use different parse errors for NaN and INF double values. · d4b9778f
      Aleksandr Sasha Sergeev authored
      Summary: To facilitate debugging of T88337980
      
      Reviewed By: rdavies
      
      Differential Revision: D27610329
      
      fbshipit-source-id: 6b01af24e2cfdda9afcbef10d2951ac071d2fd10
      d4b9778f
  2. 12 Apr, 2021 6 commits
  3. 11 Apr, 2021 1 commit
  4. 10 Apr, 2021 6 commits
    • Chad Austin's avatar
      default LockFreeRingBuffer to trivial and make it compatible with TSAN · 68104e03
      Chad Austin authored
      Summary:
      LockFreeRingBuffer is racy for nontrivial types. I could reproduce
      ASAN failures with nontrivial types like std::string, and TSAN
      failures with trivial types.
      
      Require triviality by default, and sequence the turns, loads, and
      stores with atomic_thread_fence.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D27684743
      
      fbshipit-source-id: e0b32855d3ed5c2edbd7ab72092f2c533071a70f
      68104e03
    • Chad Austin's avatar
      always copy into LockFreeRingBuffer writes · 15ee6d71
      Chad Austin authored
      Summary:
      In preparation for making LockFreeRingBuffer trivial-only, remove the
      ability to convert types during reads and writes.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D27684584
      
      fbshipit-source-id: 092009b2852d000fa3821756d1ec4bfd2bf002c8
      15ee6d71
    • Yedidya Feldblum's avatar
      make SequentialThreadId not a template · 2b9c0388
      Yedidya Feldblum authored
      Summary:
      `SequentialThreadId` is only ever instantiated with `std::atomic` so just make it no longer a template.
      
      This permits moving the implementation into the source file.
      
      Reviewed By: Mizuchi
      
      Differential Revision: D27669406
      
      fbshipit-source-id: ec2cddbff433df31434548b61111a34ca2da5cef
      2b9c0388
    • Henry Wang's avatar
      Fix compiler warnings in small_vector · 266ca3b2
      Henry Wang authored
      Summary:
      Fix a bug in small_vector reported as a warning via gcc.
      
      ```
      folly/small_vector.h: In instantiation of 'constexpr const size_t folly::small_vector<short int, 10>::MaxInline':
      folly/small_vector.h:1183:56:   required from 'class folly::small_vector<short int, 10>'
      bits/stl_stack.h:134:47:   required from 'class std::stack<short int, folly::small_vector<short int, 10> >'
      folly/small_vector.h:462:36: error: division 'sizeof (short int*) / sizeof (short int)' does not compute the number of array elements [-Werror=sizeof-pointer-div]
        462 |       constexpr_max(sizeof(Value*) / sizeof(Value), RequestedMaxInline)};
            |                     ~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~
      ```
      
      Reviewed By: yfeldblum
      
      Differential Revision: D27688663
      
      fbshipit-source-id: 8da5b017c597c542826e26f2c36b0e3b46e87ed9
      266ca3b2
    • Yedidya Feldblum's avatar
      migrate from uint64ToBufferUnsafe · c8613fb4
      Yedidya Feldblum authored
      Summary: Migrate the codebase away from the ad-hoc `folly::uint64ToBufferUnsafe` and to `folly::to_ascii_decimal` which is intended for these cases.
      
      Reviewed By: WillerZ
      
      Differential Revision: D27281875
      
      fbshipit-source-id: 0c98749e4aed9c873853eed2221cf54a89279ff4
      c8613fb4
    • Andrii Grynenko's avatar
      Fix a shutdown race in CPUThreadPoolExecutor::add · b91dd2f6
      Andrii Grynenko authored
      Summary:
      It's not safe to expect that the executor is alive after a task is added to the queue (this task could be holding the last KeepAlive and when finished - it may unblock the executor shutdown).
      The fix is to hold a KeepAlive if some operation has to be done after adding to the queue. However it's safe to avoid this in a case where executor thread count will never drop below 1 (otherwise not having active threads may result in a deadlock) and we already have the maximum number of threads running. This optimization should help avoid grabbing the KeepAlive on the fast path.
      
      Differential Revision: D27584518
      
      fbshipit-source-id: e1242e3f4c40cee4f7e0c6dfca39abe6d17415f1
      b91dd2f6
  5. 09 Apr, 2021 4 commits
    • Parvez Shaikh's avatar
      get rid of hack to support label attribute for 1.7.0 · 7c4413da
      Parvez Shaikh authored
      Summary:
      as titled, an ugly hack to support LABEL attribute for 1.7.0 which didn't exist.
      
      getting rid of it now since oss support upgraded to 1.7.1
      
      Differential Revision: D27536730
      
      fbshipit-source-id: 58653e71cbf20a1cda0b7414f66b5f0014d89b84
      7c4413da
    • Alexander Sklar's avatar
      Conditionally include fmt/format.h (#1551) · 3d0ae974
      Alexander Sklar authored
      Summary:
      Addresses - at least partly - issue https://github.com/facebook/folly/issues/1550
      
      We'd like to limit our exposure to additional libraries. Fmt is a new dependency. It doesn't look like we need it in actuality; gating the include based on whether the file exists or not makes it possible for clients (specifically React Native for Windows) to not need to fork Folly.
      
      Pull Request resolved: https://github.com/facebook/folly/pull/1551
      
      Reviewed By: yfeldblum
      
      Differential Revision: D27531237
      
      Pulled By: Orvid
      
      fbshipit-source-id: 340a7ff49be81872aaf23044945a1e8ecd157546
      3d0ae974
    • Giuseppe Ottaviano's avatar
      Add a function to check if the process is in crashing · dced0133
      Giuseppe Ottaviano authored
      Summary:
      This can be used to avoid noisy logging if the process is in the middle of a crash: symbolizing the stack trace can take a long time, so we can end up interleaving the crash report with unrelated logs.
      
      In particular, we can use this to guard monitoring code that is more likely to trip while the process is crashing (for example stall detectors).
      
      Reviewed By: yfeldblum, philippv, luciang
      
      Differential Revision: D27663380
      
      fbshipit-source-id: e3bb40292c3c57579b3eb172847ca089b0f9b07a
      dced0133
    • Yedidya Feldblum's avatar
      revert AccessSpreader thread-local and extern-template-struct · 6902012a
      Yedidya Feldblum authored
      Summary: They are observed to trigger pathological behavior.
      
      Differential Revision: D27659117
      
      fbshipit-source-id: 5e4e770e61ffde02075364f2247614cf5bbcf8ce
      6902012a
  6. 08 Apr, 2021 1 commit
  7. 07 Apr, 2021 3 commits
    • Genevieve Helsel's avatar
      flag manual import so autodeps won't add symbolizer to default init deps · eb3f0d20
      Genevieve Helsel authored
      Summary: as title, marking this import as manual will make it so autodeps won't double list it in the TARGETS file.
      
      Reviewed By: yfeldblum, luciang
      
      Differential Revision: D27418310
      
      fbshipit-source-id: 0c32ff1ad0ad7997a83b050f7fe3cdc522f582cc
      eb3f0d20
    • Seth Hinze's avatar
      Add ability to specify an offset to IOBuf::takeOwnership · c09d7af4
      Seth Hinze authored
      Summary: Allow callers to `IOBuf::takeOwnership()` to specify valid data in the middle of the underlying buffer.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D27583632
      
      fbshipit-source-id: 4651dec5e53c5eea341b2b4ad3782adfbe7ff779
      c09d7af4
    • Yedidya Feldblum's avatar
      excise AccessSpreader extern-template-struct · 67c99b13
      Yedidya Feldblum authored
      Summary:
      There appear always to be problems with `extern template struct`. In this case, some compilers either not emitting or dropping thread-local wrapper functions.
      
      ```
      stderr: Undefined symbols for architecture x86_64:
        "thread-local wrapper routine for folly::AccessSpreader<std::__1::atomic>::cpuCache", referenced from:
            folly::AccessSpreader<std::__1::atomic>::cachedCurrent(unsigned long) in CacheLocality.cpp.o
        "thread-local wrapper routine for folly::SequentialThreadId<std::__1::atomic>::currentId", referenced from:
            folly::SequentialThreadId<std::__1::atomic>::get() in CacheLocality.cpp.o
      ld: symbol(s) not found for architecture x86_64
      clang: error: linker command failed with exit code 1 (use -v to see invocation)
      ```
      
      Reviewed By: Mizuchi
      
      Differential Revision: D27579816
      
      fbshipit-source-id: b539816fb1f62d9e50e22d6cbc3c91a0d82ac629
      67c99b13
  8. 06 Apr, 2021 4 commits
    • Yedidya Feldblum's avatar
      outline AccessSpreader::initialize · ad41ba19
      Yedidya Feldblum authored
      Summary: Extract its content to an outline function and pass it the template immediates it needs.
      
      Reviewed By: Mizuchi
      
      Differential Revision: D27579474
      
      fbshipit-source-id: 193c769d83f886b4fefea4a34118c44f7da08bd1
      ad41ba19
    • Genevieve Helsel's avatar
      make libunwind portability header · b98c937c
      Genevieve Helsel authored
      Summary: Adds a portability header for later ease of use when deciding to include external_deps libunwind in TARGETS files
      
      Reviewed By: yfeldblum
      
      Differential Revision: D27416623
      
      fbshipit-source-id: fc42f010302b8caa280fdf5cbdf013ed1b23af04
      b98c937c
    • Yedidya Feldblum's avatar
      let to_ascii_port_clzll always return non-void · 54c929bc
      Yedidya Feldblum authored
      Summary: Required for msvc 32-bit builds - the result of `to_ascii_port_clzll` is used in an arithmetic expression.
      
      Reviewed By: SarahDesouky, Orvid
      
      Differential Revision: D27585004
      
      fbshipit-source-id: b84b5b6d6fe417f3c6adafcaf826505275aa827c
      54c929bc
    • Alan Huang's avatar
      revert default value of flag folly_memory_idler_purge_arenas · 897fd4bf
      Alan Huang authored
      Reviewed By: yfeldblum
      
      Differential Revision: D27575037
      
      fbshipit-source-id: fa5817ee3fe1eca99e83f26a13e43ec4a4a6e7cf
      897fd4bf
  9. 05 Apr, 2021 3 commits
    • Andrii Grynenko's avatar
      Make getWeakRef preserve SequencedExecutor tag · 2ff21656
      Andrii Grynenko authored
      Reviewed By: yfeldblum
      
      Differential Revision: D27253836
      
      fbshipit-source-id: a72531e0deca9f0f6582339d483ed56f2eb37d87
      2ff21656
    • Seth Hinze's avatar
      Validate IOBuf capacity at construction time · 355fec90
      Seth Hinze authored
      Summary: Add a CHECK() to the IOBuf constructor to ensure the underlying buffer does not include any bytes that have been poisoned by AddressSanitizer.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D25627483
      
      fbshipit-source-id: 5bca59e545c32ba7b61430d7cd4787712eecb204
      355fec90
    • Yedidya Feldblum's avatar
      avoid FOLLY_TLS in tests · 71255360
      Yedidya Feldblum authored
      Reviewed By: Orvid
      
      Differential Revision: D27559344
      
      fbshipit-source-id: cf90be5a19da87bffe72fb6646296dd46889bc90
      71255360
  10. 04 Apr, 2021 2 commits
    • Stiopa Koltsov's avatar
      File::dupCloseOnExec() · 1af19a96
      Stiopa Koltsov authored
      Summary:
      Close-on-exec is an important feature to avoid leaking file descriptors to spawned processes.
      
      This diff adds `File::dupCloseOnExec()` function which is equivalent to `File::dup` function, but sets close-on-exec flag where supported (i. e. everywhere except Windows).
      
      Practically most users want to have `closeOnExec = true` by default, but we need to preserve backwards compatibility, thus this diff leaves `dup` function as is.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D27495214
      
      fbshipit-source-id: 540deb2bc6c8fec626a0120bc20c345950dc8af7
      1af19a96
    • Yedidya Feldblum's avatar
      support disabling tests in cmake under apple · 03fa494f
      Yedidya Feldblum authored
      Reviewed By: Orvid
      
      Differential Revision: D27555992
      
      fbshipit-source-id: d68dc183f7c7b7c64975ede21e6f3018575fb7c9
      03fa494f
  11. 03 Apr, 2021 3 commits
    • Yedidya Feldblum's avatar
      multiple tags per test in cmake build · 4033d186
      Yedidya Feldblum authored
      Reviewed By: Orvid
      
      Differential Revision: D27537202
      
      fbshipit-source-id: d1aa8ad81f3749129dd6f1795e788a0e55d9f06a
      4033d186
    • Yedidya Feldblum's avatar
      avoid forcing the type of clzll · c841c365
      Yedidya Feldblum authored
      Summary: Between gnu and msvc compilers, the builtins and intrinsics `__builtin_clzll` and `__lzcnt64` have different return types. They are wrapped in a portability function in `to_ascii` which currently forces a return type, which return type could trip implicit-conversion warnings. Avoid forcing the return type to avoid tripping implicit-conversion warnings.
      
      Reviewed By: luciang
      
      Differential Revision: D27535348
      
      fbshipit-source-id: 3339d5d04e095f936c18d2b39ac1fb4b32f8a810
      c841c365
    • Yedidya Feldblum's avatar
      avoid ambiguity in to_ascii_size · 5ef57d70
      Yedidya Feldblum authored
      Summary: The ambiguity is that the name appears as both `folly::to_ascii_size` and `folly::detail::to_ascii_size`, so that anything which imports both `folly` and `folly::detail` or which is within `folly::detail` may, depending on the particular compiler, find an unqualified call ambiguous.
      
      Differential Revision: D27535255
      
      fbshipit-source-id: 05db44e6b153d75b691d67c878785cab1655051d
      5ef57d70