Christopher Dykes [Mon, 22 May 2017 22:19:00 +0000 (15:19 -0700)]
Shift the job of defining NOMINMAX into source rather than the build system
Summary:
This makes it easier to consume Folly from a non-cmake project.
Note that this also requires making libevent, which was previously categorized as a semi-portable header into a non-portable header, meaning it can't be referenced directly. (libevent includes `Windows.h` in one of its headers -_-....)
Reviewed By: yfeldblum
Differential Revision:
D5106051
fbshipit-source-id:
5ce2d4188c9036d6ab206af3036c8fd4b09516a4
Kyle Nekritz [Mon, 22 May 2017 21:36:33 +0000 (14:36 -0700)]
Add LOCK_SHAREDMUTEX SSLLockType.
Summary: To take advantage of read locks.
Reviewed By: siyengar
Differential Revision:
D5106090
fbshipit-source-id:
a32afd698e9204196aa3d23f21a7d41803b2eb66
Yedidya Feldblum [Sat, 20 May 2017 20:32:49 +0000 (13:32 -0700)]
Refactor folly/gen/test/StringTest.cpp
Summary:
[Folly] Refactor `folly/gen/test/StringTest.cpp`.
Prefer to compare whole values once per test case, rather than sequentially comparing each part in turn.
Reviewed By: andrewjcg
Differential Revision:
D5101467
fbshipit-source-id:
3d19d32bc6a5ecc5863bea76c4292935c775fe75
Song Zhou [Sat, 20 May 2017 14:53:57 +0000 (07:53 -0700)]
Added a new variant of byLine to keep the delimiter
Summary: new method byLineFull will not trim the delimiter so that consumers can check if final line is ended up with delimiter or not.
Reviewed By: philippv, yfeldblum
Differential Revision:
D5085371
fbshipit-source-id:
5045127ee11d008e3cd7d13d33bffad280fe0a7e
Stephen Chen [Sat, 20 May 2017 03:59:15 +0000 (20:59 -0700)]
fix PicoSpinLock for aarch64 and allow PackedSyncPtr to compile for aarch64
Summary:
Looks like we never tried to compile or run the PicoSpinLock under aarch64. There were two problems:
if sizeof(IntType) is 64, we try to do
1 << 63
in lock and unlock. This doesn't compile because sizeof(1) is 32.
Also for try_lock, we were returning the data instead of checking for the locked bit.
Reviewed By: djwatson, yfeldblum
Differential Revision:
D5091656
fbshipit-source-id:
4f81b8b32633e87b491548a1a24b9bcf20264063
Christopher Dykes [Sat, 20 May 2017 03:57:10 +0000 (20:57 -0700)]
Fix the linking of various tests against GMock
Summary: A define was needed to be able to link when built against a shared-library version of gmock and gtest.
Reviewed By: yfeldblum
Differential Revision:
D5100409
fbshipit-source-id:
9a4c9df20f682a25edde642c451476b2d1e1ae22
Christopher Dykes [Sat, 20 May 2017 03:36:50 +0000 (20:36 -0700)]
Make a few adjustments to the compile config for the CMake build
Summary: This includes adding options to toggle `/std:c++latest`, and `WIN32_LEAN_AND_MEAN`, along with giving a nice drop-down selector to `MSVC_FAVORED_ARCHITECTURE` when configured via the GUI, dropping `_WINSOCK_DEPRECATED_NO_WARNINGS` as it wasn't needed, and re-enabling warnings 4365 and 4775 within the scope of the standard library.
Reviewed By: yfeldblum
Differential Revision:
D5100586
fbshipit-source-id:
87b20c6415391d4f0c75837cad5bc312910bf49f
Christopher Dykes [Sat, 20 May 2017 03:33:41 +0000 (20:33 -0700)]
Mark constexpr values needed within non-implicitly-capturing lambdas as static
Summary: MSVC needs these to be marked as `static` in order to be accessible within the lambdas without capturing them.
Reviewed By: yfeldblum
Differential Revision:
D5099607
fbshipit-source-id:
0270e2191d504f74b1902789b662e1766b2056c4
Christopher Dykes [Sat, 20 May 2017 01:37:20 +0000 (18:37 -0700)]
Work around a compiler crash in VS 2017.3 Preview 1
Summary:
MSVC is being grumpy, and I want to continue to work from the preview build, so commit the workaround, as there's no significant difference.
Upstream bug report: https://developercommunity.visualstudio.com/content/problem/59614/compiler-crash-when-constexpr-constructing-stdchro.html
Reviewed By: yfeldblum
Differential Revision:
D5099876
fbshipit-source-id:
64d5573d78b14339ff3177dd910a22787463ccd2
Christopher Dykes [Fri, 19 May 2017 23:42:25 +0000 (16:42 -0700)]
Fix the header being included when trying to specialize std::hash
Summary: `std::hash` lives in `<functional>`, not `<xstddef>`.
Reviewed By: yfeldblum
Differential Revision:
D5097468
fbshipit-source-id:
d0684491d1d4b0f83f07ad837a66a915ce2ec8a6
Shoaib Meenai [Fri, 19 May 2017 23:11:07 +0000 (16:11 -0700)]
Remove FOLLY_SKIP_LIBCPP_4000_THROW_BACKPORTS
Summary: 4.0 has been out for three months now, so it's pretty safe to remove them.
Reviewed By: Orvid, yfeldblum, mzlee
Differential Revision:
D5096007
fbshipit-source-id:
595c3e86cd7cf1a1706f27f107fb0af8ab89a676
Xiao Shi [Fri, 19 May 2017 14:23:29 +0000 (07:23 -0700)]
StampedPtr: use async-signal-safe assert
Summary: Use SafeAssert instead of assert.
Reviewed By: nbronson
Differential Revision:
D5085575
fbshipit-source-id:
daeff427b9b1fc5dff6ea986d16a70364acd2dd7
Alexey Spiridonov [Wed, 17 May 2017 22:56:39 +0000 (15:56 -0700)]
ThreadedRepeatingFunctionRunner: Name all the threads
Summary: Name all repeating function threads for ease of debugging.
Reviewed By: yfeldblum
Differential Revision:
D5065281
fbshipit-source-id:
e875e654dfa644a265e44416baf5fbf23c9da434
Phil Willoughby [Wed, 17 May 2017 11:05:52 +0000 (04:05 -0700)]
Tweak basic_fbstring construction from std types
Summary:
Previously we defined an assignment from std::string on all basic_fbstring
specialisations, but we don't actually want to do that because it's nonsense to
construct a basic_fbstring<char16_t> from an std::string. Equally it's not
nonsense to construct one of those from an std::u16string but the previous code
didn't allow it.
We now permit basic_fbstring<C, T, A1, S> to be constructed or assigned-to from
std::basic_string<C, T, A2>. The character type and traits must match but the
allocator is permitted to vary.
Background on my determination that the allocator type was unimportant and
could be disregarded: In part this is because C++17 made the same choice for
basic_string_view. Another factor was C++17's std::pmr::string (it's a
std::string with a different allocator) which I thought should be convertible
to fbstring in the same way as std::string.
Reviewed By: Gownta
Differential Revision:
D5060569
fbshipit-source-id:
f8984c528b76356240970c67916c58995d3f228d
Alexey Spiridonov [Tue, 16 May 2017 21:05:49 +0000 (14:05 -0700)]
Simplify ThreadedRepeatingFunctionRunner by requiring classes that contain it to be final
Summary: It is pretty confusing to inherit from classes that manage threads. See the docblocks in this diff.
Reviewed By: yfeldblum
Differential Revision:
D4973498
fbshipit-source-id:
2fcf1ddf68ef46d4d78a9b40f304262064862715
Stephen Chen [Tue, 16 May 2017 17:06:47 +0000 (10:06 -0700)]
gate sse42 implementation of checksum for x86 only
Summary:
folly/detail/ChecksumDetail.h gets included in Checksum.h and it
unconditionally includes <immintrin.h>. This doesn't compile on
aarch64 platform. Add #ifdefs for this.
Reviewed By: yfeldblum
Differential Revision:
D5048636
fbshipit-source-id:
d2074eb1b01487b02d95f3e2eebe683237d918fe
Yedidya Feldblum [Tue, 16 May 2017 15:07:22 +0000 (08:07 -0700)]
Fix violations of unused-lambda-capture
Summary: [Folly] Fix violations of `unused-lambda-capture`.
Reviewed By: Orvid
Differential Revision:
D5060391
fbshipit-source-id:
34aabc847719603d13da8f2b52a7ec187d803a4a
Christopher Dykes [Tue, 16 May 2017 01:23:54 +0000 (18:23 -0700)]
Switch implicit references of folly::make_unique to std::make_unique
Summary: It's *almost* dead. This switches things to explicitly reference `std::make_unique` so that `folly::make_unique` can be marked as deprecated until mobile catches up and it can be killed completely.
Reviewed By: yfeldblum
Differential Revision:
D5026584
fbshipit-source-id:
aefc8cb3de84583fd3722fdb9dfea620884590c5
Yedidya Feldblum [Mon, 15 May 2017 19:31:24 +0000 (12:31 -0700)]
Drop support for deprecated associative containers
Summary:
[Folly] Drop support for deprecated associative containers.
Namely, `__gnu_cxx::hash_set` and `__gnu_cxx::hash_map`.
Reviewed By: wqfish
Differential Revision:
D5058813
fbshipit-source-id:
ebbcf10765f5d06776ee8f697cfb78614a3a566c
Nathan Bronson [Mon, 15 May 2017 19:12:55 +0000 (12:12 -0700)]
StampedPtr
Summary:
This diff adds StampedPtr, which packs a pointer and a uint16_t into
a uint64_t with maximum portability and without adding any additional
functionality.
Reviewed By: yfeldblum, davidtgoldblatt
Differential Revision:
D4804614
fbshipit-source-id:
25fe7aff47d1e126ac8edfff4eb0bbb1d34071aa
Maxim Georgiev [Mon, 15 May 2017 19:01:31 +0000 (12:01 -0700)]
In AsyncSocket::handleErrMessages() inside the loop check if the callback was not uninstalled
Summary: AsyncSocket::handleErrMessages() should check if the error message callback is still installing before calling it, since the callback could be uninstaled on the previous loop iteration.
Reviewed By: yfeldblum
Differential Revision:
D5051001
fbshipit-source-id:
fc01932c0d36bd8f72bf1905f12211fb83d28674
Yedidya Feldblum [Mon, 15 May 2017 18:33:10 +0000 (11:33 -0700)]
List the new GCC 4.9 and C++14 minimum requirement
Summary:
[Folly] List the new GCC 4.9 and C++14 minimum requirement.
We no longer support GCC 4.8 or C++11.
Closes #590.
Reviewed By: Orvid, mzlee
Differential Revision:
D5058942
fbshipit-source-id:
0545d1bb294494e5ef5f17a40c9cf3ac688bd8d0
Arkady Shapkin [Mon, 15 May 2017 14:43:38 +0000 (07:43 -0700)]
Fix compilation ChecksumDetail.cpp with MSVC
Summary:
MSVC doesn't support `^` and `&` operator for __m128i type
Closes https://github.com/facebook/folly/pull/594
Reviewed By: yfeldblum, Orvid
Differential Revision:
D5053356
Pulled By: djwatson
fbshipit-source-id:
f789824052e0c679c265c83ad704109805c21402
Koen De Keyser [Sat, 13 May 2017 18:01:56 +0000 (11:01 -0700)]
Fix compilation on Linux systems without vDSO support
Summary:
Problems:
- The vDSO check code in configure.ac is broken, and will always fail (uses AC_LANG_PROGRAM but with arguments in the AC_LANG_SOURCE style)
- On Linux, using dlopen/dlsym requires -ldl (libdl) during the link phase. This check was missing and this argument was not added to the linker arguments.
- On a Linux system without vDSO support, libfolly.so still uses vDSO in ClockGettimeWrappers.cpp
Solution:
- Switched to AC_LANG_SOURCE
- Required libdl to exist when build target is Linux. This also adds this dependency to libfolly.la, resulting in fixing a dependency issue for Proxygen (recent Proxygen build would have issues in building examples due to failing linking step due to missing dlopen / dlsym / dlclose symbols)
- In ClockGettimeWrappers.cpp, checking if `__linux__` is set is not sufficient to determine that the system has vDSO support. The autoconf script already exposes a correct check: FOLLY_HAVE_LINUX_VDSO (this is already used in folly/detail/CacheLocality.cpp), so switched to that one.
Closes https://github.com/facebook/folly/pull/592
Reviewed By: yfeldblum
Differential Revision:
D5049816
Pulled By: Orvid
fbshipit-source-id:
58b2ed4c4101274505c61b4825accf262c0d56ef
Philip Pronin [Fri, 12 May 2017 23:07:57 +0000 (16:07 -0700)]
return range from AsyncIO::cancel(), fix test
Summary:
Return not just number of cancelled ops, but all of them as well.
Test was incorrectly assuming `wait(1)` will return exactly one operation, fix
that as well.
Reviewed By: ot
Differential Revision:
D5054684
fbshipit-source-id:
1c53c3f7ba855d1fcfeac8b1b27f90f0872d2c21
Yedidya Feldblum [Fri, 12 May 2017 21:14:22 +0000 (14:14 -0700)]
exception_wrapper::get_object<>
Summary:
[Folly] `exception_wrapper::get_object<>`.
Returns a pointer to the stored object, if the template type parameter is of the right sort.
Reviewed By: jsedgwick
Differential Revision:
D5000720
fbshipit-source-id:
2869439085e8dfb56e6cb439794b03876b7d715c
Yinghai Lu [Fri, 12 May 2017 17:21:06 +0000 (10:21 -0700)]
Fix broken promise error in folly::retrying
Summary: This diff shows an issue in `folly::retrying`. When the future generation function throws an exception and `folly::retrying` is nested in another functor that returns Future, it will throw `broken promise` instead of the actual exception message, which can be very generic and confusing. Fix is to capture the exception so that exact error message can be propagated up.
Reviewed By: yfeldblum
Differential Revision:
D5050690
fbshipit-source-id:
5b9b24977788f60aa778bb8e9cdf4281ea9a0023
James Sedgwick [Fri, 12 May 2017 09:44:24 +0000 (02:44 -0700)]
include folly/portability/Sockets.h instead of netinet/tcp.h
Summary: To satisfy internal linter
Reviewed By: yfeldblum
Differential Revision:
D4957870
fbshipit-source-id:
191ac768790fbd8cc8a4d24036c016644ff1d125
James Sedgwick [Fri, 12 May 2017 04:31:22 +0000 (21:31 -0700)]
add FOLLY_FALLTHROUGH throughout to satisfy internal linter
Summary: changes as suggested by FB-internal linter
Reviewed By: yfeldblum
Differential Revision:
D4957742
fbshipit-source-id:
c9d94c477d24f153cab0d25edccfd39ff31fdfdf
James Sedgwick [Fri, 12 May 2017 02:50:27 +0000 (19:50 -0700)]
include folly/portability/OpenSSL.h instead of openssl/*.h
Summary: suggested by internal linter
Reviewed By: Orvid
Differential Revision:
D4958232
fbshipit-source-id:
69c67d80be092db17703d672524124ca5ef7ba32
Eric Niebler [Thu, 11 May 2017 23:55:38 +0000 (16:55 -0700)]
Copying a non-const FunctionRef lvalue should be a trivial operation
Summary: Before this change, when a non-const FunctionRef lvalue is copied, it is treated as any other callable: it is wrapped with an indirection. That's semantically incorrect and potentially creates lifetime problems. Instead, use the compiler generated copy constructor, which only copies the object and function pointers.
Reviewed By: yfeldblum
Differential Revision:
D5040843
fbshipit-source-id:
f691060bdced2e287ba22d22b961c02c2b924147
Nick Terrell [Thu, 11 May 2017 21:56:02 +0000 (14:56 -0700)]
Switch uncompressedLength to an Optional<uint64_t>
Summary: Instead of using `UNKNWON_UNCOMPRESSED_LENGTH` use an `Optional`.
Reviewed By: yfeldblum
Differential Revision:
D5038919
fbshipit-source-id:
7fb60542277019996be3ff50509df5a5060cb1a0
Ashwin Bharambe [Thu, 11 May 2017 20:29:41 +0000 (13:29 -0700)]
Outline `Optional::require_value()`
Summary:
We would like to use `folly/Optional` in settings where exceptions
are disabled. An example is ObjC++ codebases where supporting exceptions
doesn't quite help (since objc_msgSend() can throw) and yet we need to pay the
additional binary size cost for the unwind tables.
This patch makes a small change to outline the "assert" utility included
inside Optional.h
Reviewed By: yfeldblum, mzlee
Differential Revision:
D5030172
fbshipit-source-id:
7def3c6eda22c41c8cab2037444978e0a4c50abf
Philip Pronin [Thu, 11 May 2017 19:51:11 +0000 (12:51 -0700)]
AsyncIO::cancel
Summary:
It should be implemented with `io_cancel`, but it is not
supported (
D682836), so still have to drain events, ignoring only
op callbacks.
Reviewed By: luciang, ot
Differential Revision:
D5044020
fbshipit-source-id:
0bcd04c91a437fccaf2189ccf771a1cb61c68942
Victor Zverovich [Thu, 11 May 2017 16:19:42 +0000 (09:19 -0700)]
Fix a race on destruction of ScopedEventBaseThread
Summary:
This diff fixes a race that happens on destruction of `ScopedEventBaseThread`.
```
Thread1: ~ScopedEventBaseThread()
Thread1: eb_.terminateLoopSoon() <- preempted just after stop_ = true
Thread2: eb->loopForever() in run(...) exits because stop_ is true
Thread2: ...
Thread2: eb->~EventBase()
Thread1: queue_->putMessage(nullptr) <- accesses destroyed EventBase
```
Reviewed By: yfeldblum
Differential Revision:
D5042654
fbshipit-source-id:
95515ed7cde09ff5f125ef121bea86ab3907f98a
Yedidya Feldblum [Thu, 11 May 2017 05:41:55 +0000 (22:41 -0700)]
hasher instances for 8-bit and 16-bit integral types
Summary:
[Folly] `hasher` instances for 8-bit and 16-bit integral types.
Allowing the use of `Hash` with such types.
They are not necessarily the ideal algorithms for those widths, essentially forwarding to the 32-bit instances.
Reviewed By: luciang
Differential Revision:
D5043094
fbshipit-source-id:
6ef96dfc8d1baf0a15b9bdd585b7c7672099e4f0
Kevin Chen [Thu, 11 May 2017 02:42:00 +0000 (19:42 -0700)]
Add writable() to AsyncTransport
Summary:
This is useful for checking if it's possible to still write to a transport,
even if its read side is closed (for transports that support half shutdown,
like AsyncSocket). Default implementation just returns true for now (up
to implementers to override).
Reviewed By: yfeldblum
Differential Revision:
D4982649
fbshipit-source-id:
0a9a2e2b745ea3db57e9f151f3a8634e1bda2465
Mathieu Baudet [Thu, 11 May 2017 02:05:01 +0000 (19:05 -0700)]
add missing const to fix `folly::toDynamic` on `std::vector<bool>`
Summary: [folly] add missing const to fix `folly::toDynamic` on `std::vector<bool>`
Reviewed By: ender-wieczorek, yfeldblum
Differential Revision:
D5039403
fbshipit-source-id:
edd052c7d1d832d424166cba15fcd9f4f8bd219c
Victor Loh [Wed, 10 May 2017 06:24:57 +0000 (23:24 -0700)]
Add pollWithRusage to Subprocess
Summary:
I was looking for a way to get rusage of a particular pid because
getrusage isn't good enough since it records the rusage of all the children
that has been terminated (and waited for). Even though wait4 is marked as
deprecated, wait3 (the cousin of wait4) is still being used in places like
time. Furthermore, there is no suitable replacement to get rusage with wait*.
Reviewed By: yfeldblum
Differential Revision:
D5008084
fbshipit-source-id:
6e511ebec7464d21309e5112aca95083e9307ea1
Anton Lunov [Wed, 10 May 2017 05:48:50 +0000 (22:48 -0700)]
Revert
D4982969: [Folly] Destroy promise/future callback functions before waking waiters
Summary: This reverts commit
71134c1657bdd4c38c12d8ca17f8335ef4c27352
Differential Revision:
D4982969
fbshipit-source-id:
fc1e766a5fd03fbd02c345904c3c6587013f38d5
Arkadiy Shapkin [Tue, 9 May 2017 22:38:27 +0000 (15:38 -0700)]
Fix FBString with MSVC
Summary: Closes https://github.com/facebook/folly/pull/588
Reviewed By: ericniebler
Differential Revision:
D5029876
Pulled By: yfeldblum
fbshipit-source-id:
6a8f16373dcfb1c7b2741eb808da0f6dbc4207b6
Eric Niebler [Tue, 9 May 2017 21:30:11 +0000 (14:30 -0700)]
Add folly::Identity function object to Utility.h; replace AtomicHashArray's AHAIdentity and folly/gen's Identity with it
Summary: Code duplication is bad.
Reviewed By: yfeldblum
Differential Revision:
D5011806
fbshipit-source-id:
cab7bb3af1c934a5a63cd3fb98aa33f2578aebfb
Arkady Shapkin [Tue, 9 May 2017 03:43:36 +0000 (20:43 -0700)]
Support static gflags library
Summary: Closes https://github.com/facebook/folly/pull/587
Reviewed By: yfeldblum
Differential Revision:
D5022071
Pulled By: Orvid
fbshipit-source-id:
2506087de76ba4544bf68a330bc2a18977c65f28
Yedidya Feldblum [Tue, 9 May 2017 01:50:51 +0000 (18:50 -0700)]
Destroy promise/future callback functions before waking waiters
Summary:
Code may pass a callback which captures an object with a destructor which mutates through a stored reference, triggering heap-use-after-free or stack-use-after-scope.
```lang=c++
void performDataRace() {
auto number = std::make_unique<int>(0);
auto guard = folly::makeGuard([&number] { *number = 1; });
folly::via(getSomeExecutor(), [guard = std::move(guard)]() mutable {}).wait();
// data race - we may wake and destruct number before guard is destructed on the
// executor thread, which is both stack-use-after-scope and heap-use-after-free!
}
```
We can avoid this condition by always destructing the provided functor before setting any result on the promise.
Reviewed By: spacedentist
Differential Revision:
D4982969
fbshipit-source-id:
71134c1657bdd4c38c12d8ca17f8335ef4c27352
Philipp Unterbrunner [Tue, 9 May 2017 01:37:28 +0000 (18:37 -0700)]
back_emplace_iterator and related classes and utility functions
Summary:
C++ up to and including C++17 lacks an alternative to std::back_inserter() that uses emplace_back() instead of push_back(). This causes unnecessary temporary objects in some cases, when using std::back_inserter() together with STL functions such as std::copy() or std::transform(). The same holds for std::front_inserter() and std::inserter().
This diff introduces folly::back_emplacer(), folly::front_emplacer(), folly::emplacer(), and related iterator classes, which call emplace_back(), emplace_front(), and emplace() respectively, with perfect forwarding of any arguments to the output iterator's operator=.
Includes support for variadic emplacement / multi-argument constructors through a utility function folly::make_emplace_args() which packs its arguments into a special tuple for use with operator=.
Reviewed By: ericniebler
Differential Revision:
D4897174
fbshipit-source-id:
c85c30c457e0c946938051819baa662d1a0b8ca1
Christopher Dykes [Tue, 9 May 2017 01:16:18 +0000 (18:16 -0700)]
Codemod folly::make_unique to std::make_unique
Summary: There are still some upstream references to `folly::make_unique` that need to be removed before it can be full killed, but this gets it most of the way there.
Reviewed By: yfeldblum
Differential Revision:
D5024310
fbshipit-source-id:
6cfe8ea93662be18bb55588c8200dec72946e205
Jim Meyering [Mon, 8 May 2017 23:41:07 +0000 (16:41 -0700)]
change EXPECT_EQ(false, ... to EXPECT_FALSE(...; ditto for true/EXPECT_TRUE
Summary:
Change every instance of EXPECT_EQ(false, ... to the simpler/shorter EXPECT_FALSE(...
Likewise, convert each EXPECT_EQ(true, ... to EXPECT_TRUE(...
Differential Revision:
D5019004
fbshipit-source-id:
0203f10fa47237f869a75a057ac4456ef03e1f53
Anirudh Ramachandran [Mon, 8 May 2017 19:21:47 +0000 (12:21 -0700)]
Move OpenSSLPtrTypes.h from folly/io/async/ssl to folly/ssl
Summary:
There's nothing io/async/ssl-specific in this file. Moving to the
top-level directory will help this be more discoverable. Part of general cleanup
for io/async/ssl
Reviewed By: yfeldblum
Differential Revision:
D5005566
fbshipit-source-id:
66a05a2139ee80a6d63791d1851da3f1858e8abf
Dave Watson [Mon, 8 May 2017 16:44:17 +0000 (09:44 -0700)]
Add hardware crc impl
Summary:
A faster crc32 impl for folly. Similar to crc32c, except
intel doesn't provide crc32 directly in hardware - instead, pclmul can be used,
which is ~2x slower than crc32c, but still ~5-10x faster than software implementation.
Reviewed By: Orvid, yfeldblum
Differential Revision:
D4994761
fbshipit-source-id:
ad8ba856649eea6dc7b541d561329ff7d7fe2d60
Andrii Grynenko [Sat, 6 May 2017 18:01:13 +0000 (11:01 -0700)]
Fix folly::call_once
Summary: std::call_once implementation is broken if function throws. This fixes folly::call_once to not depend on std::call_once.
Reviewed By: yfeldblum
Differential Revision:
D5015897
fbshipit-source-id:
bcbda68becf0930cdbf0b09125cbee61d75c2015
Yedidya Feldblum [Sat, 6 May 2017 04:15:35 +0000 (21:15 -0700)]
Mark future-core get-state members as const noexcept
Summary: [Folly] Mark future-core get-state members as `const noexcept`.
Reviewed By: andrewjcg
Differential Revision:
D5014358
fbshipit-source-id:
e39b0b63c59267a4ecfab5aac02e6d96ce2e7e00
Yedidya Feldblum [Sat, 6 May 2017 00:14:26 +0000 (17:14 -0700)]
Stop trying to setrlimit(RLIMIT_AS) in ASAN builds
Summary:
[Folly] Stop trying to `setrlimit(RLIMIT_AS)` in ASAN builds.
ASAN needs to reserve plenty of memory outside of the limited address space imposed by the call to `setrlimit`.
Reviewed By: andriigrynenko
Differential Revision:
D5014679
fbshipit-source-id:
2ab71b1cca9297d3a276cf72154fac30a2057f86
Eric Niebler [Fri, 5 May 2017 21:10:47 +0000 (14:10 -0700)]
FixedString gets comparisons with folly::Range and hence with std::string
Summary: It should be possible to perform simple comparison operations between a FixedString and a std::string. By adding asymmetric comparison operators with FixedString and Range, we make FixedString comparable with anything convertible to Range, including std::string.
Reviewed By: yfeldblum
Differential Revision:
D5007704
fbshipit-source-id:
fee89d8807ac2d5378eec0d0a51eb8684976a271
Christopher Dykes [Fri, 5 May 2017 19:03:21 +0000 (12:03 -0700)]
Fix getCurrentThreadName() on OSX
Summary: OSX was sad. Now it should be happy.
Reviewed By: mzlee
Differential Revision:
D5011751
fbshipit-source-id:
77f22ff461036d8530d8f650396d8e12503448e7
Yedidya Feldblum [Fri, 5 May 2017 18:52:53 +0000 (11:52 -0700)]
Control the number of threads in TestExecutor
Summary:
[Folly] Control the number of threads in `TestExecutor`.
Let the caller control the number of threads to use in each given case.
Reviewed By: spacedentist
Differential Revision:
D4999699
fbshipit-source-id:
4acf68cf17fbca14f0779daf0268d54c5606e4a8
Sven Over [Fri, 5 May 2017 16:12:51 +0000 (09:12 -0700)]
Future: improve test with task discarding executors
Summary:
We have tests that check that the Future implementation deals
cleanly with executors discarding tasks. The existing tests destroy
the tasks immediately when they are passed to Executor::add. This
diff adds corresponding tests for the scenario where the task is
not destroyed right away, but after the call to Future::then has
completed.
This diff also adds a mechanism to detect that the passed callback
function is actually destroyed. We have tested already that the
promise returned by folly::then will be set to a BrokenPromise
exception when the executor discards the task. However, the task
passed to the executor is not only the callback we pass to
folly::then, as the Future implementation wraps it with some code
that stores the return value in a Promise. Existing tests check
that this Promise is destroyed. The added mechanism in this diff
checks that the orignal callback function itself gets destroyed.
Reviewed By: Krigpl
Differential Revision:
D5002100
fbshipit-source-id:
4155f61b075d9fe8d1869ad229f4d350571ff4c6
Yedidya Feldblum [Fri, 5 May 2017 06:43:22 +0000 (23:43 -0700)]
Add full noexcept annotations to Indestructible
Summary:
[Folly] Add full `noexcept` annotations to `Indestructible`.
And do it without requiring `<type_traits>`.
Reviewed By: Orvid
Differential Revision:
D4999243
fbshipit-source-id:
f3521237ef4d03d2b187e9ebd6d0c90887872c42
Christopher Dykes [Fri, 5 May 2017 01:32:45 +0000 (18:32 -0700)]
Delete operator=(std::string&&) from StringPiece
Summary: This can only ever be used unsafely, so delete it.
Reviewed By: yfeldblum
Differential Revision:
D4951294
fbshipit-source-id:
bbc266d1550fceb48946c7c48e76af07292b4a53
Christopher Dykes [Thu, 4 May 2017 23:46:53 +0000 (16:46 -0700)]
Support naming a ScopedEventBaseThread
Summary:
The setThreadName API is in the process of being changed to not accept a thread id, which means the thread itself needs to set the name.
There are times where a `ScopedEventBaseThread` needs to be named, and this makes that possible.
Reviewed By: yfeldblum
Differential Revision:
D4916781
fbshipit-source-id:
dab05b520a715183ce069151ed16864fa1331abc
Christopher Dykes [Thu, 4 May 2017 23:31:01 +0000 (16:31 -0700)]
Add support for getting the current thread's name
Summary: It's primarily for use in testing, but is useful for log output as well.
Reviewed By: yfeldblum
Differential Revision:
D4943072
fbshipit-source-id:
0ca259d6c90f439e733a6179e7cba85dcd1ec9e7
Maged Michael [Thu, 4 May 2017 02:57:21 +0000 (19:57 -0700)]
Add thread-safe priority queue with arbitrary priorities using flat combining
Summary: This template uses flat combining and takes any sequential priority queue implementation that supports the `std::priority_queue` interface (`empty()`, `size()`, `push()`, `top()`, `pop()`) and any lock that meets the standard //Lockable// requirements to implement a thread-safe priority queue that supports arbitrary priorities. The template supports both unbounded and bounded size, and blocking, non-blocking, and timed variants of push, pop, and peek operations.
Reviewed By: djwatson
Differential Revision:
D4873602
fbshipit-source-id:
96e1548b4f7427ecd2ee2ead7a19993df4441b33
Eric Niebler [Thu, 4 May 2017 02:12:10 +0000 (19:12 -0700)]
test the typeid of the exception before it is potentially moved
Summary: Access the RTTI of the incomming exception before it is potentially moved from when testing for slicing
Reviewed By: yfeldblum
Differential Revision:
D4999189
fbshipit-source-id:
3e6c0a9f10a27810484330e9b37a5b0ec450ff88
Aravind Anbudurai [Wed, 3 May 2017 22:33:31 +0000 (15:33 -0700)]
Helper utility to construct, returns an Expected<..>
Summary:
folly::File's throwing constructor results in many try-catches in the callsites
or bugs where the exception is not caught.
This is a helper method to return an Expected with system_error wrapped into an
exception_wrapper.
Reviewed By: yfeldblum
Differential Revision:
D4995702
fbshipit-source-id:
be0e22b37c21c35bf157ada598916b05dfd32631
Giuseppe Ottaviano [Wed, 3 May 2017 21:58:44 +0000 (14:58 -0700)]
Store pointers in EliasFanoReader and BitVectorReader only if quantum > 0
Summary:
No need to store the pointers to forward and skip arrays if
they're not used.
Reviewed By: luciang
Differential Revision:
D4977014
fbshipit-source-id:
2ed13fdcd1561da1a294f5895f3a5e1b77f1701c
Giuseppe Ottaviano [Wed, 3 May 2017 21:58:42 +0000 (14:58 -0700)]
Template EliasFanoReader on size type
Summary: Shrink the reader type if less than 64 bits are sufficient for sizes. Do the same for `BitVectorCoding`, where we don't need an extra template parameter because the size is limited by the value domain.
Reviewed By: philippv, luciang
Differential Revision:
D4976756
fbshipit-source-id:
685047da81a556d049fb924c612f99cea3056a82
Giuseppe Ottaviano [Wed, 3 May 2017 21:58:40 +0000 (14:58 -0700)]
Do not store inner_ in EliasFanoReader and BitVectorReader
Summary: It is not used anywhere except for `previousValue`, but there it can be computed from the other fields. This reduces the `EliasFanoReader` size by 8 bytes.
Reviewed By: philippv, yfeldblum
Differential Revision:
D4976704
fbshipit-source-id:
62bc63248b66649e483b59cb9ddf48dfd2c0e992
Giuseppe Ottaviano [Wed, 3 May 2017 21:58:39 +0000 (14:58 -0700)]
Do not store the lower bits mask in EliasFanoReader
Summary:
Computing the mask on access has negligible cost as it can be
hoisted out of the linear search loop, and furthermore on Haswell we
can use the the `BZHI` instruction.
I also experimented with `BEXTR` but it ended up being slower because
computing the pattern operand requires a shift and an or (it's
probably meant for when the pattern is precomputed).
Reviewed By: philippv
Differential Revision:
D4976657
fbshipit-source-id:
e4c4ca5f0a785595587e6d6ad4676f5b216291cf
Dave Watson [Wed, 3 May 2017 17:57:29 +0000 (10:57 -0700)]
Domain destruction fixes
Summary:
If a retired object's destructor retire()s other hazard pointers, currently
these are not cleaned up correctly when the domain destructs.
Retired pointers must be cleaned up before destroying hazptr_recs, and must be done
iteratively until no more garbage is generated.
Reviewed By: magedm
Differential Revision:
D4987333
fbshipit-source-id:
bcdd61abb47caca0892a8c4dbb864d17d4f2fa30
Christopher Dykes [Wed, 3 May 2017 16:28:45 +0000 (09:28 -0700)]
Fix a couple of issues with the CMake script
Summary:
The first issue is that I was missing a `G` in `LIBGFLAGS_INCLUDE_DIR` meaning that it always claimed it couldn't find gflags even though it did, and worked just fine anyways.
The second issue is that it was passing `/permissive-` to MSVC 2015 builds, even though MSVC 2015 doesn't support the permissive flag.
Reviewed By: yfeldblum
Differential Revision:
D4988813
fbshipit-source-id:
f1ea009226baee54032409ce7add3c41f1fe7a19
Christopher Dykes [Wed, 3 May 2017 14:43:46 +0000 (07:43 -0700)]
Fix compiling with PThread support on Windows... Again...
Summary: I was bad and didn't follow through on my previous test plan, and so didn't check that it completely fixed the issue, instead only fixing one of the two problems. This fixes the other one.
Reviewed By: yfeldblum
Differential Revision:
D4988408
fbshipit-source-id:
44389bc67bcb7d6db984075f3600948460fa8c5a
Christopher Dykes [Wed, 3 May 2017 14:11:09 +0000 (07:11 -0700)]
Fix ExceptionWrapper under MSVC 2015 Update 3
Summary: Besides this code being utterly bizzare, it doesn't work under MSVC 2015 Update 3, so rather than doing a `static_cast<bool>(std::conjunction<>())` just do `std::conjunction<>::value`.
Reviewed By: yfeldblum
Differential Revision:
D4988341
fbshipit-source-id:
ededdbf3f3d945d9e2f140f4e87643ec3c0d4239
Christopher Dykes [Tue, 2 May 2017 21:21:15 +0000 (14:21 -0700)]
Fix FBString in MSVC 2015
Summary: MSVC 2015 Update 3 is even worse in this particular case, and requires it to be explicitly referenced as being within the `folly` namespace >_>...
Reviewed By: ot, yfeldblum
Differential Revision:
D4988025
fbshipit-source-id:
dea311dd42f40b442951b8df3c5f68cf70d13769
Christopher Dykes [Tue, 2 May 2017 17:31:18 +0000 (10:31 -0700)]
Fix the CMake build on Windows when using PThreads
Summary: I broke things, so now I have to fix them.
Reviewed By: yfeldblum
Differential Revision:
D4981674
fbshipit-source-id:
7df4d78cf4f984df158051a3a8b277a0bcad583b
Christopher Dykes [Tue, 2 May 2017 01:41:19 +0000 (18:41 -0700)]
Fix FBString under MSVC
Summary: MSVC requires the declaration to match a bit closer than GCC does.
Reviewed By: yfeldblum, ot
Differential Revision:
D4981404
fbshipit-source-id:
92ee40c40f66725c09a3087e49d99ebae222c2f2
Andrii Grynenko [Mon, 1 May 2017 21:51:49 +0000 (14:51 -0700)]
Fix a race in Observable context destruction
Summary: In the subscribe callback It's possible that we lock the Context shared_ptr and while update is running, all other shared_ptr's are released. This will result in Context to be destroyed from the wrong thread (thread runnning subcribe callback), which is not desired.
Reviewed By: yfeldblum
Differential Revision:
D4964605
fbshipit-source-id:
285327a6873ccb7393fa3067ba7e612c29dbc454
Anirudh Ramachandran [Mon, 1 May 2017 21:48:15 +0000 (14:48 -0700)]
More OpenSSL 1.1.0 compatibility fixes
Summary: A bunch of changes to make fbcode targets build with OpenSSL 1.1.0
Reviewed By: ivmaykov
Differential Revision:
D4949822
fbshipit-source-id:
35eda632d8335c4194352196264afeff69d87519
Ted Percival [Sun, 30 Apr 2017 08:26:27 +0000 (01:26 -0700)]
Remove unused `constexpr strlen` check
Summary:
Superseded by `<folly/portability/Constexpr.h>`'s `constexpr_strlen()`.
Closes https://github.com/facebook/folly/pull/585
Reviewed By: yfeldblum
Differential Revision:
D4973136
Pulled By: Orvid
fbshipit-source-id:
ada9bedf53ce219b0924c3f7aaad75bce9605f0f
Alexey Spiridonov [Fri, 28 Apr 2017 22:52:16 +0000 (15:52 -0700)]
Simplify BackgroundThreads, move them to folly/experimental/ThreadedRepeatingFunctionRunner
Summary:
- `FunctionScheduler` and `EventBase` are great for sharing one thread for many functions, but one-function-per-thread is messy.
- Both of those implementations are complicated, but `FunctionThreads` is dead-simple.
- I made it even simpler by eliminating the former `incrementalSleep` in favor of `std::future::wait_for`, which allows instant interruption without a tweakable param. h/t aru777 for suggesting `std::future` instead of `std::condition_variable`.
Reviewed By: yfeldblum
Differential Revision:
D4742134
fbshipit-source-id:
b520bbcd5f218b2276200ffe8926722ae8a8d6ca
Giuseppe Ottaviano [Thu, 27 Apr 2017 21:34:29 +0000 (14:34 -0700)]
Fix a comment
Reviewed By: yfeldblum
Differential Revision:
D4965608
fbshipit-source-id:
db8d67ca7174e0296fbd8da2e1fe90cbbcef9496
Philipp Unterbrunner [Thu, 27 Apr 2017 21:03:31 +0000 (14:03 -0700)]
folly::rvalue_reference_wrapper for store&forward of rvalue references
Summary:
Class template that wraps a reference to an rvalue. Similar to std::reference_wrapper but with three important differences:
1) folly::rvalue_reference_wrappers can only be moved, not copied;
2) the get() function and the conversion-to-T operator are destructive and not const, they invalidate the wrapper;
3) the constructor-from-T is explicit.
These restrictions are designed to make it harder to accidentally create a a dangling rvalue reference, or to use an rvalue reference multiple times. (Using an rvalue reference typically implies invalidation of the target object, such as move-assignment to another object.)
Reviewed By: yfeldblum
Differential Revision:
D4931483
fbshipit-source-id:
68453553bf4656ec41976699669a4491fcab79c9
Philip Pronin [Thu, 27 Apr 2017 17:06:51 +0000 (10:06 -0700)]
fix silent eax clobbering in CpuId ctor
Summary:
`cpuid` uses all of `eax`, `ebx`, `ecx`, `edx` for output, but we
aren't providing such information to GCC in a couple of cases, which might
result in incorrect code being produced;
GCC (unlike clang) doesn't allow the same register to appear both in input and
clobber list, so put `eax` into output list where required.
Reviewed By: ot, lbrandy
Differential Revision:
D4961841
fbshipit-source-id:
07ca1977373496bfd794d3b0c8a4ba21333820e4
Otto Ebeling [Thu, 27 Apr 2017 15:49:50 +0000 (08:49 -0700)]
Don't return a nullptr from IOBufQueue::split(0)
Summary: There's a gotcha case for IOBufQueue::split when n==0, it will then return an unique_ptr wrapping a nullptr, which many call sites do not expect.
Reviewed By: meyering
Differential Revision:
D4868228
fbshipit-source-id:
418256dba8ca3bcfbae420b6099baa240055b9bb
Maged Michael [Thu, 27 Apr 2017 13:59:10 +0000 (06:59 -0700)]
Flat combining: Add lock holder with deferred option. Minor fixes.
Summary: Added a lock holder with deferred option for cases where the caller may want to call try_lock() later.
Reviewed By: djwatson
Differential Revision:
D4949736
fbshipit-source-id:
31e0dc349dc3af9d04a33878e26cef1e48cce674
Yedidya Feldblum [Thu, 27 Apr 2017 08:18:43 +0000 (01:18 -0700)]
Casing consistency for exception_wrapper::throw_exception
Summary: [Folly] Casing consistency for `exception_wrapper::throw_exception`.
Reviewed By: Orvid
Differential Revision:
D4944818
fbshipit-source-id:
72056fb24ab6362e9a0319f73b5bbf8c92d658ca
Pavel Aslanov [Wed, 26 Apr 2017 21:15:50 +0000 (14:15 -0700)]
folly::overload and folly::variant_match
Summary: Pattern matching like functionality for boost::vairant. See unittests for usage examples.
Reviewed By: yfeldblum, ericniebler
Differential Revision:
D4851133
fbshipit-source-id:
cda7dc766dac5870bcc4ab1859de0e4e7f0a6599
Nick Terrell [Wed, 26 Apr 2017 17:32:25 +0000 (10:32 -0700)]
small_vector improvements
Summary:
1. `emplace_back()` is broken when there are at least two arguments and one is a reference to inside the vector. See the `ForwardingEmplaceInsideVector` test.
2. Only `push_back(value_type&&)` did exponential growth, every other function grew linearly. The bug is hidden inside of facebook because `goodMallocSize()` grows fast enough to not be horribly slow. When not using jemalloc, it will grow one element at a time.
3. `push_back(value_type const& t)` performed a copy and a move on `t` when `size() == capacity()`. Remove the extra move.
Fixes https://github.com/facebook/folly/issues/541.
Reviewed By: luciang
Differential Revision:
D4875084
fbshipit-source-id:
eefa76028c6bfd9d7c73af65e8bb9d4baf49b8cb
Dave Watson [Wed, 26 Apr 2017 17:04:04 +0000 (10:04 -0700)]
Make folly pass TSAN checks
Summary:
Currently, contbuild has a blanket TSAN suppression for folly.
Fix PicoSpinLock instead
This should fix TSAN errors as an alternative to
D4781776
Some of the tests even had TSAN errors, fixed those.
Reviewed By: davidtgoldblatt
Differential Revision:
D4795284
fbshipit-source-id:
9f0fc6868399da2f86be355ce3c081990260a649
Dave Watson [Wed, 26 Apr 2017 16:56:26 +0000 (09:56 -0700)]
Fix virtual struct bug
Summary: virtual classes currently don't work in hazard pointers, and get incorrectly reclaimed.
Reviewed By: magedm
Differential Revision:
D4951584
fbshipit-source-id:
8200df6bb8d500af2e89086edf7835d4fb90b6a2
Christopher Dykes [Wed, 26 Apr 2017 00:14:26 +0000 (17:14 -0700)]
Delete the non-char integeral forms of fbstring::operator=
Summary: They allow for assignments that make no sense, so make it impossible to do so.
Reviewed By: yfeldblum
Differential Revision:
D4919606
fbshipit-source-id:
24d8e036eff33a8c6def4672c0d098f0edd5c5b3
Neel Goyal [Tue, 25 Apr 2017 14:31:39 +0000 (07:31 -0700)]
Guard SSLContext lock checks with init mutex and add setSSLLockTypesAndInitOpenSSL
Summary:
Improve concurrency guards for `setSSLLockTypes` and `isSSLLockDisabled` by using initMutex. Also verify that openssl has been initialized w/ DCHECK in isSSLLockDisabled.
We also add a method to do the setting of locks and initialization in one shot.
Reviewed By: knekritz
Differential Revision:
D4937242
fbshipit-source-id:
308f516c17485281604d4322954c09beb58688e2
Andrii Grynenko [Tue, 25 Apr 2017 03:04:03 +0000 (20:04 -0700)]
Remove thread-safety comment from EventBaseLocal
Reviewed By: vitaut
Differential Revision:
D4940976
fbshipit-source-id:
0c4ebf8f358e6f54a7b5e81f2e73649b030006c9
Christopher Dykes [Tue, 25 Apr 2017 01:41:06 +0000 (18:41 -0700)]
Shift the implementation of setThreadName out of the header
Summary:
`folly::setThreadName` is neither performance critical, nor (after this diff) is it a template, so exposing the mess that is its implementation to the world isn't worth it.
The implementation is expected to get even more messy as I add support for getting the current thread's name, and eventually also add support for Windows as well.
This also required exposing whether the current platform supports setting the name of the current or other threads.
Reviewed By: yfeldblum
Differential Revision:
D4942401
fbshipit-source-id:
0e3be203995fa6ed667a5fd28dac7ba7fa49d683
Michael Lee [Mon, 24 Apr 2017 17:47:24 +0000 (10:47 -0700)]
Properly gate the wchar tests in FBStringTest
Summary: Using `#ifndef __ANDROID__` does not necessarily handle all cases of gating the wchar tests
Reviewed By: Orvid
Differential Revision:
D4938634
fbshipit-source-id:
a2184e20b8c382e5a34947c029f7e3746272b407
Neel Goyal [Sat, 22 Apr 2017 16:18:55 +0000 (09:18 -0700)]
Ignore setSSLLockTypes() calls after SSLContext is initialized.
Summary: We set the actual locks on initialization, so attempting to change locks after had no affect, other than making isSSLLockDisabled() report incorrect information.
Reviewed By: yfeldblum
Differential Revision:
D4935475
fbshipit-source-id:
7b80cd530801c925ade769163579b86b1a8f0027
Michael Lee [Sat, 22 Apr 2017 00:19:25 +0000 (17:19 -0700)]
Also exclude the wstring test
Summary: ^^^
Reviewed By: yangchi
Differential Revision:
D4934455
fbshipit-source-id:
babf56f4705ae06f46b694df3f2890d4b9506861
Michael Lee [Fri, 21 Apr 2017 22:08:18 +0000 (15:08 -0700)]
Turn off failing FBString test for Android
Summary: The traitsLength call in the basic_fbstring constructor is returning a bad value.
Reviewed By: Orvid
Differential Revision:
D4930106
fbshipit-source-id:
556a61c4496c2af91cb70db6d62cdc7e915edd55
Christopher Dykes [Fri, 21 Apr 2017 18:25:39 +0000 (11:25 -0700)]
Use std::nullptr_t in dynamic
Summary: It was changed to a `void*` previously due to an ICE in GCC 4.7. GCC 4.7 hasn't been supported in quite a while, and newer versions of GCC don't crash, so it's time to switch it back to `nullptr_t`.
Reviewed By: yfeldblum
Differential Revision:
D4917389
fbshipit-source-id:
fc48642026c7e3aaeadef27bb949f70648c2312c
Cameron Pickett [Fri, 21 Apr 2017 01:33:05 +0000 (18:33 -0700)]
Fix issue where compiler cannot determine address of ::free at compile time
Reviewed By: Orvid
Differential Revision:
D4921738
fbshipit-source-id:
69848cda3638fca4ead73dcc9949af0f600f33bb
Andrii Grynenko [Thu, 20 Apr 2017 21:54:37 +0000 (14:54 -0700)]
EventBaseLocal cleanup
Summary:
1. Restrict EventBaseLocal API to only be used from EventBase thread to avoid extra locking.
2. Make sure objects stored in EventBaseLocal are destroyed in EventBase thread.
Reviewed By: yfeldblum
Differential Revision:
D4918282
fbshipit-source-id:
b7cb4c2b62fef85a9b1d796fa71af8af9087479d
Nicholas Ormrod [Thu, 20 Apr 2017 17:07:21 +0000 (10:07 -0700)]
Remove unpackHack from smallVector
Summary:
This change removes the unpackHack function from small_vector, which was blocking ##-Waddress-of-packed-member## from being enabled. The fix is split the pointer-getting non-const ##getCapacity## into a normal getter and setter.
(lithium is flakey, according to continuous, and is push-blocking)
Reviewed By: yfeldblum
Differential Revision:
D4918188
fbshipit-source-id:
435e030ad659f5dc9c42d90e9bfee9ca564a120a