1. 04 Jun, 2019 10 commits
    • Joe Loser's avatar
      Fix -Wstring-plus-int in FixedStringTest.cpp (#1148) · 7b7cc4e6
      Joe Loser authored
      Summary:
      - Clang 8 warns about appending integers to a string using `operator+`
        without a cast.
      
      ```
      ../folly/test/FixedStringTest.cpp:353:23: warning: adding 'unsigned int' to a string does not append to the string [-Wstring-plus-int]
        a.append("X world!" + 2u, 5u);
                 ~~~~~~~~~~~^~~~
      ../folly/test/FixedStringTest.cpp:353:23: note: use array indexing to silence this warning
        a.append("X world!" + 2u, 5u);
                            ^
                 &          [   ]
      ../folly/test/FixedStringTest.cpp:354:23: warning: adding 'unsigned int' to a string does not append to the string [-Wstring-plus-int]
        a.append("X world!" + 7u);
                 ~~~~~~~~~~~^~~~
      ../folly/test/FixedStringTest.cpp:354:23: note: use array indexing to silence this warning
        a.append("X world!" + 7u);
                            ^
                 &          [   ]
      ../folly/test/FixedStringTest.cpp:365:23: warning: adding 'unsigned int' to a string does not append to the string [-Wstring-plus-int]
        a.append("X world!" + 2u, 5u);
                 ~~~~~~~~~~~^~~~
      ../folly/test/FixedStringTest.cpp:365:23: note: use array indexing to silence this warning
        a.append("X world!" + 2u, 5u);
                            ^
                 &          [   ]
      ../folly/test/FixedStringTest.cpp:366:23: warning: adding 'unsigned int' to a string does not append to the string [-Wstring-plus-int]
        a.append("X world!" + 7u);
                 ~~~~~~~~~~~^~~~
      ../folly/test/FixedStringTest.cpp:366:23: note: use array indexing to silence this warning
        a.append("X world!" + 7u);
                            ^
                 &          [   ]
      ```
      
      - Fix this warning by creating a local char[] and using that to append
        to the fixed string
      Pull Request resolved: https://github.com/facebook/folly/pull/1148
      
      Reviewed By: ericniebler
      
      Differential Revision: D15618465
      
      Pulled By: yfeldblum
      
      fbshipit-source-id: 7f72b3597f51d99665da85744aeb8805eb2e8f00
      7b7cc4e6
    • Yedidya Feldblum's avatar
      Fix PriorityUnboundedBlockingQueue priority translation · de733c9a
      Yedidya Feldblum authored
      Summary: [Folly] Fix `PriorityUnboundedBlockingQueue` priority translation from `Executor` priorities to offsets as used by `PriorityUnboundedBlockingQueueSet`.
      
      Reviewed By: andriigrynenko
      
      Differential Revision: D15619435
      
      fbshipit-source-id: 1c5d4d68dfde7f9db07d64f40e73717b48a05ed1
      de733c9a
    • Neel Goyal's avatar
      Fix issue when reading more than 2GB · 07d59092
      Neel Goyal authored
      Summary: If performRead is called with bufLen > 2GB then overflow occurred with the value passed to SSL_read.  We'll clamp it here. This should be fine since performRead will be called again as there is more data to read.
      
      Reviewed By: andriigrynenko, knekritz, mingtaoy
      
      Differential Revision: D15625000
      
      fbshipit-source-id: 8633686a76baebd796139f3b20c6f7286339fd96
      07d59092
    • Kyle Nekritz's avatar
      Return 1 on success in OPENSSL_init_ssl compatibility implementation. · 6c11bbdc
      Kyle Nekritz authored
      Summary: Exposed by D15610738.
      
      Reviewed By: ngoyal
      
      Differential Revision: D15622920
      
      fbshipit-source-id: f3058c69098ae740d43f27e8161e99b3c5a6b995
      6c11bbdc
    • Matthieu Martin's avatar
      Simplify doSetContextData · fd879f06
      Matthieu Martin authored
      Summary:
      The main change is to grab a wlock in doSetContextData, which enables to make the code more readable, and (insignificantly) more efficient.
      
      Grabbing the wlock directly is also strictly better for both `set` and `override`.
      `setIfAbsent` is the only one to potentially suffers from the lock change, in the case where it already exists. But `setIfAbsent` isn't used by either of the guards, which are the recommended way of changing values in RequestContext. So it seems incorrect to optimize for it, and overkill to fork the code.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D15604768
      
      fbshipit-source-id: 44f564b09ff50e8bfe0e1c4cf5ee2d77b654e929
      fd879f06
    • Orvid King's avatar
      Add a basic utility for checked addition of unsigned integers · c79ac334
      Orvid King authored
      Summary: A very basic function that if done incorrectly will get optimized away as undefined behavior. For now keep things simple and only support addition of unsigned values.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D15609542
      
      fbshipit-source-id: 70d5fde784f57d3d52e6122352a885d7a835f104
      c79ac334
    • Yedidya Feldblum's avatar
      Fix SerialExecutor task destruction order · da3a4406
      Yedidya Feldblum authored
      Summary: [Folly] Fix SerialExecutor task destruction order where tasks are destroyed outside of the request-context scope.
      
      Reviewed By: andriigrynenko
      
      Differential Revision: D15618395
      
      fbshipit-source-id: e2ec8e42137d14f549f92dbc96a904eadcc0a587
      da3a4406
    • REDMOND\acoates's avatar
      Allow building x86 and arm on msvc (#1147) · 436efbfe
      REDMOND\acoates authored
      Summary:
      A couple of changes to support building folly in MSVC/win32 for x86 and arm platforms.
      Pull Request resolved: https://github.com/facebook/folly/pull/1147
      
      Reviewed By: Orvid
      
      Differential Revision: D15592309
      
      Pulled By: yfeldblum
      
      fbshipit-source-id: 97860ab9309e5492bfadac56079af735741d4a2c
      436efbfe
    • Mingtao Yang's avatar
      Fail early in folly::ssl::init() if we can't initialize · f8b36eab
      Mingtao Yang authored
      Reviewed By: yfeldblum
      
      Differential Revision: D15610738
      
      fbshipit-source-id: 367919db6465536e215735400652d33290930ca0
      f8b36eab
    • Stiopa Koltsov's avatar
      folly/string: better strerror_r detection · fa3eb5b1
      Stiopa Koltsov authored
      Summary:
      There are two variants of `strerror_r` function, one returns
      `int`, and another returns `char*`. Selecting proper version using
      preprocessor macros portably is extremely hard.
      
      For example, on Android function signature depends on `__USE_GNU` and
      `__ANDROID_API__` macros (https://git.io/fjBBE).
      
      So we are using C++ overloading trick: we pass a pointer of
      `strerror_r` to `invoke_strerror_r` function, and C++ compiler
      selects proper function.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D15484722
      
      fbshipit-source-id: fc0bd45fa67dc401631ffc185c9fad231e4a48a0
      fa3eb5b1
  2. 03 Jun, 2019 6 commits
    • Matthieu Martin's avatar
      Avoid extra lookup in overrideContextData · 03078807
      Matthieu Martin authored
      Summary: Title, unecessary cost in case of conflict
      
      Reviewed By: yfeldblum
      
      Differential Revision: D15604536
      
      fbshipit-source-id: ad40c84b822a96a688f810b24b539ac162fdcc59
      03078807
    • REDMOND\acoates's avatar
      Add some explicit casts to prevent some compiler warnings (#1145) · 762658ad
      REDMOND\acoates authored
      Summary:
      As part of trying to get our internal usage of folly off a fork, we have various compiler warnings set ridiculously high.
      
      This gets rid of a couple of warnings that we hit in folly code.
      Pull Request resolved: https://github.com/facebook/folly/pull/1145
      
      Reviewed By: LeeHowes, Orvid
      
      Differential Revision: D15586209
      
      Pulled By: yfeldblum
      
      fbshipit-source-id: dc3ebffbddd0b82f2c2ee719a95b7fcfe551e074
      762658ad
    • Woo Xie's avatar
      catch the exception of getPeerAddress/getLocalAddress, separately · fe97992a
      Woo Xie authored
      Summary: swap the order of getLocalAddress() and getPeerAddress(), so we are more likely to log the local address after exception happens.
      
      Reviewed By: knekritz
      
      Differential Revision: D15559814
      
      fbshipit-source-id: 50b7a964d6c3add2e4ae1da9fde8e9e4e67e5715
      fe97992a
    • Matthieu Martin's avatar
      Avoid mutex in RequestContext setShallowCopyContext · 40e49acc
      Matthieu Martin authored
      Summary:
      Grabbing the mutex of the newly created context is unecessary cost.
      folly::Synchronized's constructor support this optimization, so it's an easy change.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D15600165
      
      fbshipit-source-id: e92570a7ac7ac1e908d7c797bed6de22f940d387
      40e49acc
    • Matthieu Martin's avatar
      Optimize RequestData shallow copying · 0f3478f4
      Matthieu Martin authored
      Summary:
      The shallow copy code path currently iterates and copies all values manually. Instead of benefiting from the underlying data collection's implementation of self-copy.
      At the very least, it's simpler looking code.
      
      Inheriting unique_ptr is usually discouraged, but in this specific (internal impl detail) use case, I can't foresee any risk.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D15599260
      
      fbshipit-source-id: 73edea27a27e8fbc8b057cf3e974ee3d83a6e760
      0f3478f4
    • Gautham B A's avatar
      Fix typos in README.md · c719177f
      Gautham B A authored
      Summary: Pull Request resolved: https://github.com/facebook/folly/pull/1141
      
      Reviewed By: LeeHowes
      
      Differential Revision: D15560029
      
      Pulled By: yfeldblum
      
      fbshipit-source-id: 5f0d5ee8d09be834d1216d4c39244e0855d2dc0f
      c719177f
  3. 01 Jun, 2019 5 commits
  4. 31 May, 2019 2 commits
    • Roman Leventov's avatar
      Refer to 7-bit tag in comment · 48926dfb
      Roman Leventov authored
      Summary: Pull Request resolved: https://github.com/facebook/folly/pull/1142
      
      Reviewed By: shixiao
      
      Differential Revision: D15574540
      
      Pulled By: yfeldblum
      
      fbshipit-source-id: 2ce91f7510f0b278677c15b3a9f1f80b43ff8998
      48926dfb
    • Miroslav Crnic's avatar
      SerialExecutor correctly set RequestContext · 76747b04
      Miroslav Crnic authored
      Summary:
      SerialExecutor can not rely on parent to save RequestContext for him as it is not guaranteed that an enqueued function will be executed from the parent callback that was scheduled at the same time.
      This diff captures RequestContext in a lambda and sets it correctly
      
      Reviewed By: shixiao
      
      Differential Revision: D15381223
      
      fbshipit-source-id: 9b3ab1f96e57eb5e697efb52105d80c2899a03e7
      76747b04
  5. 30 May, 2019 2 commits
    • Rui Zhang's avatar
      Add eligible-for-elision accessors to folly::SharedMutex. · 9feea214
      Rui Zhang authored
      Summary: Lock elision requires non-mutating functions that check if lock acquisition could succeed. This diff adds such functions to folly::SharedMutex, including eligible_for_lock_elision(), eligible_for_lock_upgrade_elision(), and eligible_for_lock_shared_elision(). The diff also adds assertions to validate these functions' correctness under single-threaded executions.
      
      Reviewed By: nbronson
      
      Differential Revision: D15545398
      
      fbshipit-source-id: 0037d473c9dd360f7143ea4c4c9092fb9bb7f5f9
      9feea214
    • Phil Willoughby's avatar
      mark lock holders with nodiscard · 1ebd3b04
      Phil Willoughby authored
      Summary: Acquiring a lock into an unused temporary is generally a bug. Anyone wanting to do this can suppress their compiler's warning by explicitly `static_cast`ing the lock holder to void.
      
      Reviewed By: vitaut
      
      Differential Revision: D15504563
      
      fbshipit-source-id: 1033f6a1d4044b9698934b73eefebfe83602a2a5
      1ebd3b04
  6. 29 May, 2019 4 commits
    • Yedidya Feldblum's avatar
      Use a M:N caches-to-lifetimes in SingletonThreadLocal · 670c29f0
      Yedidya Feldblum authored
      Summary:
      [Folly] Use a M:N caches-to-lifetimes in `SingletonThreadLocal`.
      
      Sometimes the compiler will not merge cache and lifetime thread-local variables in expected ways, so be resilient. More details in #1135 and https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90562.
      
      At the same time, move the tracking into the wrapper. The lifetime was an intrusively-linked-list node which was clever and which was allocation-free but which increased the TLS requirement per instantiation of `SingletonThreadLocal`. Moving the tracking into the wrapper decreases the TLS requirement, although the tracking now requires heap allocations.
      
      Fixes #1135.
      Closes #1138.
      
      Reviewed By: andriigrynenko
      
      Differential Revision: D15508024
      
      fbshipit-source-id: a344e8dd52bd52724ad85b0fc88d6a2a349952ac
      670c29f0
    • Rui Zhang's avatar
      Fix cacheline padding in folly/test/SharedMutexTest with alignment. · 4856057b
      Rui Zhang authored
      Summary: The cacheline padding in folly/test/SharedMutexTest is not actually functional since the compiler is free to rearrange and optimize away local variables. This diff fixes the cacheline padding by aligning the false-sharing critical values.
      
      Reviewed By: nbronson
      
      Differential Revision: D15523241
      
      fbshipit-source-id: c51b1e4cd78ab33f75fa93faf0a0346ec602f97c
      4856057b
    • Yedidya Feldblum's avatar
      Move executor to local var immediately in doCallback · 47ef08c0
      Yedidya Feldblum authored
      Summary:
      [Folly] Move executor to local var immediately in `doCallback`, and always move it into the callback.
      
      (Note: this ignores all push blocking failures!)
      
      Reviewed By: andrewcox
      
      Differential Revision: D15532854
      
      fbshipit-source-id: 971a6a64ad3c47c9f5e9f75907a568f9d59b1000
      47ef08c0
    • Joe Loser's avatar
      Replace some NDEBUG with kIsDebug (#1128) · 0236a802
      Joe Loser authored
      Summary:
      - Replace some calls sites from `NDEBUG` to `kIsDebug`.
      - Not every call site is changed; there are a few useful cases of
        `NDEBUG` still. `xlog` macros are one of them. Another is where we
        conditionally define a variable in a local scope and then use it in an
        `assert`.
      Pull Request resolved: https://github.com/facebook/folly/pull/1128
      
      Reviewed By: aary
      
      Differential Revision: D15490848
      
      Pulled By: yfeldblum
      
      fbshipit-source-id: 4f2d1689f90baa25d3ba9e99a2a388d9cfc43f18
      0236a802
  7. 28 May, 2019 2 commits
    • Yedidya Feldblum's avatar
      Cut extra return in Core::doCallback · c3cb8f09
      Yedidya Feldblum authored
      Summary: [Folly] Cut extra `return` statement in `Core<T>::doCallback`.
      
      Reviewed By: Orvid
      
      Differential Revision: D15527963
      
      fbshipit-source-id: 9396f9ac2b246676b33d13c87e24dd42198a11ec
      c3cb8f09
    • Yedidya Feldblum's avatar
      Template fibers baton timed-wait over the deadline types · 8e6901ad
      Yedidya Feldblum authored
      Summary:
      [Folly] Template fibers baton timed-wait over the deadline types, rather than over the timeout types and rather than fixing a single timeout type.
      
      Templating it over the deadline types permits specifying the clock and permits passing a single deadline through unchanged through multiple time-delayed layers of code without skew, and templating it rather than fixing it over a single timeout type permits caller-specified fine-grained or coarse-grained timings.
      
      Currently, the `EventBase` timers do not parameterize over the deadline types, so the fiber version of timed-wait must for now convert to a fixed timeout type.
      
      Reviewed By: andriigrynenko
      
      Differential Revision: D15448177
      
      fbshipit-source-id: aa3fdbffdcb83cbfd1571780ba431a1004beb347
      8e6901ad
  8. 27 May, 2019 1 commit
    • Yuhan Hao's avatar
      update README.md on ubuntu and gcc version · 988daad8
      Yuhan Hao authored
      Summary: it seems since D15286181, we updated docker os_image to ubuntu18 and gcc7. this diff changes the README.md to reflect the change.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D15513580
      
      fbshipit-source-id: 0a3518c21c912903921946850bd08c948f09c295
      988daad8
  9. 25 May, 2019 1 commit
    • Yedidya Feldblum's avatar
      Disable TLS optimization in PIC mode for SingletonThreadLocal · 2320c70c
      Yedidya Feldblum authored
      Summary:
      [Folly] Disable TLS optimization in PIC mode for `SingletonThreadLocal`.
      
      In PIC mode, TLS offsets must be translated to addresses at runtime via calls to `__tls_get_addr`. Since there is not much of a small inline path or a fast inline fast path anymore anyway in PIC mode, might as well skip the bulky slow caching.
      
      Also, some versions of gcc do not properly support this code in shared libraries. Details in #1135 and https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90562.
      
      Fixes #1135.
      
      Reviewed By: andriigrynenko
      
      Differential Revision: D15468086
      
      fbshipit-source-id: 038e13f900fe56205c1333a620ea3bfa468c47ad
      2320c70c
  10. 24 May, 2019 6 commits
    • Greg McGary's avatar
      Avoid redefinition of mmsghdr with Android NDK unified headers · 41ed69c4
      Greg McGary authored
      Summary:
      Avoid redefinition of mmsghdr with newer Android NDK unified headers.
      
      Unified headers unconditionally define `struct mmsghdr`, but the `sendmmsg` and `recvmmsg` system calls are conditional on API level. API 21 is the first level that supports them.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D15344307
      
      fbshipit-source-id: 6f03197cc346511ce7a02f1468c35313140a8fd7
      41ed69c4
    • Brandon Schlinker's avatar
      Support TIMESTAMP_TX flag · eccdeae9
      Brandon Schlinker authored
      Summary:
      The `TIMESTAMP_TX` flag can be used to signal a request TX / NIC timestamping. This flag needs to be passed through to the application via the `SendMsgParamsCallback::getAncillaryData` callback so that the application can populate a socket control message with the ancillary data required to signal the timestamping request.
      
      `folly::AsyncSSLSocket` has extra logic for tracking the end of record (EOR) byte, but this logic currently only passes through `WriteFlags::EOR`. This diff adds support for passing through any flag specified as an EOR-related write flag when the EOR byte is written. As part of this change, the relevant unit tests are extended / cleaned up (some of them still reference MSG_EOR, which is out of date).
      
      Reviewed By: yfeldblum
      
      Differential Revision: D15465432
      
      fbshipit-source-id: 2ab5619607959dd829427a695aefd95a33b4abce
      eccdeae9
    • Brandon Schlinker's avatar
      Fix ErrMessageCallback test flakyness · 1308f879
      Brandon Schlinker authored
      Summary: Need to use `readAll` to ensure that we've read in all bytes that were written (up to EOF). Current approach sometimes returns after only a partial read.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D15456484
      
      fbshipit-source-id: f36a596a3557d4d3714bfaa3c33c726d81ccb5a4
      1308f879
    • Wez Furlong's avatar
      getdeps: configure testpilot to use collections and tag tests · 5ca2f8ad
      Wez Furlong authored
      Summary:
      This should enable test pilot to skip broken/flakey tests.
      
      The `--tag-new-tests` flag is only appropriate for code that has been
      landed on master and is used by the FB infra to classify new tests
      appropriately.
      
      For continuous builds we use a test collection with different parameters
      from the normal developer facing flow so that the infra can re-assess
      their status and enable/disable high-signal/noisy tests.
      
      Depends: D15495344
      
      Reviewed By: Ben0mega
      
      Differential Revision: D15500955
      
      fbshipit-source-id: f3b7976cec6a5cf70f5d128b38bde11620b26918
      5ca2f8ad
    • Aaryaman Sagar's avatar
      Pessimize DistributedMutex::try_lock, update comments and benchmarks · 85d3f301
      Aaryaman Sagar authored
      Summary:
      A few changes:
      
      - Pessimize DistributedMutex::try_lock() to do a load on the mutex before
        attempting a lock bts.  This allows us to be effectively parallel in the case
        where acquiring the mutex is not a hard requirement for the application.
        Whenever perf-sensitive try_lock()s come up, they tend to fit this model -
        where acquiring the mutex allows one thread to make progress, but missing the
        lock doesn't lead to incorrectness.  It mostly does not hurt performance, so
        seems like a good thing to have for all cases.
      - More unit tests that test progress in the presence of many locks
      - Also update comments to be slightly more clear, in particular add a small
        comparison with std::atomic in the first few sentences, and
      - Rerun benchmarks on both broadwell and skylake
      
      Reviewed By: djwatson
      
      Differential Revision: D15378164
      
      fbshipit-source-id: 124db4052dadae102a5c04ad9b9f3bcbefe74458
      85d3f301
    • Rosen Penev's avatar
      Fix compilation without deprecated OpenSSL APIs · 5b35fe63
      Rosen Penev authored
      Summary: Pull Request resolved: https://github.com/facebook/folly/pull/1127
      
      Reviewed By: reanimus
      
      Differential Revision: D15329439
      
      Pulled By: yfeldblum
      
      fbshipit-source-id: 6c639ef367be6adede2e6a1c487a8d89a146779d
      5b35fe63
  11. 23 May, 2019 1 commit
    • Matt Glazar's avatar
      Reuse old build dir on Windows · 78ee6b85
      Matt Glazar authored
      Summary:
      getdeps.py's find_existing_win32_subst_for_path function tries to reuse an existing build directory alias. (On Windows, the build directory is aliased to a drive letter to shorten paths.) If this function does not find and existing build directory alias, getdeps.py invalidates many of its caches.
      
      On my Windows machine, find_existing_win32_subst_for_path always fails, so all of my builds are super slow. This happens because find_existing_win32_subst_for_path is given a path with a lower-case drive letter ("c:\users\..."), but the subst command returns paths with an upper-case drive letter ("C:\users\...").
      
      When comparing paths, use ntpath.normpath. This makes the comparison case-insensitive for drive letters. (It also makes the comparison case-insensitive for other path components.)
      
      On Linux and macOS, this diff should not change behavior.
      
      Reviewed By: wez
      
      Differential Revision: D15466096
      
      fbshipit-source-id: 1669aa0a6eaeb6012154f3a9e24eba3f835262c6
      78ee6b85