Nicholas Ormrod [Thu, 30 Oct 2014 18:26:45 +0000 (11:26 -0700)]
Override for include-guard
Summary:
In certain circumstances, it is appropriate to omit and include
guard. Add an option to allow this to be overriden.
Test Plan: Build flint, run it on folly, see no include-guard errors.
Reviewed By: andrei.alexandrescu@fb.com
Subscribers: sdwilsh, louisk, njormrod, folly-diffs@
FB internal diff:
D1644170
Tasks:
5486739
Signature: t1:
1644170:
1414616595:
7ac52f474c1312a0c28e89255b1151d56c680acf
Nicholas Ormrod [Thu, 30 Oct 2014 18:16:39 +0000 (11:16 -0700)]
Allow catch-int to be nolinted
Summary:
Add a hidden nolint option to suppress this lint warning. It is
appropriate in this case.
Test Plan:
Build flint, run it on ExceptionWrapperTest.cpp, see no catch
lint failure
Reviewed By: andrei.alexandrescu@fb.com
Subscribers: sdwilsh, louisk, njormrod, folly-diffs@
FB internal diff:
D1644165
Tasks:
5486739
Signature: t1:
1644165:
1414616664:
a6b9dc34660df84b33ed8faaf48ec048a02bad01
Nicholas Ormrod [Thu, 30 Oct 2014 17:18:27 +0000 (10:18 -0700)]
Move constructors should be noexcept
Summary:
The compiler is able to make some nice optimizations when it
knows that move constructors are noexcept. (In particular, it helps a
lot inside of std::vector if you don't need to worry about the
possibility of exception during reallocation.) Some move constructors in
folly are obviously moveable: change them.
Test Plan: unit tests
Reviewed By: delong.j@fb.com
Subscribers: trunkagent, sdwilsh, njormrod, folly-diffs@
FB internal diff:
D1644296
Tasks:
5486739
Signature: t1:
1644296:
1414618605:
d9a0db5193c82650b96e9c62b019d5da218b15c5
James Sedgwick [Wed, 29 Oct 2014 23:33:14 +0000 (16:33 -0700)]
Future<T>::then([](T&&)), aka next()
Summary:
variants of then() that bypass Try and forward any exceptions up to the next future. like next() from http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2014/n3865.pdf
this could go a long way towards reducing spurious rethrows wherever people don't ever actually catch exceptions in their continuations, which is probably often enough.
anyone know if there's something in folly or standard library that does what my silly VoidWrapper struct does?
there's a lot of copypasta here but i'm not sure consolidating into helpers would actually be useful considering the amount of template crap, i don't feel strongly about it though.
Test Plan: added unit
Reviewed By: hans@fb.com
Subscribers: trunkagent, sammerat, fugalh, njormrod, folly-diffs@, bmatheny
FB internal diff:
D1641776
Signature: t1:
1641776:
1414610434:
c742563b8061a748fca9292fc2765081edcf9d52
dcsommer [Wed, 29 Oct 2014 23:08:59 +0000 (16:08 -0700)]
Bump version to 14:0
Sean Cannella [Wed, 29 Oct 2014 21:02:55 +0000 (14:02 -0700)]
Really fix the clang warning in Format-inl.h
Summary:
The previous diff added the safety check but didn't actually
remove the compiler warning. This is what I get for doing an incremental
compile.
Test Plan: fbmake clean ; fbmake dbg with clang:dev
Reviewed By: meyering@fb.com
Subscribers: trunkagent, mathieubaudet, njormrod, folly-diffs@, bmatheny, ranjeeth, subodh, kmdent, fma, benyluo
FB internal diff:
D1646519
Signature: t1:
1646519:
1414602292:
cfb908ae094caaef02e64eb2c42544fd34fa1389
Nicholas Ormrod [Wed, 29 Oct 2014 20:16:38 +0000 (13:16 -0700)]
Fix spelling mistakes
Summary: @override-unit-failures
Test Plan: n/a
Reviewed By: rhysparry@fb.com
Subscribers: sdwilsh, njormrod, folly-diffs@
FB internal diff:
D1644139
Tasks:
5486739
Signature: t1:
1644139:
1414553949:
626fdca07a583c3d5017abe19408a5e6f6d7674a
Nicholas Ormrod [Wed, 29 Oct 2014 20:10:39 +0000 (13:10 -0700)]
Fix multiple includes
Summary:
Add nolint comments to appropriate multiply-included files.
Removed raw duplicates from StlVectorTest.
Test Plan:
fbconfig -r folly && fbmake runtests
Ran arc lint, see no "included multiple times" messages
Reviewed By: robbert@fb.com
Subscribers: sdwilsh, njormrod, folly-diffs@
FB internal diff:
D1644144
Tasks:
5486739
Signature: t1:
1644144:
1414599280:
1e403ea3675d2173b249c4ce6db0dad621a139bc
Nicholas Ormrod [Wed, 29 Oct 2014 20:03:02 +0000 (13:03 -0700)]
operator bool()
Summary:
Among all of the implicit conversions, conversion to bool is
particularly tricky. For this reason, it has a special call-out in flint
among the casts, and canno be overriden.
However, there are legitimate use cases for operator bool. In this case,
we have a iterator that logically represents a bool but in practice
represents a packed integral. This class needs to be implicitly
convertible to bool, and warrants an override.
Push blocking tests have had over a day to complete, but haven't.
Further, the folly change is comment-only.
Test Plan:
Build flint, run it on BitIteratorDetail.h, see no more lint
errors
Reviewed By: andrei.alexandrescu@fb.com
Subscribers: sdwilsh, louisk, njormrod, folly-diffs@
FB internal diff:
D1644141
Tasks:
5486739
Signature: t1:
1644141:
1414607351:
29740da758b75187f4f7b6b5a5ad28a523e7080d
Nicholas Ormrod [Wed, 29 Oct 2014 19:55:12 +0000 (12:55 -0700)]
__attribute__s need double underscore
Summary: See
D1508946 for the rationale behind this lint message.
Test Plan:
fbconfig -r folly && fbmake dbg
Reviewed By: hans@fb.com
Subscribers: sdwilsh, fugalh, njormrod, folly-diffs@
FB internal diff:
D1644190
Tasks:
5486739
Signature: t1:
1644190:
1414532704:
87d5851f363cf37c58f7e7c3d57c8210a792d4a1
Hans Fugal [Wed, 29 Oct 2014 18:30:02 +0000 (11:30 -0700)]
move clang disable of Thens.cpp to thens.rb
Summary: So when we re-generate code we don't cause clang test failures (until #
4412111 is done)
Test Plan: git diff
Reviewed By: jsedgwick@fb.com
Subscribers: net-systems@, fugalh, mathieubaudet, exa, njormrod, folly-diffs@
FB internal diff:
D1644550
Tasks:
4412111
Signature: t1:
1644550:
1414604128:
8184e7ce1f3e417c170ef67346d553caecc1f013
Dave Watson [Tue, 28 Oct 2014 17:52:49 +0000 (10:52 -0700)]
Move AsyncSocket to folly (try 2)
Summary:
Test Plan:
Reviewed By: dcsommer@fb.com
Subscribers: trunkagent, mcduff, hitesh, doug, alandau, bmatheny, njormrod, mshneer, folly-diffs@
FB internal diff:
D1644071
Signature: t1:
1644071:
1414526899:
c41dd55e2957a7e1fcc54508e20cdb4a9c8c30d4
Sean Cannella [Wed, 29 Oct 2014 02:09:35 +0000 (19:09 -0700)]
Address clang sign warning in Format-inl.h
Summary:
This has been bothering me for a while when I've been trying to
verify builds with clang. This generates a ton of spew and the current
code can't actually handle non-default negatives so let's just throw if
we see one.
Test Plan: fbmake runtests
Reviewed By: meyering@fb.com
Subscribers: mathieubaudet, njormrod, folly-diffs@, bmatheny, benyluo, shikong, kmdent, fma, ranjeeth, subodh
FB internal diff:
D1643881
Signature: t1:
1643881:
1414523777:
085035ce8c280fbd6481b2ce0354b53b96fdc50e
Hans Fugal [Tue, 28 Oct 2014 21:49:16 +0000 (14:49 -0700)]
kill threadgate
Test Plan: This diff is actually the test plan. It was contbuild bait. I fixed things that broke (in separate diffs), and then I rebased this on top, everything is green, and we pulled the trigger.
Reviewed By: davejwatson@fb.com
Subscribers: trunkagent, net-systems@, fugalh, exa, njormrod, folly-diffs@
FB internal diff:
D1632937
Tasks:
5409538
Signature: t1:
1632937:
1414531009:
4dbaa9513fbfba45a9c63e7be3de3646a6e926a2
Dave Watson [Mon, 27 Oct 2014 17:11:03 +0000 (10:11 -0700)]
Add MemoryIdler suppot to IOThreadPoolExecutor
Summary:
Idle memory in IO threads. If loop is unused for a period of time, free associated memory, and call epoll again.
Had to add a new list of callbacks that don't make the loop nonblocking (i.e. using runInLoop() instead would use the nonblocking version of epoll).
Could bake this in to EventBase directly, but that seems like the wrong abstraction, since EventBase doesn't actually control the thread - for example, that approach would also free up memory for stack-allocated EventBases where they are used synchronously by clients.
This diff doesn't change IO scheduling at all - current IO work is round robin, so this probably only helps if the whole server is idle (at least until we add smarter scheduling)
Test Plan:
Based on top of
D1585087.
fbconfig thrift/perf/cpp; fbmake dbg
_bin/thrift/perf/cpp/ThriftServer
_bin/thrift/perf/cpp/loadgen -num_threads=100 -weight_sendrecv=1 -cpp2 -async
Ran loadgen for a while, watched res memory in top. Stopped loadgen. After ~5 sec, res memory was much reduced.
Reviewed By: jsedgwick@fb.com
Subscribers: trunkagent, doug, fugalh, njormrod, folly-diffs@
FB internal diff:
D1641057
Tasks:
5002425
Viswanath Sivakumar [Tue, 28 Oct 2014 02:38:10 +0000 (19:38 -0700)]
Revert "
D1587625 [thrift->folly] AsyncSocket"
Summary:
Test Plan:
Reviewed By: cgheorghe@fb.com
Subscribers:
FB internal diff:
D1642334
Blame Revision:
D1587625
Adam Simpkins [Sun, 28 Sep 2014 01:24:18 +0000 (18:24 -0700)]
improve io::Appender functionality
Summary:
Add an operator()(StringPiece) method to Appender, so it can be used
directly with folly::Formatter objects. Also add printf() and vprintf()
methods to Appender, for places that need to use existing printf-style
formatting.
This also includes versions of push() and pushAtMost() that accept
ByteRange objects. Previously we only had push() implementations that
took separate buffer and size arguments.
Test Plan: Added new unit tests to IOBufCursorTest.cpp
Reviewed By: davejwatson@fb.com
Subscribers: trunkagent, doug, net-systems@, exa, njormrod
FB internal diff:
D1583684
Dave Watson [Mon, 27 Oct 2014 21:25:00 +0000 (14:25 -0700)]
Add missing headers
Summary: Jenkins ci build was broken for a while, so we missed these in contbuild results
Test Plan: watch ci build for fbthrift
Reviewed By: hans@fb.com
Subscribers: doug, fugalh, njormrod, folly-diffs@
FB internal diff:
D1641515
Dave Watson [Fri, 26 Sep 2014 20:49:11 +0000 (13:49 -0700)]
AsyncSocket
Summary:
Move async socket to folly.
Changes:
* Made an AsyncSocketException type instead of TTransportException: Some of the exceptions didn't fit nicely in to std::exception types (like TIMED_OUT). There are some wrappers in thrift/lib/cpp/async to convert back to TTransportException, so all existing code still compiles.
* Moved read/write callbacks out of AsyncTransport: filters are going to want to do the read/write stuff separately (see revproxy/tunnel/filters, and discussions in
D1483148).
Test Plan:
fbconfig -r thrift; fbmake runtests
contbuild should catch everything else - exception types shouldn't change for existing code
Reviewed By: dcsommer@fb.com
Subscribers: mshneer, folly-diffs@, trunkagent, doug, alandau, bmatheny, njormrod, fugalh, jsedgwick
FB internal diff:
D1587625
Hans Fugal [Mon, 27 Oct 2014 15:53:23 +0000 (08:53 -0700)]
(wangle) fix race in Core::detachOne()
Summary:
In
D1618240 I introduced a race condition in `detachOne()`, where `detached_` is incremented and then tested. If the promise and future are racing, they can both see `detached_ == 2` in the conditional, and then they'll both try to free the Core object. This fixes that.
It also fixes a related problem (which actually showed up more often with the test I wrote), where we transition into the Done state before setting the value, and then `maybeCallback()` observes the state is Done (because we're just reading an atomic, not grabbing the lock, which is intentional), tries to execute the callback, but `folly::Optional` throws an exception because the value hasn't been set yet (at least in debug it does). I should have listened to my gut and kept the state assignment after the transition action in the first place.
Test Plan: New unit test
Reviewed By: jsedgwick@fb.com
Subscribers: trunkagent, net-systems@, fugalh, exa, njormrod, folly-diffs@, mnd
FB internal diff:
D1636490
Tasks:
5438209
Blame Revision:
D1618240
Hans Fugal [Mon, 27 Oct 2014 15:53:20 +0000 (08:53 -0700)]
unrevert "(wangle) express current Core functionality with a state machine""
Summary: Reverts
D1633874. Companion to
D1636490 which fixes the bug.
Test Plan:
git reverting code that was git reverted and hasn't changed in the interim
Won't be checked in without the companion bugfix diff (
D1636490)
Reviewed By: davejwatson@fb.com
Subscribers: trunkagent, net-systems@, fugalh, exa, njormrod, folly-diffs@
FB internal diff:
D1636487
Tasks:
5438209
Blame Revision:
D1633874
Dave Watson [Thu, 23 Oct 2014 22:21:23 +0000 (15:21 -0700)]
Remove some leftover apache paths
Summary: EventBaseManager was moved to folly, we shouldn't have deps on apache
Test Plan: fbconfig -r folly/experimental/wangle/concurrency; fbmake runtests
Reviewed By: hans@fb.com
Subscribers: doug, fugalh, njormrod, folly-diffs@
FB internal diff:
D1636006
Hans Fugal [Thu, 23 Oct 2014 01:11:34 +0000 (18:11 -0700)]
Revert "(wangle) express current Core functionality with a state machine"
Summary:
Test Plan:
Reviewed By: harishs@fb.com
Subscribers: net-systems@, fugalh, exa, njormrod, folly-diffs@
FB internal diff:
D1633874
Tasks:
5438209
Hans Fugal [Wed, 22 Oct 2014 17:14:45 +0000 (10:14 -0700)]
fix Future<const T>::value()
Summary: I'm still a little mystified how one makes a Future<const T> (I tried to make one explicitly in a test and failed), but this is clearly the bug @reedriley sees in https://phabricator.fb.com/
D1620805#21
Test Plan:
fbconfig titan/cachius/test
fbmake
Reviewed By: reedriley@fb.com
Subscribers: trunkagent, net-systems@, fugalh, exa, njormrod, reedriley, folly-diffs@
FB internal diff:
D1630378
Blame Revision:
D1620805
Dave Watson [Thu, 25 Sep 2014 17:17:20 +0000 (10:17 -0700)]
move asyncserversocket to folly
Summary:
Changes:
* namespace to folly
* some std::chrono replacesments
* Moves shutdownSocketSet stuff to thriftserver instead of in the socket itself.
* Changes exception types. I don't see anywhere that uses the TAsync*SERVER*socket exceptions depending on the TTransport type, but I could be wrong? I don't really know what to do about the exception types
unittests still postponed overnight.
Test Plan:
fbconfig -r thrift; fbmake runtests
fbconfig -r proxygen; fbmake runtests
Reviewed By: dcsommer@fb.com
Subscribers: hphp-diffs@, ps, folly-diffs@, anca, trunkagent, jsedgwick, fugalh, doug, alandau, bmatheny, njormrod
FB internal diff:
D1579187
James Sedgwick [Wed, 22 Oct 2014 01:04:44 +0000 (18:04 -0700)]
add PortableSpinLock.h to Makefile, unbreak fbthrift build
Summary:
fix fbthrift build, broken in
D1621717
I wonder if it'd be possible to lint against this somehow. The jenkins failures are easy to miss especially on folly diffs with lots of failure noise (in this case it was still postponed)
Test Plan: wait for jenkins
Reviewed By: davejwatson@fb.com
Subscribers: njormrod, folly-diffs@
FB internal diff:
D1630833
Andrii Grynenko [Tue, 21 Oct 2014 20:55:56 +0000 (13:55 -0700)]
Schedule destroyInstances in registrationComplete
Summary: This makes it more likely that SingletonVaultDestructor won't be created after atexit calls.
Test Plan: unit test
Reviewed By: chip@fb.com
Subscribers: hphp-diffs@, ps, njormrod, folly-diffs@
FB internal diff:
D1629804
Tasks:
5353022
Hans Fugal [Tue, 21 Oct 2014 22:50:02 +0000 (15:50 -0700)]
(wangle) fix after-delete assert
Summary:
This would cause debug builds to do a bad thing (access the variable `this->detached_` within an assert, after `delete this`).
Test Plan: unit tests
Hopefully now that we have a dummy cpp file in `folly/wangle/detail` contbuild
will pick it up and all the dependencies will also run their tests.
Right now, we suspect maybe maelstrom (@wez) and adinvoicer and zookeeper
(@jying) and probably others are seeing this in unit test failures (esp. if
they use asan, which is rightly detecting read after free). Hoping contbuild
will catch them.
Reviewed By: davejwatson@fb.com
Subscribers: net-systems@, fugalh, exa, njormrod, folly-diffs@, wez, dcapel, jying, cgheorghe
FB internal diff:
D1630301
Tasks:
5424546,
5435720
Blame Revision:
D1618240
Hans Fugal [Tue, 21 Oct 2014 18:55:52 +0000 (11:55 -0700)]
waitFor race workaround
Summary: Fixing the race will take some more thought. Probably a condvar. For now, I'm turning waitFor into a spinlock instead.
Test Plan: new unit test that deadlocks before the workaround
Reviewed By: jsedgwick@fb.com
Subscribers: trunkagent, net-systems@, fugalh, exa, njormrod, folly-diffs@
FB internal diff:
D1628015
Tasks:
5427828
Hans Fugal [Tue, 21 Oct 2014 17:24:14 +0000 (10:24 -0700)]
(wangle) Fix a couple compilation issues
Summary: If you try to include files in a weird (but not incorrect) order you get compilation errors. No longer!
Test Plan: new dummy `cpp_binary` target
Reviewed By: davejwatson@fb.com
Subscribers: trunkagent, folly-diffs@, net-systems@, fugalh, exa, njormrod
FB internal diff:
D1621083
Hans Fugal [Tue, 21 Oct 2014 17:24:10 +0000 (10:24 -0700)]
(wangle) Interrupts (and therefore, cancellation)
Summary:
Modeled very closely after Finagle's interrupts. Compare with https://github.com/twitter/util/blob/master/util-core/src/main/scala/com/twitter/util/Promise.scala if you like.
The basic idea is the promise holder can register an interrupt handler, and then interrupts will call that handler. A typical handler would fulfil the promise with an exception (or special value) indicating that it was interrupted (if it was interrupted in time).
Raising an interrupt does not prevent setting a value or callbacks executing or any of that - it is only advisory to the promise holder.
Test Plan: I wrote some unit tests.
Reviewed By: davejwatson@fb.com
Subscribers: folly-diffs@, net-systems@, fugalh, exa, hannesr, njormrod
FB internal diff:
D1620805
Tasks:
4618297
Hans Fugal [Tue, 21 Oct 2014 17:24:06 +0000 (10:24 -0700)]
(wangle) express current Core functionality with a state machine
Summary:
This is a refactor of the current functionality to use a state machine (inheriting from `FSM`). This will make it easier to extend to do cancellation and Future collapsing. Performance is the same, maybe slightly faster (abt 1%). (I might be a little conservative with the atomics, it might be worth going through and reasoning about whether they all need to be atomic).
The state machine is two states, Waiting (no value), and Done (has a value). Transitioning to Done will execute the callback if it exists and we are active. Otherwise the callback will happen when it is set and active is true.
There is a subjective balancing act in place here, between making a state for every single mutable bit of information (which results in an explosion of states: hasValue X hasCallback X isActive X isCancelled …), and finding a sweet spot of expressivity. This isn't too far from the way Twitter did it, though we don't (yet) have all the states they have (and they don't have the concept of hot/cold futures). But I got there by way of replacing the `mutex_` with the state, after changing all those variables to atomics so they didn't need mutex protection (resulting in only `callback_` and `result_` needing it). I expect the state machine will morph as the rest of the functionality is added, but hopefully it will be easier to understand and keep correct (that's the idea anyway).
Test Plan: Tests still pass (and not by accident, I made several awesome mistakes along the way).
Reviewed By: davejwatson@fb.com
Subscribers: net-systems@, fugalh, exa, njormrod
FB internal diff:
D1618240
Tasks:
4618297
Hans Fugal [Tue, 21 Oct 2014 17:24:03 +0000 (10:24 -0700)]
(wangle) Core cleanup
Summary:
Just some cleaning hose in `detail::Core`.
- rename `fulfil` to `setResult`
- and `value_` to `result_`
- remove unnecessary helper methods (I can be convinced to keep them if you like them)
Test Plan: mechanical changes, tests still pass
Reviewed By: davejwatson@fb.com
Subscribers: trunkagent, folly-diffs@, net-systems@, fugalh, exa, njormrod
FB internal diff:
D1618161
Hans Fugal [Mon, 20 Oct 2014 19:11:19 +0000 (12:11 -0700)]
(wangle) deprecate Later and ThreadGate
Summary:
Later is now superfluous. Just hold a deactivated future (e.g. one returned by `Future::via`), and then activate (or destruct) it when you're ready.
ThreadGate has proven to be too rigid and too heavyweight to be really useful. And is superfluous now with `Future::via`. e.g.
f.via(east).then(a1).then(a2).via(west).then(b1).then(b2);
This is stage one of removing these: mark them deprecated.
Stage two will be for me to change all existing usages in our code to not use them.
Stage three will be to rip these out altogether.
Test Plan:
Everything still builds and works, but now with deprecation warnings.
fbconfig -r \
common/smc/cpp/tests \
folly/wangle \
langtech/audiens \
messaging/avscan \
messaging/maelstrom \
neteng/traffic_manager \
notifications/nudge/service \
tao/client \
zeus/datashuttle
fbmake runtests
Reviewed By: jsedgwick@fb.com
Subscribers: net-systems@, fugalh, exa, njormrod, folly-diffs@
FB internal diff:
D1626412
Tasks:
5409538
Hans Fugal [Mon, 20 Oct 2014 19:10:53 +0000 (12:10 -0700)]
remove fireAndForget
Summary: It was deprecated, now is unused.
Test Plan:
fbgs
check contbuild
Reviewed By: hannesr@fb.com
Subscribers: trunkagent, net-systems@, fugalh, exa, njormrod, folly-diffs@
FB internal diff:
D1624221
Hans Fugal [Mon, 20 Oct 2014 19:10:34 +0000 (12:10 -0700)]
(wangle) dummy cpp files
Summary: for great contbuild
Test Plan: these projects still build. contbuild should start picking them up, based on discussion with @robbert
Reviewed By: robbert@fb.com
Subscribers: trunkagent, net-systems@, fugalh, exa, mwa, jgehring, fuegen, njormrod, folly-diffs@, robbert
FB internal diff:
D1624206
Tasks:
5399744
Hans Fugal [Mon, 20 Oct 2014 19:07:36 +0000 (12:07 -0700)]
(FSM) updateState with unprotected action
Summary:
Like the magic macros. As it says in the comment, this can lead to nicer code.
Also added/moved a couple examples to the top of `test/FSM.cpp`.
Test Plan: new tests
Reviewed By: davejwatson@fb.com
Subscribers: trunkagent, net-systems@, fugalh, exa, njormrod
FB internal diff:
D1618184
James Sedgwick [Mon, 20 Oct 2014 18:21:35 +0000 (11:21 -0700)]
sync wangle to makefile
Summary: as above
Test Plan:
Reviewed By: davejwatson@fb.com
Subscribers: fugalh, njormrod
FB internal diff:
D1589439
Sean Cannella [Fri, 17 Oct 2014 20:38:30 +0000 (13:38 -0700)]
Support NotificationQueue on iOS/Android
Summary:
This is the last of the changes to make folly and thrift portable. The old
thrift SpinLock implementation is exactly we
want here so port this into folly. If someone later makes efficeint
versions of MicroSpinLock for platforms other than x64 they can easily
be plugged in without changing the calling code.
Test Plan: compiled on multiple platforms
Reviewed By: alandau@fb.com
Subscribers: folly-diffs@, ldbrandy, trunkagent, alandau, bmatheny, njormrod, mshneer, shikong, benyluo, kmdent, fma, ranjeeth, subodh
FB internal diff:
D1621717
Tasks:
5183325
dcsommer [Fri, 17 Oct 2014 18:46:07 +0000 (11:46 -0700)]
Bump version to 13:0
Pavlo Kushnir [Fri, 17 Oct 2014 18:04:46 +0000 (11:04 -0700)]
Fix mcrouter opensource build
Summary: mcrouter now uses folly::Singleton.
Test Plan: visual
Reviewed By: andrii@fb.com
Subscribers: njormrod
FB internal diff:
D1622647
Max Wang [Thu, 16 Oct 2014 21:50:48 +0000 (14:50 -0700)]
Fix typo in folly::Optional
Test Plan: yes
Reviewed By: delong.j@fb.com
Sean Cannella [Thu, 16 Oct 2014 19:16:41 +0000 (12:16 -0700)]
Fix one missing conditional for Android
Summary:
pthread_atfork isn't defined in the Android NDK (and therefore is not actually supported even though it's in bionic/libc) so don't call it there.
Test Plan: compiled on Linux, Android
Reviewed By: meyering@fb.com
Subscribers: njormrod, shikong, kmdent, fma
FB internal diff:
D1620584
Tasks:
5183325
Tom Jackson [Thu, 16 Oct 2014 18:19:00 +0000 (11:19 -0700)]
Fixing broken bits test
Summary: Buffer size calculation had fallen out of sync with the actual test code. Good thing we have ASAN!
Test Plan: Run the test with address santizer.
Reviewed By: mpawlowski@fb.com
Subscribers: mpawlowski, trunkagent, njormrod
FB internal diff:
D1619472
Tasks:
5325399
Max Wang [Wed, 15 Oct 2014 20:00:18 +0000 (13:00 -0700)]
Allow Optional<T> {=,!}= T comparisons in folly::Optional
Summary:
I can see how {<,>}{,=} comparisons might be ambiguous, but equality
should never be, so let's allow it.
Test Plan: tests
Reviewed By: tjackson@fb.com
Subscribers: trunkagent, njormrod
FB internal diff:
D1618088
Dave Watson [Fri, 3 Oct 2014 19:05:37 +0000 (12:05 -0700)]
move shutdown socket set
Summary:
Move shutdownsocketset to folly, since it is a dep of the asyncsockets
Previoulsy tried moving it in to the server directly:
D1583629, but had issues - close(fd) is called before the error callback, so we can't remove the fd before the close, which is essential to it working properly.
Just move it to folly instead.
Test Plan: fbconfig -r thrift/lib/cpp thrift/lib/cpp2; fbmake runtests
Reviewed By: dcsommer@fb.com
Subscribers: mshneer, trunkagent, fugalh, jsedgwick, doug, alandau, bmatheny, njormrod
FB internal diff:
D1594950
Sean Cannella [Thu, 16 Oct 2014 17:09:06 +0000 (10:09 -0700)]
Conditionals for iOS / Android compilation
Summary:
Conditional changes required to compile on iOS and Android.
Test Plan: compiled on all platforms
Reviewed By: meyering@fb.com
Subscribers: kmccray, trunkagent, shilin, njormrod, ranjeeth, subodh, bmatheny
FB internal diff:
D1618028
Tasks:
5183325
Andrii Grynenko [Thu, 16 Oct 2014 00:40:15 +0000 (17:40 -0700)]
Don't throw in Singleton::get() if singleton is alive
Summary:
This is a fix for situations where you know that something is keeping singleton alive (by getting a weak_ptr and locking it), and request a singleton instance via get() method (if e.g. it's hard to pass a pointer to singleton instance from a method which locked it). If shutdown was scheduled - an exception was previously thrown even though the object was not destroyed yet.
This simplifies conversion of legacy code to folly::Singleton.
Test Plan:
fbconfig -r gatehouse/usersets/tests
fbmake runtests
Reviewed By: chip@fb.com
Subscribers: trunkagent, njormrod
FB internal diff:
D1619311
Subodh Iyengar [Wed, 15 Oct 2014 23:51:46 +0000 (16:51 -0700)]
Remove global init of ThreadLocal in Random
Summary:
Remove global initialization of ThreadLocal in Random
See https://gcc.gnu.org/onlinedocs/gcc-4.8.3/gcc/Thread-Local.html
Static initializers and thread locals don't mix well, and we're seeing
some crashes which we think might be related to this.
Test Plan: Unit tests
Reviewed By: seanc@fb.com
Subscribers: trunkagent, seanc, njormrod
FB internal diff:
D1617455
James Sedgwick [Wed, 15 Oct 2014 22:58:44 +0000 (15:58 -0700)]
support for move-only types
Summary: universal references and perfect forwarding to the rescue
Test Plan: added demonstrative unit tests
Reviewed By: hans@fb.com
Subscribers: trunkagent, fugalh, njormrod, bmatheny
FB internal diff:
D1592032
Tasks:
5283342
James Sedgwick [Wed, 15 Oct 2014 22:57:47 +0000 (15:57 -0700)]
merge wangle/Executor.h and experimental/wangle/concurrent/Executor.h
Summary:
the one in concurrent/ is a bit more generic, so I kept that as Executor and renamed the existing one ScheduledExecutor
because Hans is surfing I took the liberty of renaming Action->Func as an alias for std::function<void()>, because I think it's more reflective
also kept the version of add() that doesn't force rvalue-reference as it's more user friendly and probably not less performant in common cases (insert reference to "want speed? pass by value" here)
Test Plan: compiled some major relevant bits, will let contbuild show me anything I missed
Reviewed By: hans@fb.com
Subscribers: trunkagent, rushix, fbcode-common-diffs@, fugalh, msk, njormrod
FB internal diff:
D1591237
Tasks:
5279196
Andrii Grynenko [Tue, 14 Oct 2014 03:08:15 +0000 (20:08 -0700)]
Schedule destroyInstances only when first singleton is created
Summary:
Right now destroyInstances is scheduled when SingletonVault is requested. This may change singleton destruction order (folly::Singleton-managed vs unmanaged singletons) when new singleton is registered with folly::Singleton (no matter if it is even used).
This diff changes it to be more stable.
Test Plan: servicerouter unittests
Reviewed By: chip@fb.com
Subscribers: njormrod, mshneer
FB internal diff:
D1615213
Tasks:
5353022
Sean Cannella [Wed, 15 Oct 2014 21:30:46 +0000 (14:30 -0700)]
Fix clang test compilation failures
Summary:
While working on other code I noticed folly stopped compiling
with clang. Fix that.
Test Plan: fbmake runtests (clang and gcc)
Reviewed By: lucian@fb.com
Subscribers: mathieubaudet, njormrod, bmatheny
FB internal diff:
D1618189
Hans Fugal [Wed, 15 Oct 2014 21:05:59 +0000 (14:05 -0700)]
(wangle) Use MicroSpinLock
Summary: For great speed
Test Plan:
Before
============================================================================
folly/wangle/test/Benchmark.cpp relative time/iter iters/s
============================================================================
constantFuture 241.19ns 4.15M
promiseAndFuture 100.32% 240.42ns 4.16M
withThen 44.63% 540.47ns 1.85M
----------------------------------------------------------------------------
oneThen 519.20ns 1.93M
twoThens 62.83% 826.41ns 1.21M
fourThens 36.80% 1.41us 708.75K
hundredThens 1.79% 29.05us 34.42K
----------------------------------------------------------------------------
no_contention 4.82ms 207.27
contention 62.91% 7.67ms 130.39
============================================================================
After
============================================================================
folly/wangle/test/Benchmark.cpp relative time/iter iters/s
============================================================================
constantFuture 159.79ns 6.26M
promiseAndFuture 101.23% 157.84ns 6.34M
withThen 41.78% 382.47ns 2.61M
----------------------------------------------------------------------------
oneThen 358.23ns 2.79M
twoThens 63.07% 568.00ns 1.76M
fourThens 36.89% 971.07ns 1.03M
hundredThens 1.76% 20.34us 49.17K
----------------------------------------------------------------------------
no_contention 3.75ms 266.75
contention 59.83% 6.27ms 159.59
============================================================================
That's a 150% speedup.
Reviewed By: davejwatson@fb.com
Subscribers: net-systems@, fugalh, exa, njormrod
FB internal diff:
D1617363
Tasks:
5278220
Jun LI [Wed, 15 Oct 2014 20:53:51 +0000 (13:53 -0700)]
Defer query string parsing from ctor to getQueryParams()
Summary:
Query string parsing uses a lot of CPU, it happens in ctor killing CPU
even in users that are not interested in query string. Move it to
getQueryParams().
Test Plan:
fbconfig folly/test
fbmake runtests_dbg
Reviewed By: ldemailly@fb.com
Subscribers: trunkagent, njormrod, zellux
FB internal diff:
D1604973
Tasks:
5304484
Blame Revision: https://phabricator.fb.com/
D1455158
Pavlo Kushnir [Wed, 15 Oct 2014 18:46:22 +0000 (11:46 -0700)]
Fix opensource build
Summary:
D1615707 moved State.h to Core.h, but didn't update Makefile.am
Test Plan: visual
Reviewed By: davejwatson@fb.com
Subscribers: fugalh, njormrod
FB internal diff:
D1617562
Blame Revision:
D1615707
Daniel Sommermann [Tue, 14 Oct 2014 23:39:00 +0000 (16:39 -0700)]
Fix thread local random number generator static issue
Summary:
See https://gcc.gnu.org/onlinedocs/gcc-4.8.3/gcc/Thread-Local.html
Thanks to @subodh for finding the first instance of this problem in the
proxygen codebase.
Test Plan:
unit tests, tested proxygen unit tests on my laptop and I don't
get segfaults on program exit now.
Reviewed By: subodh@fb.com
Subscribers: trunkagent, doug, njormrod, subodh
FB internal diff:
D1616149
Tasks:
5217022
Hans Fugal [Wed, 15 Oct 2014 16:32:03 +0000 (09:32 -0700)]
wangle::detail::FSM
Summary:
spaghetti-monster
Finagle uses a nifty state machine pattern in `Promise.scala`. Each state carries its data with it, then you switch statements with a simple state `cas`, within the functions (which are the transition inputs). See https://github.com/twitter/util/blob/master/util-core/src/main/scala/com/twitter/util/Promise.scala#L133-L139 and https://github.com/twitter/util/blob/master/util-core/src/main/scala/com/twitter/util/Promise.scala#L604-L635 for example.
I was thinking we could do something similar in C++, though we don't quite have the same luxury of type cases like scala. (Although as an aside I found this really cool paper on implementing type cases in C++, it's kind of evil but very cool. http://www.stroustrup.com/OOPSLA-typeswitch-draft.pdf)
I was looking at having a union of the different state classes, e.g.
union U {
State base;
Done done;
Waiting waiting;
...
};
and then you could use the memoized `dynamic_cast` trick in that paper to make a fairly efficient type case (and fairly readable, depending on how evil you want to get with the macros). However, `dynamic_cast<Done*>(&u.base)` blissfully segfaults. I'm not sure if this is something that should work and it's a compiler bug, or whether trying to (ab)use a union this way is against some arcane C++ rules. @hannesr suggested maybe a variant type might work. We can also do memory mangling on our own if it comes to it - however those are all more advanced techniques to play with at a later optimization time.
So instead, I went for a this-is-context helper base class. The mutable context is just `this`, and you inherit from `FSM`.
The magic macros make it more succint to lay out state transitions. See the tests for examples. Maybe in the future we can get really clever and find a way to generate state machine diagrams from code using this, especially when macros are being used.
Test Plan: unit tests were written and pass
Reviewed By: davejwatson@fb.com
Subscribers: meisner, trunkagent, net-systems@, fugalh, exa, njormrod, hannesr
FB internal diff:
D1613497
Hans Fugal [Wed, 15 Oct 2014 16:31:52 +0000 (09:31 -0700)]
(wangle) s/State/Core/
Summary:
codemod
`State` is such an overloaded term, and not really the best to describe this backing future/promise object. Yes, it holds the state but it's more than that and it gets in the way of calling the states of the state machines `State`s.
Test Plan: builds and tests pass
Reviewed By: davejwatson@fb.com
Subscribers: trunkagent, net-systems@, fugalh, exa, njormrod
FB internal diff:
D1615707
Andrii Grynenko [Wed, 15 Oct 2014 01:00:43 +0000 (18:00 -0700)]
Bump version to 12:0
Andrii Grynenko [Wed, 8 Oct 2014 01:12:33 +0000 (18:12 -0700)]
Don't re-create singletons
Summary:
This forbids singleton creation/re-creation after destroyInstances() was called.
Only after reset() is called singletons can be created again.
registrationComplete() behavior is also slightly change. We disallow singleton registration after registrationComplete() is called even in Relaxed mode. Strict mode now only controlls whether singletons can be constructed before registation is complete.
Test Plan: unit test
Reviewed By: chip@fb.com
Subscribers: hphp-diffs@, ps, trunkagent, njormrod, mshneer, lins
FB internal diff:
D1605136
Yedidya Feldblum [Fri, 10 Oct 2014 19:44:26 +0000 (12:44 -0700)]
Basic tests for EventHandler.
Summary:
[Folly] Basic tests for EventHandler.
Worthwhile adding some tests just for this class since it's rather fundamental. It is already tested indirectly via any number of things that make use of EventBase under the hood, but it's worth having a place for more direct tests.
Test Plan: Unit tests in `folly/io/async/test/EventHandlerTest.cpp`.
Reviewed By: njormrod@fb.com
Subscribers: njormrod, chalfant, dougw
FB internal diff:
D1606284
Soren Lassen [Fri, 10 Oct 2014 14:12:31 +0000 (07:12 -0700)]
Better estimateSpaceNeeded(double)
Test Plan: fbconfig -r folly/test && fbmake runtests
Reviewed By: mpawlowski@fb.com
Subscribers: njormrod
FB internal diff:
D1604761
Blame Revision:
D1420588
Hans Fugal [Thu, 9 Oct 2014 23:27:12 +0000 (16:27 -0700)]
Go back to a regular mutex
Summary:
D1428504 changed the `detail::State` mutex to be a recursive mutex to get around deadlock in our project, where a promise was being freed within a callback, racing with the mechanics happening while fulfilling the promise in the first place. At least, that's what seemed to be happening. I couldn't easily pull it into gdb because it's a python test.
I made my own test to repro, and it did in fact deadlock:
TEST(Future, DISABLED_promiseDestructedDuringCallback) {
auto p = folly::make_unique<Promise<void>>();
p->getFuture().then([&](Try<void>&&) {
// shouldn't deadlock.
p.reset();
});
p->setValue();
}
However, although this code fixes our project it does not fix this code, which still fails (not with a deadlock, but with a promiseAlreadyFulfilled exception). So whatever our project is doing is a bit more intricate.
I'm not convinced that even allowing this is ok - so I suspect out project is doing something bad. However, I also think it's probably bad to hold the lock while doing the callback so I am presenting this as a fix/compromise.
Test Plan:
unit tests
Reviewed By: hannesr@fb.com
Subscribers: net-systems@, fugalh, exa, njormrod, fsilva, davejwatson
FB internal diff:
D1604829
Blame Revision:
D1428504
Sean Cannella [Thu, 9 Oct 2014 19:40:37 +0000 (12:40 -0700)]
Port compilation fixes (1/3)
Test Plan: existing tests
Reviewed By: meyering@fb.com
Subscribers: trunkagent, bmatheny, ranjeeth, njormrod, subodh
FB internal diff:
D1605942
Tasks:
5183325
Francis Ma [Thu, 9 Oct 2014 15:35:06 +0000 (08:35 -0700)]
Move static global variable randomDevice inside the scope of the function
Test Plan: Tests under folly passed
Reviewed By: seanc@fb.com
Subscribers: trunkagent, seanc, njormrod
FB internal diff:
D1600266
Tasks:
5317470
Nathan Bronson [Thu, 9 Oct 2014 02:44:16 +0000 (19:44 -0700)]
add per node mutex for emulated futex
Summary:
The emulated futex (not used on Linux) has an optimization to defer
notification until after the bucket mutex has been unlocked, to avoid
lock collisions between the waiter and waker. That code will have a
use-after-free problem if the waiter's condition_variable has a spurious
wakeup, which is allowed by the spec. That code also doesn't do
anything about contention between multiple waiters.
This diff adds a mutex to each wait node, relieving the waiters from
having to acquire the bucket lock on wakeup. Rather than trying to
perform a racy late notification, we just make sure to release the node
lock immediately after calling notify_one, which seems to have the
desired effect.
Test Plan:
1) existing unit tests
2) new unit tests
Reviewed By: delong.j@fb.com
Subscribers: boseant, njormrod
FB internal diff:
D1602360
James Sedgwick [Thu, 9 Oct 2014 00:15:17 +0000 (17:15 -0700)]
move TEventBaseManager to folly/io/async/EventBaseManager
Summary:
This class isn't thrift specific anymore, especially now that TEventBase->EventBase.
Specific use case in folly: folly/experimental/wangle/concurrent/IOThreadPoolExecutor
EventBaseManager itself needs some work/cleanup, but that is for a later diff
For instance, we might try to push towards only allowing access to the singleton, and towards
removing overrides. i.e. only getEventBase. But that's pending an audit of how people are using it now.
Note that the ProfiledMutex protecting the event base set has been made a regular std::mutex
Test Plan:
compiled thrift/lib/cpp|cpp2, made a pass at fixing forward declarations elsewhere
will let contbuild help me iterate
Reviewed By: hans@fb.com
Subscribers: trunkagent, nli, fbcode-common-diffs@, davejwatson, hero-diffs@, zeus-diffs@, andrewcox, netego-diffs@, alandau, apollo-diffs@, antonl, laser-diffs@, ads-dsp-eng@, darshan, micha, njormrod, panin, hdoshi, scuba-diffs@, bmatheny
FB internal diff:
D1590827
Tasks:
5247981
Nathan Bronson [Wed, 8 Oct 2014 23:59:27 +0000 (16:59 -0700)]
fix flaky test from uninitialized std::atomic
Summary:
std::atomic<bool> default constructor doesn't initialize it to
false, so the flag to end the LifoSem.multi_try_wait test was sometimes
set too early. This diff fixes that, makes the test more deterministic,
and also fixes a couple of other benign uninitialized values (so that
they won't be used as prototypes for places where it does matter).
Test Plan: unit tests
Reviewed By: mpawlowski@fb.com
Subscribers: njormrod, lovro
FB internal diff:
D1604508
Tasks:
5336837
Hans Fugal [Wed, 8 Oct 2014 19:06:38 +0000 (12:06 -0700)]
Fix via
Summary: Sometimes you just have to take a step back. :-P
Test Plan: All the unit tests including the one that had been disabled, now pass.
Reviewed By: hannesr@fb.com
Subscribers: meisner, trunkagent, net-systems@, fugalh, exa, njormrod, davejwatson
FB internal diff:
D1596368
Tasks:
4920689,
4480567,
5306911
Marcin Pawlowski [Wed, 8 Oct 2014 19:00:18 +0000 (12:00 -0700)]
stop prereserving space in toAppend
Summary:
previous change would cause reserving
as much as needed in toAppend for all arguements.
This had bad consequences for appending in a loop,
described here: https://phabricator.fb.com/
D1420588#22
Not split the interfaces so that user has to decide
Test Plan: unit tests
Reviewed By: soren@fb.com
Subscribers: trunkagent, njormrod
FB internal diff:
D1601614
Tasks:
5303991
Hans Fugal [Tue, 7 Oct 2014 22:50:32 +0000 (15:50 -0700)]
wangle-bench gflags
Summary: Because windtunnel quirkloads needs `--json` which is enabled with gflags
Test Plan:
$ wangle-bench --json
{
"%hundredThens" :
28133155.
113220215,
"no_contention" :
4842268655.11322,
"%fourThens" :
1495655.
1132202148,
"%twoThens" : 882311.
3632202148,
"oneThen" : 581053.
5507202148,
"-" : 0,
"%withThen" : 559830.
8944702148,
"%promiseAndFuture" : 250840.
66009521484,
"%contention" :
8074419655.11322,
"constantFuture" : 239916.
83197021484
}
Reviewed By: meisner@fb.com
Subscribers: robbert, net-systems@, fugalh, exa, njormrod, davejwatson, jsedgwick
FB internal diff:
D1601364
Tasks:
5277907
Hans Fugal [Tue, 7 Oct 2014 23:36:06 +0000 (16:36 -0700)]
serial/parallel benchmark
Summary: This is an attempt to see the effect of lock contention in the less-common case where a promise is fulfilled at the same time the future is still being set up. Although this is expected to be off the beaten path, it will still be a good idea to keep an eye on how we improve or worsen perf, when we play with different locking strategies (which I intend to do a lot of in the near term).
Test Plan:
============================================================================
folly/wangle/test/Benchmark.cpp relative time/iter iters/s
============================================================================
constantFuture 249.31ns 4.01M
promiseAndFuture 100.88% 247.13ns 4.05M
withThen 43.87% 568.30ns 1.76M
----------------------------------------------------------------------------
oneThen 569.62ns 1.76M
twoThens 63.46% 897.60ns 1.11M
fourThens 39.64% 1.44us 695.90K
hundredThens 1.91% 29.75us 33.61K
----------------------------------------------------------------------------
serial 4.97ms 201.21
parallel 60.22% 8.25ms 121.18
============================================================================
Reviewed By: jsedgwick@fb.com
Subscribers: meisner, net-systems@, fugalh, exa, njormrod
FB internal diff:
D1593010
Nathan Bronson [Wed, 1 Oct 2014 22:46:20 +0000 (15:46 -0700)]
add emulation of futex() for non-Linux systems
Summary:
Inside the Linux kernel, futex() works by hashing the source
address to a fixed set of wakeup lists. When using folly on non-Linux
systems we can emulate something similar by using std::mutex and
std::condition_variable.
Emulating futex() using higher-level APIs is less crazy than it sounds,
because the emulated futex still provides the advantages of the real
one: it allows all of the fast-paths of a new synchronization construct
to be inlined; it is space efficient, taking only 1 word and allowing
the caller to encode state into all of the word's bits; and it avoids
system calls unless a thread actually needs to be put to sleep or
woken up. Think of this as a way of boostrapping something with the
same properties as futex() on platforms that don't expose it publically.
(Presumably these platforms have private APIs that do something similar.)
This diff moves all of the Linux-specific futex stuff into Futex.cpp,
where it is gated by #ifdef __linux__. It also adds an emulated
implementation. The emulated futex will be selected by default on
non-Linux platforms, or it can be used on Linux by using an Atom template
type of folly::detail::EmulatedFutexAtomic. This means, for example,
that you can test MPMCQueue on top of the emulated API by instantiating
a MPMCQueue<ElemType, EmulatedFutexAtomic>.
As a bonus, this refactoring provides a small speed boost by removing
an unnecessary evaluation of the errno macro in the success path of
futexWait.
Test Plan:
1. existing unit tests
2. new unit tests (including tests of Futex users)
3. compile Futex.cpp on OS X (some other build failures still occur)
Reviewed By: delong.j@fb.com
Subscribers: trunkagent, njormrod, yiding, boseant, mssarang
FB internal diff:
D1596118
Tasks:
4952724
Francis Ma [Mon, 6 Oct 2014 23:34:06 +0000 (16:34 -0700)]
Move static member inside the scope of the function
Summary: We are seeing crashes which comes from the initialization of the static global variable. This particular variable is used only in a single function that was never invoked. So moving it into the scope of the function will at least solve the problem. The real issue still requires some deep investigation though.
Test Plan: unitest under folly passed
Reviewed By: subodh@fb.com
Subscribers: seanc, njormrod
FB internal diff:
D1598048
Tasks:
5316441
Nathan Bronson [Mon, 6 Oct 2014 04:04:53 +0000 (21:04 -0700)]
detail::MemoryIdler should use Malloc.h's mallctl decl
Summary:
Malloc.h now takes care to declare mallctl properly on platforms
that aren't using weak symbols, so we should just rely on that for
MemoryIdler.cpp. This is one step toward fixing folly build on OS X.
Test Plan: fbmake runtests
Reviewed By: je@fb.com
Subscribers: yiding, njormrod
FB internal diff:
D1596930
Tasks:
4952724
Blame Revision:
Andrii Grynenko [Thu, 2 Oct 2014 03:34:27 +0000 (20:34 -0700)]
Leak folly::SingletonVault
Summary:
This will ensure SingletonVault is always available. It matters for singletons, not managed by folly::Singleton. Singletons in it will actually be destroyed via static SingletonVaultDestructor.
This does the same as
D1591270 (which got reverted), but doesn't change destruction order.
Test Plan:
Run tests which were broken by
D1591270
Reviewed By: chip@fb.com
Subscribers: njormrod
FB internal diff:
D1594898
Andrii Grynenko [Wed, 17 Sep 2014 04:12:10 +0000 (21:12 -0700)]
Fix folly::Singleton to work with objects w/o default constructor
Test Plan:
unit tests
Reviewed By: ostap@fb.com
Subscribers: netego-diffs@, trunkagent, alikhtarov, marccelani, mshneer, zhuohuang, lins, njormrod
FB internal diff:
D1560406
Anton Likhtarov [Thu, 2 Oct 2014 20:23:20 +0000 (13:23 -0700)]
Add extra assertions in ExternalUnixAddr::free()
Summary: Memory corruption investigation
Test Plan: fbconfig folly/test:network_address_test; fbmake runtests
Reviewed By: davejwatson@fb.com
Subscribers: trunkagent, andrii, njormrod
FB internal diff:
D1592539
Tasks:
5230657
Blame Revision:
D1575098
Daniel Sommermann [Thu, 2 Oct 2014 22:19:27 +0000 (15:19 -0700)]
Revert "Leak folly::SingletonVault"
Summary:
This reverts commit
3cf88fb680b4b9c47189d1e12699e2c24c7ac38b. It
was timing out our tests. probably static (de)initialization order fiasco related.
Test Plan: watch contbuild, git bisect
Reviewed By: afrind@fb.com
Subscribers: doug, njormrod
FB internal diff:
D1593016
Andrii Grynenko [Thu, 2 Oct 2014 03:34:27 +0000 (20:34 -0700)]
Leak folly::SingletonVault
Summary: This will ensure SingletonVault is always available. It matters for singletons, not managed by folly::Singleton. Singletons in it will actually be destroyed via static SingletonVaultDestructor.
Test Plan: unit test
Reviewed By: chip@fb.com
Subscribers: njormrod
FB internal diff:
D1591270
Hans Fugal [Thu, 2 Oct 2014 17:39:53 +0000 (10:39 -0700)]
add some benchmarks
Summary: For great speed
Test Plan:
$ fbmake opt && _bin/folly/wangle/wangle-bench
============================================================================
folly/wangle/test/Benchmark.cpp relative time/iter iters/s
============================================================================
constantFuture 235.92ns 4.24M
promiseAndFuture 100.37% 235.04ns 4.25M
withThen 41.97% 562.17ns 1.78M
----------------------------------------------------------------------------
oneThen 539.03ns 1.86M
twoThens 64.01% 842.14ns 1.19M
fourThens 39.27% 1.37us 728.62K
hundredThens 1.82% 29.63us 33.75K
============================================================================
Reviewed By: davejwatson@fb.com
Subscribers: trunkagent, net-systems@, fugalh, exa, njormrod
FB internal diff:
D1587871
James Sedgwick [Thu, 2 Oct 2014 03:49:37 +0000 (20:49 -0700)]
subscriptions
Summary:
I'm not thrilled with this implementation, but it mostly works. The big bummer is enforcing that Observables are held in shared_ptrs, or rather that enforcing that condition is impossible.
The protected constructor / friended dumb make_shared() pattern is clunky, and it'd be really easy for a subclasser to shoot themselves in the foot or even in the face.
It does seem like maybe Observable should be made an interface again, and all these details should live in a subclass (FanoutObservable?) where the restriction are super obvious. For instance, the langtech AudioStream object doesn't need all this crap because it overrides subscribe() without using the observer list, but it subclasses anyways.
I'm noodling another approach that (if it works) will not require the shared_ptr dancing, but will probably have some additional overhead... the observable would have to keep track of the subscriptions itself.
I like the RAII subscriptions, though perhaps subscriptions should be optional as long as it's clear that your subscription will last forever it you opt out of them.
Thoughts?
Test Plan: added unit
Reviewed By: hans@fb.com
Subscribers: rushix, hannesr, trunkagent, fugalh, mwa, jgehring, fuegen, njormrod, bmatheny
FB internal diff:
D1580443
Adam Simpkins [Sun, 28 Sep 2014 01:08:34 +0000 (18:08 -0700)]
add stringVPrintf() and stringVAppendf()
Summary:
This adds versions of stringPrintf() and stringAppendf() that accept
va_list arguments.
This also fixes the existing stringPrintf() functions to properly call
va_end() even when an exception is thrown in stringPrintfImpl().
Test Plan: Included new unit tests for stringVPrintf() and stringVAppendf().
Reviewed By: davejwatson@fb.com
Subscribers: trunkagent, doug, net-systems@, exa, njormrod
FB internal diff:
D1583675
James Sedgwick [Wed, 1 Oct 2014 17:55:34 +0000 (10:55 -0700)]
in thread pools, take factory as shared ptr
Summary: needed for thrift server. if you have some sort of stateful/functional factory and you want to hold on to it, you should be allowed to
Test Plan: compiles with forthcoming thrift diff
Reviewed By: davejwatson@fb.com
Subscribers: trunkagent, fugalh, njormrod
FB internal diff:
D1584587
James Sedgwick [Wed, 1 Oct 2014 17:32:54 +0000 (10:32 -0700)]
expose event base from IOThreadPoolExecutor
Summary:
I'm not 100% sure this is the best way to go about this but I don't hate it either.
I'm going to start seeing how it might fit into tserver - my guess is that some sort Cpp2WorkerFactory which also manages those objects would get plugged in as the thread factory
Haven't fleshed out how this would relate to TEventBaseManager
Test Plan: added unit, starting to play with this in Thrift2 server
Reviewed By: davejwatson@fb.com
Subscribers: alandau, bmatheny, trunkagent, fugalh, njormrod
FB internal diff:
D1574660
Dave Watson [Tue, 30 Sep 2014 23:17:25 +0000 (16:17 -0700)]
Bump version to 11:0
Hans Fugal [Tue, 30 Sep 2014 22:52:52 +0000 (15:52 -0700)]
(Wangle) Make via behave more like gate
Summary:
Could the problem be that via continues the existing chain of futures,
whereas we actually want to start a new chain?
Is there any particular reason this wasn't implemented like this originally?
Test Plan:
Ran all the unit tests. I hope to try to reproduce the thread issue and
see if this improves things.
Reviewed By: davejwatson@fb.com
Subscribers: trunkagent, net-systems@, exa, njormrod, fugalh
FB internal diff:
D1500225
Tasks:
4920689
Andrii Grynenko [Wed, 24 Sep 2014 00:50:05 +0000 (17:50 -0700)]
remove get_weak from singleton
Summary:
Test Plan: fbmake runtests, OBC no longer segfaults
Reviewed By: mshneer@fb.com
Subscribers: fbcode-common-diffs@, trunkagent, mcduff, marccelani, hitesh, mshneer, njormrod, lins
FB internal diff:
D1573880
Alan Frindell [Mon, 29 Sep 2014 20:01:49 +0000 (13:01 -0700)]
Fix compiler warning in Subprocess
Summary: I'm going to have hhvm depend on Subprocess, found this build error in open source. I figured the intent was to set r to the return code. I could also delete the whole thing since it's unused.
Test Plan: Unit tests
Reviewed By: tudorb@fb.com
Subscribers: trunkagent, mwilliams, doug, njormrod
FB internal diff:
D1583626
Haijun Zhu [Mon, 29 Sep 2014 17:09:38 +0000 (10:09 -0700)]
Fix bug and unit test failure in HHWheelTimer
Summary: Some minor bug and a failed test caused by
D1578466
Test Plan:
fbconfig thrift/lib/cpp/test:HHWheelTimerTest && fbmake
runtests
Reviewed By: davejwatson@fb.com
Subscribers: trunkagent, alandau, bmatheny, njormrod
FB internal diff:
D1581949
Andrii Grynenko [Fri, 26 Sep 2014 02:02:37 +0000 (19:02 -0700)]
Use RWSpinLock for Singleton mutex
Summary: We only need exclusive lock when we add items to singletons_. Each SingletonEntry has its own mutex, so it's safe to rely on it for any modifications within individual entries.
Test Plan: Applied
D1573880 and ran fbconfig -r servicerouter/client/cpp2 && fbmake runtests
Reviewed By: chip@fb.com
Subscribers: trunkagent, njormrod, hitesh, mshneer
FB internal diff:
D1579877
Nicholas Ormrod [Fri, 26 Sep 2014 22:19:51 +0000 (15:19 -0700)]
Malloc cannot include ScopeGuard
Summary:
Because we put FBString and Malloc into libgcc, we have to be
careful which dependencies they each have. We cannot include ScopeGuard.
Reorganize the code a bit to avoid the ScopeGuard.
Test Plan: fbconfig -r folly && fbmake runtests
Reviewed By: je@fb.com
Subscribers: trunkagent, sdwilsh, njormrod
FB internal diff:
D1580957
Nicholas Ormrod [Fri, 26 Sep 2014 22:17:04 +0000 (15:17 -0700)]
Rare bug fix
Summary:
By the birthday paradox, this will fail about once every 8,100
runs. Dropping to 100 with cut that to 1 in 860,000.
Test Plan: fbconfig -r folly && fbmake runtests
Reviewed By: sdoroshenko@fb.com
Subscribers: sdwilsh, njormrod
FB internal diff:
D1581238
Anton Likhtarov [Fri, 26 Sep 2014 22:29:05 +0000 (15:29 -0700)]
Bump version to 10:0
James Sedgwick [Fri, 26 Sep 2014 19:17:01 +0000 (12:17 -0700)]
fix future executor test
Summary: This should be the last test abusing sleeps.
Test Plan: ran
Reviewed By: hannesr@fb.com
Subscribers: fugalh, njormrod
FB internal diff:
D1580830
Tasks:
5225808
Jim Meyering [Thu, 25 Sep 2014 22:25:36 +0000 (15:25 -0700)]
folly/GroupVarint: fix a clang-caught heap-buffer-overrun
Summary:
Avoid a clang-caught heap-buffer-overrun
and document that the ssse3 decoder requires at least 17 bytes
of readable memory starting at the source pointer.
* folly/GroupVarint.h (decode_simple): Comment out dead code.
(decode)[__SSSE3__]: Add a comment describing this limitation.
* folly/test/GroupVarintTest.cpp (testGroupVarint32):
Include <algorithm> for use of std::max.
Ensure that the buffer we use has length at least 17.
Test Plan:
Now, all tests succeed with clang/asan
Reviewed By: simpkins@fb.com
Subscribers: trunkagent, mathieubaudet, njormrod
FB internal diff:
D1579109
Tasks:
5241437
James Sedgwick [Fri, 26 Sep 2014 15:56:56 +0000 (08:56 -0700)]
RFC: future executor
Summary:
not necessarily for commit but i was playing around a bit with this today to see what it might look like, so i figured i'd put it up
i assume this shenanigan isn't remotely safe (threadwise) without some extra magic... @fugalh we chatted about this last time i was in mpk a bit.
maybe addFuture could take an executor to make sure the promise is fulfilled on the correct thread, or something.
you'd have to reimplement timeouts via this executor or via futures if you wanted to get them via the futures channel, which makes sense anyway as this could be used with any Executor and not just ThreadPoolExecutor which owns that implementation
also specialized makeFutureTry to take functions that return futures but treat them like they return the contained types, so fulfil() can be used liked then()
this specialization could just as easily be done on fulfil() itself if we wanted.
Test Plan: unit
Reviewed By: davejwatson@fb.com
Subscribers: hannesr, trunkagent, craffert, njormrod, fugalh, bmatheny
FB internal diff:
D1566369
James Sedgwick [Fri, 26 Sep 2014 15:48:09 +0000 (08:48 -0700)]
fix more flakey tests
Summary: fixed the one brought up in the task, and tweaked another that could theoretically cause problems
Test Plan: ran, though I have not been able to repro the
Reviewed By: njormrod@fb.com
Subscribers: fugalh, njormrod
FB internal diff:
D1575632
Tasks:
5225808
Simon Jenkins [Thu, 25 Sep 2014 21:25:47 +0000 (14:25 -0700)]
Set DEFAULT_CATCHUP_EVERY_N to 10 to get more accurate task expiration when busy event base
Summary:
when event base is busy we're timing out tasks too early.
BIG SCARY NOTE: I don't have the thrift knowledge to know if there's sideeffects of this, please let
me know if this change is harmful.
Test Plan:
tested internally
Reviewed By: davejwatson@fb.com
Subscribers: njormrod
FB internal diff:
D1578466
Tasks:
4752162