1. 27 Dec, 2021 1 commit
  2. 23 Dec, 2021 1 commit
    • Nicholas Ormrod's avatar
      Fix timing-out ConcurrentAdd test · 5b5e5909
      Nicholas Ormrod authored
      Summary:
      This test regularly times out under load.
      
      I made a benchmark diff (prior to this in the stack, but not to be landed) which confirms (a) that the slowness of this test is due to spawning excessively-many threads, and (b) that the performance of ConcurrentSkipList doesn't unduly degrade with excessively-many threads. Conclusion: it is safe to simplify these tests.
      
      To simplify the test, I ratcheted down the number of threads, but also bumped the number of iterations. Much more work is being done (so this test should retain its race-condition-hunting properties), but the time spent is much less, even when the system is under load.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D33269899
      
      fbshipit-source-id: 9f7a0ec5f3c1077c91e5d09c89f3752d552a6320
      5b5e5909
  3. 22 Dec, 2021 1 commit
    • Nicholas Ormrod's avatar
      Fix broken singleton test · ec7f9f6f
      Nicholas Ormrod authored
      Summary: This test regularly fails on account of timing. Remove the timing upper bound.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D33271470
      
      fbshipit-source-id: 1369ef45bd49e500c35816bfdb21869f346b6f70
      ec7f9f6f
  4. 21 Dec, 2021 4 commits
    • Nicholas Ormrod's avatar
      Mark sorted_vector swap as conditionally noexcept · 6fcea2fc
      Nicholas Ormrod authored
      Summary: swap should be noexcept when possible
      
      Reviewed By: yfeldblum
      
      Differential Revision: D33198827
      
      fbshipit-source-id: f8d52a21087b0346332359b66e1dc00f982ddbbb
      6fcea2fc
    • Alex Hornby's avatar
      add BadgeTest.cpp to cmake build · 16a0583c
      Alex Hornby authored
      Summary:
      Externally this was failing on fedora 35's GCC 11.2.1
      
      Add test so we know what platforms it's good on
      
      Reviewed By: yfeldblum
      
      Differential Revision: D33192636
      
      fbshipit-source-id: 028255b6dbb34762b4e788122d948f26a449a57c
      16a0583c
    • Alex Hornby's avatar
      enable MemcpyTest in cmake · 65d40bbb
      Alex Hornby authored
      Summary: Try enabling this, need to handle linux ASM build case in cmake config
      
      Reviewed By: Gownta
      
      Differential Revision: D33191710
      
      fbshipit-source-id: 6d49a08106c8a0044380f592fd2862080cbea454
      65d40bbb
    • Alex Hornby's avatar
      folly: fixes to renable cmake tests (#1691) · 819becba
      Alex Hornby authored
      Summary:
      Having the tests is useful to be able to test the OSS builds. A few were failing for me locally so I've tagged them BROKEN in CMakeLists.txt which folly's cmake config filters out
      
      Pull Request resolved: https://github.com/facebook/folly/pull/1691
      
      Test Plan:
      tested with:
      
      ```
      ./build/fbcode_builder/getdeps.py --allow-system-packages build
      ./build/fbcode_builder/getdeps.py --allow-system-packages test
      ...
      100% tests passed, 0 tests failed out of 2736
      
      Total Test time (real) =  14.95 sec
      ````
      
      Reviewed By: yfeldblum, Gownta
      
      Differential Revision: D33169409
      
      Pulled By: ahornby
      
      fbshipit-source-id: 9c781a84b8873c295af96368dd8315254a78f096
      819becba
  5. 20 Dec, 2021 2 commits
  6. 19 Dec, 2021 1 commit
    • Yedidya Feldblum's avatar
      rename Function private helper ResultOf · 72229769
      Yedidya Feldblum authored
      Summary: `ResultOf` is no longer the most-suitable name. This helper has long been a SFINAE helper and its actual output did not matter, but now that it is always `void` the name is even less clear. Rename it.
      
      Reviewed By: luciang
      
      Differential Revision: D33155200
      
      fbshipit-source-id: a12842c6be38bd82bfffa85193ec265bc4d803ad
      72229769
  7. 18 Dec, 2021 3 commits
    • Yedidya Feldblum's avatar
      wrappers around operator new and operator delete · c465314c
      Yedidya Feldblum authored
      Summary:
      C++14 introduces sized overloads of `operator delete` and C++17 introduces aligned overloads of `operator new` and `operator delete`. Offer wrappers which call the best available versions.
      
      Using these overloads, asking for heap storage suitable for a non-over-aligned class can be done via these wrappers without requiring call-sites to test feature macros.
      
      Most potential use-cases should use `std::allocator<T>::allocate` and `std::allocator<T>::deallocate`. Reasons why use-cases may turn to these wrappers instead are:
      * These wrappers are not temnplates and using them does not instantiate class or function templates.
      * `std::allocator<T>::deallocate` may, but is not specified to, use sized deallocation when sized deallocation is available.
      
      Reviewed By: luciang
      
      Differential Revision: D33180956
      
      fbshipit-source-id: 944fe6a01ca4cfff72f3b92639158e4a5ac029b8
      c465314c
    • Yedidya Feldblum's avatar
      hide lambda return types from Function ctor symbols · 9a154f6e
      Yedidya Feldblum authored
      Summary: It is possible for return type names to be long. They are present in the ctor symbol names for SFINAE, but we can get around that by inserting a cast-to-void.
      
      Reviewed By: luciang
      
      Differential Revision: D32976770
      
      fbshipit-source-id: d5a972543f4d97f94abea9631a4bed35fa67f8f3
      9a154f6e
    • Yedidya Feldblum's avatar
      a std::decay_t lookalike · a61cf107
      Yedidya Feldblum authored
      Summary: Appears to be measurably faster to compile under clang as compared to `std::decay_t`.
      
      Reviewed By: Alfus, luciang
      
      Differential Revision: D32946561
      
      fbshipit-source-id: 028c79633095ae9d6dc75d967282b53b32986736
      a61cf107
  8. 17 Dec, 2021 10 commits
    • Andrew Smith's avatar
      Add support for rate limiting to transform and resumableTransform · eadf2192
      Andrew Smith authored
      Summary:
      The channels framework provides most of its memory savings by allowing channels to be used without long-lived coroutine frames consuming them. However, coroutines are still used for short-lived transformation functions. While these coroutines should not increase steady state memory (as they are short-lived), it is possible that a large number of transformations occur at exactly the same time. This can cause a large memory spike (due to the size of the coroutine frames), and lead to heap fragmentation.
      
      This diff solves this problem by adding support for rate limiting. An optional RateLimiter object can be provided to any transform or resumableTransform. A RateLimiter has a maximum concurrency specified on construction. For any transforms using the same rate limiter, the channels framework will ensure that the concurrency constraint is not violated. This limits the number of simultaneous coroutine frames (and therefore allows one to eliminate these spikes).
      
      Reviewed By: aary
      
      Differential Revision: D33037310
      
      fbshipit-source-id: f7c5d30e08d155db7825d01f1b4c4b7354b14def
      eadf2192
    • Andrew Smith's avatar
      Avoid having to catpure initialization arguments when using resumableTransform · 05e0144b
      Andrew Smith authored
      Summary:
      Currently, resumableTransform takes a paremeter-less initializeTransform method for initialization, and re-initializes the transform when the transformValue function throws an OnClosedException. This requires users to capture any initialization parameters in the initializeTransform lambda, so they can re-initialize with the same parameters when needed.
      
      However, it is often the case that the initialization arguments are stored elsewhere in the app, and passed along as part of each message coming through input receiver to the transform. (This is the case in all uses of resumableTransform so far.) If we could take advantage of already having the initialization arguments available, we would not need to waste memory capturing and storing them in the initializeTransform lambda.
      
      This diff does exactly that. Instead of triggering a reinitialization by throwing an OnCloseException, reinitialization is triggered by throwing a ReinitializeTransformException<InitializationArg>. This initialization argument is then passed to initializeTransform. By doing this, we no longer need to store initialization arguments in resumable transforms.
      
      (As a side benefit, this also makes the code slightly clearer to the reader. Rather than the reader having to know that throwing an OnClosedException triggers a reinitialization, throwing a ReinitializeTransform exception makes it clear.)
      
      Reviewed By: aary
      
      Differential Revision: D33037191
      
      fbshipit-source-id: 891394d79d3fc43a8e253031472256bb852c7716
      05e0144b
    • Andrew Smith's avatar
      Add closeSubscribers to FanoutChannel · ef8b5db6
      Andrew Smith authored
      Summary: This diff adds a new function to FanoutChannel that closes all existing subscribers, without closing the FanoutChannel itself.
      
      Reviewed By: aary
      
      Differential Revision: D33035825
      
      fbshipit-source-id: 6aa867344c280ad7a91bd8248e745aa2f27e7cc8
      ef8b5db6
    • Andrew Smith's avatar
      Add Transform overload that accepts a Transformer object · 9385c05a
      Andrew Smith authored
      Summary:
      This diff reduces the amount of memory that transforms use. Currently, transform explicitly takes an executor and a transform lambda (and resumableTransform takes an additional initialization lambda). Often, these lambdas end up taking pointers to some shared state object. In the case of resumableTransform, both lambdas need to capture a pointer to the same shared state object. Furthermore, the executor passed to transform is often present on the shared state object, making it redundant. This problem will get even worse when we add rate limiters (which also can be placed on the shared state object).
      
      To reduce memory waste, this diff adds a new overload for transform and resumableTransform that takes in a Transformer object. This transformer object must implement getExecutor, transformValue, and (for resumableTransform) initializeValue. This allows a transformer object to contain just one 8-byte pointer to a shared state object, rather than duplicating pointers for different lambda captures and executors (and eventually rate limiters). It also slightly simplifies code in certain cases, as the shared state can be accessed as a field from any function in the class (rather than captured and passed down as a parameter to all helper functions).
      
      We will still leave the existing overloads for cases where it is simpler for the user to not create a new object (when the user doesn't care as much about the memory savings).
      
      Reviewed By: aary
      
      Differential Revision: D33033365
      
      fbshipit-source-id: e57c16e76ad3795a7f93b39565f553e76623beb6
      9385c05a
    • Andrew Smith's avatar
      Change Transform template parameter names · b56cffe6
      Andrew Smith authored
      Summary: This diff changes Transform's parameter names to conform to folly's style guidelines.
      
      Reviewed By: Gownta
      
      Differential Revision: D33033366
      
      fbshipit-source-id: 3713ec7995df3dd913f4da11d21fbd9377c1f00f
      b56cffe6
    • Andrew Smith's avatar
      Change MergeChannel public interface · aa378653
      Andrew Smith authored
      Summary:
      The current MergeChannel public interface has a few problems that have become more apparent with usage.
      
      1. The output receiver does not contain the key associated with the input receiver that geneated the value. This often means that the user must transform input receivers before adding them to the merge channel to include the key, which wastes memory.
      2. There is no way for a consumer of the output receiver to know if an input receiver was closed without an exception.
      3. If an input receiver was closed with an exception, the entire merge channel is closed.
      
      This diff changes the public API to fix these problems. Instead of returning an output receiver that just has values, the output receiver contains MergeChannelEvent objects. Each such event object has the key of the corresponding input receiver associated with the event, along with the event contents. There are four types of events:
      
      1. A value was received through an existing input receiver.
      2. A new input receiver was added.
      3. An existing input receiver was removed.
      4. An existing input receiver was closed (with or without an exception).
      
      Note that we could theoretically avoid genering events for 2 and 3 (as they correspond with calls that the user makes to add and remove), but there are cases where it is useful to have 2 and 3. This way, an event is always received when an existing input receiver is removed, regardless of whether it was removed explicitly (through a call to removeReceiver) or implicitly (if the input receiver was closed).
      
      Reviewed By: aary
      
      Differential Revision: D33033374
      
      fbshipit-source-id: 34f74cba3f765c76a80d448647d733ad38a2195c
      aa378653
    • Andrew Smith's avatar
      Change MergeChannel template parameter names · 017ba9c3
      Andrew Smith authored
      Summary: This diff changes MergeChannel's template parameter names to adhere to folly's style guidelines. It also reverses the order of the template parameters, as it conceptually makes more sense when the key parameter precedes the value parameter.
      
      Reviewed By: Gownta
      
      Differential Revision: D33033369
      
      fbshipit-source-id: 0b31d18baa26778ba9e8d17b9419cb180b1939d9
      017ba9c3
    • Yedidya Feldblum's avatar
      move FOLLY_DECLVAL to the top of its header · e1e39232
      Yedidya Feldblum authored
      Summary: It is pure syntax - so, dependency-free - and we may like to use in other places in the same header.
      
      Reviewed By: iahs, luciang
      
      Differential Revision: D32946539
      
      fbshipit-source-id: 2843b73af178c6d7636a4431e61017f35a48681e
      e1e39232
    • Yedidya Feldblum's avatar
      let badge and any_badge be literal types · b9e08c66
      Yedidya Feldblum authored
      Summary: By marking their constructors and conversions as `constexpr`.
      
      Reviewed By: dstechenko
      
      Differential Revision: D33176575
      
      fbshipit-source-id: bb650e768caac787ea7faf249e9c5e80b67c03a1
      b9e08c66
    • Nicholas Ormrod's avatar
      Add cr(begin,end) to sorted_vector · 5c94d653
      Nicholas Ormrod authored
      Summary: Standard containers are expected to have cr iterator variants.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D33165480
      
      fbshipit-source-id: fa6ca8899afc22fed3502ef7ed03723047c129e4
      5c94d653
  9. 16 Dec, 2021 4 commits
    • Yedidya Feldblum's avatar
      more docs for FOLLY_KEEP · b665a38d
      Yedidya Feldblum authored
      Summary: Describing the common use of keeping test/benchmark check functions.
      
      Reviewed By: Gownta, luciang
      
      Differential Revision: D33146116
      
      fbshipit-source-id: 00b2ee2a02eaa5fd23ab3fa5547dce1828b66db2
      b665a38d
    • Mingtao Yang's avatar
      OpenSSLPtrTypes: Fix build with OpenSSL 3 · bbb5e928
      Mingtao Yang authored
      Summary:
      The history of `sk_*_free` and `sk_*_pop_free`:
      
      * OpenSSL 1.0.2 - These were macros
      * OpenSSL 1.1.0 - These changed to static inline functions
      * OpenSSL 1.1.1 - These remained as static inline functions (for ABI compatibility with OpenSSL 1.1.0)
      * OpenSSL 3.0.0 - These changed to macros.
      
      Give up on trying to use the public interface for deleting `STACK_OF(T)`. This diff switches the implementation
      to just invoke the underlying `OPENSSL_sk_free` and `OPENSSL_sk_pop_free`.
      
      If a future version of OpenSSL changes the implementation such that `STACK_OF(T)` is not simply an alias of `OPENSSL_STACK`, we'll deal with it then.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D33071607
      
      fbshipit-source-id: ef37c984a120652c5224410cbafa634d4dc763d1
      bbb5e928
    • Nicholas Ormrod's avatar
      Rm unused variable · 43ba86e2
      Nicholas Ormrod authored
      Summary: This doesn't compile on platform010
      
      Reviewed By: yfeldblum
      
      Differential Revision: D33143441
      
      fbshipit-source-id: b91d08ffb06acfc1ee70399405bca978016da204
      43ba86e2
    • Shaun Ruffell's avatar
      Add subprocess_benchmark · 4e0cbf3c
      Shaun Ruffell authored
      Summary:
      Create a benchmark to measure subprocess creation time.
      
      This can be used as a baseline to measure future changes to process creation time. Specifically I am interested in optimizing the clean-up of open file descriptors in spawned process.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D33128497
      
      fbshipit-source-id: fa9b69877e6ee77a4a0e3e612e875c88d6bf4e88
      4e0cbf3c
  10. 15 Dec, 2021 5 commits
    • Yedidya Feldblum's avatar
      cut unused code in GroupVarint · 8b13ae3d
      Yedidya Feldblum authored
      Summary: It is SSE < 4 code in a block marked only for SSE >= 4. So, unused.
      
      Reviewed By: Gownta
      
      Differential Revision: D33117027
      
      fbshipit-source-id: d6957d6c73a002cc93623116f79a24f8ff599d79
      8b13ae3d
    • Vitaly Berov's avatar
      strip logging flag · a3960b9f
      Vitaly Berov authored
      Reviewed By: simpkins
      
      Differential Revision: D33062040
      
      fbshipit-source-id: a4d36cc558fc14de5929aa4ba278d394e5ca6225
      a3960b9f
    • Alex Hornby's avatar
      remove rust-shed from fb303 manifest · 424e2c6a
      Alex Hornby authored
      Summary: fb303 contains rust thrift build, but it doesn't run it from its cmake CI, so we can remove this dependency providing that its rust using consumers have the dependency
      
      Reviewed By: Imxset21, fanzeyi
      
      Differential Revision: D33091506
      
      fbshipit-source-id: fc738602104249b5de6c656d59b2eb82987bf1a0
      424e2c6a
    • Laurent Stacul's avatar
      Fix wrong SSE version (#1330) · 5269697c
      Laurent Stacul authored
      Summary:
      For some reasons, I only allow my compiler (gcc 10.0.1) to use x86 SSE, SSE2 and SSE3 instructions. When I compile, I get the following error:
      
      ```
      In file included from /opt/1A/toolchain/x86_64-v20.0.12/lib/gcc/x86_64-1a-linux-gnu/10.0.1/include/smmintrin.h:32,
                       from /opt/1A/toolchain/x86_64-v20.0.12/lib/gcc/x86_64-1a-linux-gnu/10.0.1/include/nmmintrin.h:31,
                       from /home/docker/opensource/folly/folly/GroupVarint.h:43,
                       from /home/docker/opensource/folly/folly/test/GroupVarintTest.cpp:17:
      /home/docker/opensource/folly/folly/GroupVarint.h: In function 'folly::GroupVarint<unsigned int>::decode(char const*, unsigned int*, unsigned int*, unsigned int*, unsigned int*)':
      /opt/1A/toolchain/x86_64-v20.0.12/lib/gcc/x86_64-1a-linux-gnu/10.0.1/include/tmmintrin.h:136:1: error: inlining failed in call to 'always_inline' '_mm_shuffle_epi8(long long __vector(2), long long __vector(2))': target specific option mismatch
        136 | _mm_shuffle_epi8 (__m128i __X, __m128i __Y)
            | ^~~~~~~~~~~~~~~~
      ```
      
      After some searches, it seems the requested instructions are defined in SSE4.1 as we can see in `nmmintrin.h`:
      ```
      #ifndef _NMMINTRIN_H_INCLUDED
      #define _NMMINTRIN_H_INCLUDED
      
      /* We just include SSE4.1 header file.  */
      #include <smmintrin.h>
      
      #endif /* _NMMINTRIN_H_INCLUDED */
      ```
      This PR fix the code accordingly.
      Pull Request resolved: https://github.com/facebook/folly/pull/1330
      
      Reviewed By: Orvid
      
      Differential Revision: D20323400
      
      Pulled By: yfeldblum
      
      fbshipit-source-id: 9913cc5eb378d180403094589b852d45e70978bc
      5269697c
    • Yedidya Feldblum's avatar
      no-recursion constexpr_min and constexpr_max · 88d9bf7b
      Yedidya Feldblum authored
      Reviewed By: Mizuchi
      
      Differential Revision: D33008210
      
      fbshipit-source-id: 1615b4a0a268343d79b7b83c558c0b60d65be616
      88d9bf7b
  11. 14 Dec, 2021 3 commits
    • Michael Voznesensky's avatar
      Make IOBuf construct from Stringpiece instead of std::string · c06378a2
      Michael Voznesensky authored
      Summary: Title
      
      Reviewed By: yfeldblum
      
      Differential Revision: D32573163
      
      fbshipit-source-id: c34c4de9795dcb2796045f5c464757b8f3d544d9
      c06378a2
    • Yedidya Feldblum's avatar
      let IPAddressFormatException inherit std::runtime_error · a1e3a3cf
      Yedidya Feldblum authored
      Summary: The current approach is not nothrow-copy-constructible, which violates assumptions about exception types.
      
      Differential Revision: D33009493
      
      fbshipit-source-id: 323e0cd8b8a6808dd79c6b8a0f066d46e2293289
      a1e3a3cf
    • Huseyin Tan's avatar
      Fix allocate_unique for cases where allocator with different type is provided · 891c8860
      Huseyin Tan authored
      Summary:
      While working on adding allocator support for thrift reference data types (cpp.ref) I realized that allocate_unique always expects the allocator to be of the same type. This means we cannot allocate a unique_ptr<T> with a Alloc<char>. Other allocate methods like std::allocate_shared allows this and it is quite useful. This is critical since it is almost impossible to pass a scoped_allocator of a different type for all the containers in a struct if this kind of allocator type conversion magic is not supported.
      
      This diff adds support to folly::allocate_unique to use an allocator for different type.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D33026177
      
      fbshipit-source-id: 88c43374c2482b76697485c1667064441a057117
      891c8860
  12. 13 Dec, 2021 1 commit
  13. 12 Dec, 2021 1 commit
    • Giuseppe Ottaviano's avatar
      Remove IOBufQueue::clear() · 8597900f
      Giuseppe Ottaviano authored
      Summary: All usages in fbsource have been removed.
      
      Reviewed By: yfeldblum, philippv
      
      Differential Revision: D33007671
      
      fbshipit-source-id: 74c5f2dad098c6b7cf7085aa1a2e01a48fa81781
      8597900f
  14. 10 Dec, 2021 3 commits
    • Giuseppe Ottaviano's avatar
      Remove usages of IOBufQueue::clear() · 6e2d12f8
      Giuseppe Ottaviano authored
      Summary:
      Drop all the remaining instances of `IOBufQueue::clear()`.
      
      (Note: this ignores all push blocking failures!)
      
      Reviewed By: yfeldblum
      
      Differential Revision: D32941992
      
      fbshipit-source-id: c7d983e215a78014ffda74a944b628e1886c35c8
      6e2d12f8
    • Xavier Deguillard's avatar
      cmake: include FB doctor checks · 0721ade3
      Xavier Deguillard authored
      Summary:
      These checkers do verify that a user is on the VPN, or that their certificates
      are not expired. Since we didn't bundle these, we weren't checking these
      things...
      
      Reviewed By: chadaustin
      
      Differential Revision: D33007790
      
      fbshipit-source-id: e4e7f8b625443b4f2f9bfd0fe181b7a1858cd97c
      0721ade3
    • Nick Wolchko's avatar
      split smart exception tracer read + register code · 5acc01c2
      Nick Wolchko authored
      Summary:
      Split the SmartExceptionTracer so that code can depend on the reading
      functionality without also registering the throw hooks at the same time. This
      is because hooks during throwing an exception can be expensive for some users.
      Splitting it this way allows a library to try to print an async trace if it's
      available, but still letting the end application decide if it wants to enable
      the feature or not.
      
      Reviewed By: yfeldblum, ot
      
      Differential Revision: D32766294
      
      fbshipit-source-id: 1ffd86d026fc2c18dfa547cf2d169f6491a877cc
      5acc01c2