- 15 Nov, 2015 1 commit
-
-
Yedidya Feldblum authored
Summary: [Folly] Simplify some checks by using `__CLANG_PREREQ`. Reviewed By: meyering Differential Revision: D2656842 fb-gh-sync-id: e762fba423fb7cc08907d10dc10f8f46d93a9fe4
-
- 13 Nov, 2015 2 commits
-
-
Andrii Grynenko authored
Summary: This doesn't make the code less efficient, because RVO can't be used for CollectVariadicContext. Thus moving existing tuple vs constructing new tuple by moving all values from other tuple (where each value is wrapped in folly::Optional) should be pretty much the same. Reviewed By: hannesr Differential Revision: D2650293 fb-gh-sync-id: 648a358bf093a0bb9d058a997af9bf59014ad77c
-
Yang Chi authored
Summary: Add a new method to cork a socket in a persistent manner, instead of the current on-off manner. This is default to false. The liger part of turning this on will be in a separate diff. I thought about whether I need to turn cork off based on some criteria to alleviate the perf degradation. The obvious things I can think off is just amount of data written as a threshold, or a timeout. But TCP is doing this already for us, unless we want the data threshold to be less than MSS, or we want the timeout to be less than 200ms. THoughts? Reviewed By: shikong Differential Revision: D2639260 fb-gh-sync-id: 2821f669c9f72d5ac4c33195bb192fc4110ffe9d
-
- 12 Nov, 2015 3 commits
-
-
Giuseppe Ottaviano authored
Reviewed By: philippv Differential Revision: D2643313 fb-gh-sync-id: 10b9f735725ce47fab4bbfaa5972b3863357365f
-
Giuseppe Ottaviano authored
Reviewed By: Gownta Differential Revision: D2643850 fb-gh-sync-id: 2c4bb844ea2006215b0637cb1ba08c636faefe05
-
Chad Parry authored
Summary: Changing the definition of `HHWheelTimer::UniquePtr` wasn't safe, because some clients were using that type outside of the `HHWheelTimer::newTimer` helper. I'm changing that part back. We'll still be able to proceed with my other codemod to `HHWheelTimer`, but we'll always have two different smart pointer types to manage: `UniquePtr` and `IntrusivePtr`. Reviewed By: djwatson Differential Revision: D2644721 fb-gh-sync-id: 14685be62355f09d39c4139ef7186d60b5f48dcd
-
- 11 Nov, 2015 6 commits
-
-
Stepan Palamarchuk authored
Summary: It never throws. Reviewed By: pavlo-fb Differential Revision: D2640886 fb-gh-sync-id: cd643f8847f4bf5619415731484f91fb07116784
-
Yang Chi authored
Summary: This is probably easier than D2612490. The idea is just to add a callback to write, writev and writeChain in AsyncSocket, so upper layer can know when data starts to buffer up Reviewed By: mzlee Differential Revision: D2623385 fb-gh-sync-id: 98d32ca83871aaa4f6c75a769b5f1bf0b5d62c3e
-
Chad Parry authored
Summary: There have been several ASAN bugs cropping up because the lifetime of an `HHWheelTimer` is not being manager properly. I think that people are too comfortable passing around a raw `HHWheelTimer*` even when it is difficult to prove correctness. The recommended solution used to be to create a `DestructorGuard` every time it was needed. There is enough friction there that not everyone is doing that like they should. We should make resource management easier---as easy as using raw pointers, in fact. I've fixed the broken copy semantics of `DestructorGuard` and added the operators that allow it to be used as a smart pointer. I added the `IntrusivePtr` helper that can manage an arbitrary derived class of `DelayedDestructionBase`. Now, clients can all safely pass around an `IntrusivePtr` everywhere they used to use a raw pointer. They will get automatic resource management for free. If you are not convinced that `DestructorGuard` should be changed, then note that the existing behavior is dangerously buggy. Consider the following innocent code that accidentally uses the implicitly-defined copy constructor: auto d = DestructorGuard(p); This results in undefined behavior, because `p` can be accessed after it is destroyed! The bug happens because the default copy constructor copies the raw pointer but doesn't increment the count. In a separate diff, I'll change all clients who pass around a raw `HHWheelTimer*` to use an `IntrusivePtr<HHWheelTimer>` instead. Then we can even entertain a long-term plan of switching from intrusive pointers to the standard `shared_ptr`. Reviewed By: djwatson Differential Revision: D2627941 fb-gh-sync-id: 58a934d64540d0bbab334adc4f23d31d507692da
-
Shaft Wu authored
Summary: My colleague tuomaspelkonen discovered a weird UNSYNCHRONIZED issue a few weeks ago and we ever since stopped using it. Now I finally have some time to root cause it. It turns out UNSYNCHRONIZED unlock the mutex then lock the mutex again, because it copy constructs LockedPtr for overriding the name within the scope, and copy construct locks the mutex again. A one character fix here is to take a reference of LockedPtr instead of copy construct it. However since this is my first time look at the code here, please advise if this is horribly wrong or propose better fix. Also added a test to reproduce the issue without the fix as well as verify the fix. Reviewed By: yfeldblum Differential Revision: D2633028 fb-gh-sync-id: a9e8d39b08d4d1265979f8bdaae83619566d10a0
-
Alexander Shaposhnikov authored
Summary: hg backout -r c9f7b5f3185a Revert my change (which broke down the cont build t9048692) Reviewed By: djwatson Differential Revision: D2640797 fb-gh-sync-id: 51f196ac5a3560fde4dc8fe7bb6ef278d74136e5
-
Alexander Shaposhnikov authored
Summary: Wait uses baton & callback running baton.post when the original future is ready. However wrapping baton.post with a funciton call (preparing a new value) adds the following race: baton.wait wakes up before that function has call actually finished. The explanation is the following: to prepare the value of the new future it's necessary 1. baton.post() 2. set the value (move constructor, memory operations, ...) and this code is executed in a separate thread. The main idea of this fix is to avoid creating a new future (whose value is calculated using that 2-step procedure) and set a callback instead. This callback will be executed when the future is ready and actually it either will be the last step of promise.setValue or it will run immediately if the future we are waiting for already contains a value. Reviewed By: fugalh Differential Revision: D2636409 fb-gh-sync-id: df3e9bbcc56a5fac5834ffecc63f1bcb94ace02c
-
- 10 Nov, 2015 2 commits
-
-
Shijin Kong authored
Summary: Revert D2632752 Reviewed By: afrind Differential Revision: D2638974 fb-gh-sync-id: 43d8135421510db7840a99f3c197119a0fd26c09
-
Steve O'Brien authored
Summary: Using Clang 3.7, this minimal test program: #include <folly/FBString.h> struct S { folly::basic_fbstring<char> x; }; int main(int argc, char *argv[]) { S s {}; return 0; } ... breaks with the following error output: FBStringTest.cpp:5:8: error: chosen constructor is explicit in copy-initialization S s {}; ^ ./folly/FBString.h:1009:12: note: constructor declared here explicit basic_fbstring(const A& = A()) noexcept { ^ FBStringTest.cpp:3:40: note: in implicit initialization of field 'x' with omitted initializer struct S { folly::basic_fbstring<char> x; }; ... because this `basic_fbstring` is used in a struct and the struct is being default-constructed. This patch splits the (nevertheless-correct) one-default-arg constructor into two, to deconfuse clang and have it select the correct constructor to avoid this issue. Reviewed By: matbd Differential Revision: D2632953 fb-gh-sync-id: 2c75ae85732678c31543f5cccc73d58317982f07
-
- 09 Nov, 2015 3 commits
-
-
Shijin Kong authored
Summary: The check was originally added to track down cpu pinning issue. In reality the check did get triggered, so change to soft error to calm the crazy nag bot. what's wrong with you. We need to see the soft error log to find out what the error is. Reviewed By: mzlee Differential Revision: D2632752 fb-gh-sync-id: 87c11b186f97f0eb4a6c5ac13a1117b280198673
-
Alexey Spiridonov authored
Summary: Without this tag type, it's impossible to use `Synchronized` with types like: ``` struct A { A(int, const char*); A(const A&) = delete; A& operator=(const A&) = delete; A(A&&) = delete; A& operator=(A&&) = delete; }; ``` In-place construction solves this problem. Usage: ``` Synchronized a(construct_in_place, 5, "c"); ``` Reviewed By: nbronson Differential Revision: D2610071 fb-gh-sync-id: 251fe8f8f6a2d7484dda64cf04dcacb998145230
-
Chad Parry authored
Summary: This is the solution for our `HHWheelTimer` crashes, as suggested in D2617966#25. djwatson also mentioned that maybe there should be some logging in the destructor if there are outstanding callbacks. I couldn't think of anything that would add to the `assert` that already exists in `destroy`. I'm open to suggestions though. Reviewed By: djwatson Differential Revision: D2628154 fb-gh-sync-id: f3db6e9384517c9bf3cbb60af8c1e711703a07fa
-
- 08 Nov, 2015 1 commit
-
-
Igor Sugak authored
Summary: Define `__CLANG_PREREQ` macro to check version of clang. Reviewed By: yfeldblum Differential Revision: D2630325 fb-gh-sync-id: 3d666e554e8ddfc2c1fecd439aaf93f015829025
-
- 06 Nov, 2015 2 commits
-
-
Chad Parry authored
Summary: Callbacks sometimes outlive the `HHWheelTimer` that they reference. Then the `Callback` tries to reference the dead `HHWheelTimer` and it could either misbehave or crash. This was caught reliably by ASAN tests. Since `HHWheelTimer` already supports intrusive ref counting, the solution is to acquire a reference within the `Callback`. Reviewed By: djwatson Differential Revision: D2617966 fb-gh-sync-id: 02be9ffc5851c269d5933288a17ad394b33ac2dd
-
Jon Maltiel Swenson authored
Summary: Do not cancel TimeoutHandler timeout if the timeout has already run. Reviewed By: spalamarchuk Differential Revision: D2622280 fb-gh-sync-id: 27d83b44ab225e2859695f4e5f21cef934824a35
-
- 05 Nov, 2015 3 commits
-
-
Haijun Zhu authored
Summary: This name's meaning is very obscure, this method actually sets the queue size of each acceptor and if all acceptor's queues are full the connection will be dropped. Change it to a more meaningful name. Reviewed By: alandau Differential Revision: D2613681 fb-gh-sync-id: baa374cdf0a87c460df3dd5687e3d755b55f4b4f
-
Jon Maltiel Swenson authored
Summary: Start server timeout after socket write succeeds in mcrouter. Add neceessary Fibers logic to enable this behavior. Reviewed By: pavlo-fb Differential Revision: D2613344 fb-gh-sync-id: 1bc0fbe8b325a3e91cd010f89104b83ebf183679
-
Viswanath Sivakumar authored
Summary: If SharedPromise::getFuture() is invoked after a call to setInterruptHandler, then the interrupt handler isn't set on the newly created promise. This diff fixes that. Reviewed By: yfeldblum Differential Revision: D2610289 fb-gh-sync-id: bf8fce9e881b83ccac17d13c6788ec2afd0b0153
-
- 04 Nov, 2015 4 commits
-
-
Subodh Iyengar authored
Summary: Allows AsyncTransportWrapper's to supply the underlying application protocol being used, for example using NPN for SSL or some other generic protocol indication mechanism. Reviewed By: ranjeeth Differential Revision: D2614179 fb-gh-sync-id: 2079782bb7d44f898fb14c7df15077209b022424
-
Zhen (Growth) Li authored
Summary: Our use case need to have the IndexType Configurable, but in this diff D2583752, lint error shows Code from folly::detail is logically private, please avoid use outside of folly. In order to fix this lint error, I switch the IndexType and Allocator in the AtomicUnorderedInsertMap template. Reviewed By: nbronson Differential Revision: D2610921 fb-gh-sync-id: ae81b41e7c8c971f26c61b8c67dabeadff379584
-
Subodh Iyengar authored
Summary: Allow underlying transport to be accessible from AsyncTransportWrapper. There are some code paths where we need access to the real transport from the AsyncTransportWrapper. This allows us to retrieve the underlying transport and have clients like HTTPSession use it. Reviewed By: afrind Differential Revision: D2609200 fb-gh-sync-id: 2b317d1825a005bb64468f83c64bc3f1c9bdfe2c
-
Louis Brandy authored
Summary: .. by including what you use. These are a handful of headers that are depended on via viral inclusion. Include them directly. Reviewed By: yfeldblum Differential Revision: D2601172 fb-gh-sync-id: 215e87263325d085fbcb651f83f429f47d14fc1b
-
- 03 Nov, 2015 2 commits
-
-
Louis Brandy authored
Summary: Prefer `std::move` to `move` for clarity's sake. Reviewed By: yfeldblum Differential Revision: D2601138 fb-gh-sync-id: 64a9e58a83da9c8416a0be5c45a292c6b1342664
-
Anton Likhtarov authored
Summary: This disallows implicitly constructing a StringPiece from a literal nullptr at compile time (without this change, nullptr would cause a segfault in strlen()). Reviewed By: meyering, andriigrynenko Differential Revision: D2603597 fb-gh-sync-id: cafbc45945bacc72a7c89310b99aa62d19a3ff9f
-
- 31 Oct, 2015 1 commit
-
-
Andrew Gallagher authored
Summary: Fix warnings triggered by `-Wcomment`. Reviewed By: ajtulloch Differential Revision: D2603992 fb-gh-sync-id: aae721f7c236d7d8b8bff2c077a481fc2affcf71
-
- 30 Oct, 2015 2 commits
-
-
Delyan Kratunov authored
Summary: New API - write an entry and advance a cursor to the just-written slot. Reviewed By: ritzau, yfeldblum Differential Revision: D2599388 fb-gh-sync-id: f3ebfae97de0341f01ffc80ab10221313d02087c
-
Jon Maltiel Swenson authored
Summary: Reclaim unnecessary fibers from the free pool periodically. Turn this behavior on in mcrouter/proxy.cpp Reviewed By: pavlo-fb Differential Revision: D2584825 fb-gh-sync-id: eabc58eefe6fd38972c9e23ca3cbe79eb8316a3e
-
- 29 Oct, 2015 4 commits
-
-
Justin Gibbs authored
Summary: Convert helper classes to using statements now that GCC has caught up with c++11. Reviewed By: simpkins Differential Revision: D2561364 fb-gh-sync-id: 712591549aba9450d159468dc3b26a987ffe9b82
-
Andrii Grynenko authored
Summary: try_get() API is confusing right now, because it returns nullptr in release, but crashes in debug build. Users end up handling nullptr returns, yet their code crashes in debug builds for no reason. See https://www.facebook.com/groups/fbthrift/permalink/1222120054481561/ for an example. If we want to keep the crashing API we should probably name it differently and make it crash both in debug and release. Reviewed By: dhruvbird Differential Revision: D2587818 fb-gh-sync-id: 5834bfa08eb5d9bc6db1c5edf4a048a5b1d3212c
-
Xiaofan Yang authored
Summary: In my use case, 1.5 billion keys with loadFactor 0.8, the linear probing performs really bad. Reviewed By: nbronson Differential Revision: D2579243 fb-gh-sync-id: 5081356de55f770823a4afad55bf7e2114b4e313
-
Pavlo Kushnir authored
Summary: startWork_ is used only if enableTimeMeasurement is set. It gives ~0.5% performance win for mcrouter. Reviewed By: yfeldblum Differential Revision: D2590176 fb-gh-sync-id: 07f2189ebdec751cd0d91d191d8f595780d2808a
-
- 28 Oct, 2015 2 commits
-
-
Blake Matheny authored
Summary: SYNCHRONIZED warns with -Wshadow due to `for (auto& FB_ARG_1(__VA_ARGS__) =`. This diff just suppresses that warning. Reviewed By: djwatson Differential Revision: D2587348 fb-gh-sync-id: 3a2e39fb6ce28da014950ca94e4b62ea80deb65f
-
Beny Luo authored
Summary: The implicit conversion loses integer precision: 'size_type' (aka 'unsigned long') to 'int'. Reviewed By: mzlee Differential Revision: D2585883 fb-gh-sync-id: 1fc7c84b66c8f19cc36b798dd198730764e19b28
-
- 27 Oct, 2015 2 commits
-
-
Shaft Wu authored
Summary: Further race is presented in HHWheelTimer and EventBase, since changing them to track callback life cycle is more involving, I am hacking around it to prove the concept. With the *fix*, no reproduce of the segmentation fault anymore. This is for proving the concept, code will be cleaned up if we agree this is reasonable direction to pursue. Reviewed By: fugalh Differential Revision: D2577715 fb-gh-sync-id: e0bb0317268e6f02a54fc70454e63959fba70c10
-
Jun Li authored
Summary: Expose pending messages in accept queue in AsyncServerSocket. Set default accept message queue size to 1024. Reviewed By: djwatson Differential Revision: D2525161 fb-gh-sync-id: a69ea0ee77729e4a8300bde3e3c07840f2d5d3cb
-