- 23 Dec, 2021 1 commit
-
-
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
-
- 22 Dec, 2021 1 commit
-
-
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
-
- 21 Dec, 2021 4 commits
-
-
Nicholas Ormrod authored
Summary: swap should be noexcept when possible Reviewed By: yfeldblum Differential Revision: D33198827 fbshipit-source-id: f8d52a21087b0346332359b66e1dc00f982ddbbb
-
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
-
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
-
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
-
- 20 Dec, 2021 2 commits
-
-
Nicholas Ormrod authored
Summary: Ensure that folly continues to work with platform010. Since folly is depended on my many projects, some on platform009 and some on platform010, we should make sure it continues to work with both platforms. Since many platform010 projects depend on folly, folly's platform010 coverage is actually pretty good. However, changes to test files in folly have been observed to break with 010-incompatible changes (such as D33143441 (https://github.com/facebook/folly/commit/43ba86e231ab5cb158835433df656d3026c3c6cf) and D32930335 (https://github.com/facebook/folly/commit/cec3fcac4dded35478747e4d9b8e533b491140e1)); these are not caught by dependent projects' configs. Reviewed By: yfeldblum, meyering Differential Revision: D33172025 fbshipit-source-id: 94b46471bee604289b3b6a11763a27c6e3ed9fcf
-
Yedidya Feldblum authored
Summary: sosorry Differential Revision: D33218659 fbshipit-source-id: e7c77b503ef6c7144fd8da684b22f793d85b2b96
-
- 19 Dec, 2021 1 commit
-
-
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
-
- 18 Dec, 2021 3 commits
-
-
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
-
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
-
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
-
- 17 Dec, 2021 10 commits
-
-
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
-
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
-
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
-
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
-
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
-
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
-
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
-
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
-
Yedidya Feldblum authored
Summary: By marking their constructors and conversions as `constexpr`. Reviewed By: dstechenko Differential Revision: D33176575 fbshipit-source-id: bb650e768caac787ea7faf249e9c5e80b67c03a1
-
Nicholas Ormrod authored
Summary: Standard containers are expected to have cr iterator variants. Reviewed By: yfeldblum Differential Revision: D33165480 fbshipit-source-id: fa6ca8899afc22fed3502ef7ed03723047c129e4
-
- 16 Dec, 2021 4 commits
-
-
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
-
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
-
Nicholas Ormrod authored
Summary: This doesn't compile on platform010 Reviewed By: yfeldblum Differential Revision: D33143441 fbshipit-source-id: b91d08ffb06acfc1ee70399405bca978016da204
-
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
-
- 15 Dec, 2021 5 commits
-
-
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
-
Vitaly Berov authored
Reviewed By: simpkins Differential Revision: D33062040 fbshipit-source-id: a4d36cc558fc14de5929aa4ba278d394e5ca6225
-
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
-
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
-
Yedidya Feldblum authored
Reviewed By: Mizuchi Differential Revision: D33008210 fbshipit-source-id: 1615b4a0a268343d79b7b83c558c0b60d65be616
-
- 14 Dec, 2021 3 commits
-
-
Michael Voznesensky authored
Summary: Title Reviewed By: yfeldblum Differential Revision: D32573163 fbshipit-source-id: c34c4de9795dcb2796045f5c464757b8f3d544d9
-
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
-
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
-
- 13 Dec, 2021 1 commit
-
-
Mark Santaniello authored
Summary: Rather than demanding precisely a random access iterator, allow any base of it. https://en.cppreference.com/w/cpp/iterator/iterator_tags Reviewed By: yfeldblum, Gownta Differential Revision: D33039206 fbshipit-source-id: 7b62c03704a70340a605df4d7885dce29726d8ee
-
- 12 Dec, 2021 1 commit
-
-
Giuseppe Ottaviano authored
Summary: All usages in fbsource have been removed. Reviewed By: yfeldblum, philippv Differential Revision: D33007671 fbshipit-source-id: 74c5f2dad098c6b7cf7085aa1a2e01a48fa81781
-
- 10 Dec, 2021 4 commits
-
-
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
-
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
-
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
-
Giuseppe Ottaviano authored
Summary: `IOBuf` capacity can be rounded up if supported by the allocator, and the round-up is done including the size of the shared info struct in `createCombined()`, so requesting 8 and 16 can return buffers with the same capacity, breaking the test. Reviewed By: yfeldblum Differential Revision: D33004319 fbshipit-source-id: 2bfbeaf7249e89325f9d2b11aa844c1ae1b1ceca
-