1. 15 Oct, 2014 15 commits
    • Hans Fugal's avatar
      wangle-bench gflags · b692cb2f
      Hans Fugal authored
      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
      b692cb2f
    • Hans Fugal's avatar
      serial/parallel benchmark · d6488e3c
      Hans Fugal authored
      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
      d6488e3c
    • Nathan Bronson's avatar
      add emulation of futex() for non-Linux systems · d3ee675f
      Nathan Bronson authored
      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
      d3ee675f
    • Francis Ma's avatar
      Move static member inside the scope of the function · e401a93c
      Francis Ma authored
      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
      e401a93c
    • Nathan Bronson's avatar
      detail::MemoryIdler should use Malloc.h's mallctl decl · a04f2681
      Nathan Bronson authored
      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:
      a04f2681
    • Andrii Grynenko's avatar
      Leak folly::SingletonVault · f0c0e8d2
      Andrii Grynenko authored
      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
      f0c0e8d2
    • Andrii Grynenko's avatar
      Fix folly::Singleton to work with objects w/o default constructor · 21487671
      Andrii Grynenko authored
      Test Plan:
      unit tests
      
      Reviewed By: ostap@fb.com
      
      Subscribers: netego-diffs@, trunkagent, alikhtarov, marccelani, mshneer, zhuohuang, lins, njormrod
      
      FB internal diff: D1560406
      21487671
    • Anton Likhtarov's avatar
      Add extra assertions in ExternalUnixAddr::free() · 692a99ce
      Anton Likhtarov authored
      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
      692a99ce
    • Daniel Sommermann's avatar
      Revert "Leak folly::SingletonVault" · 11ff9f99
      Daniel Sommermann authored
      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
      11ff9f99
    • Andrii Grynenko's avatar
      Leak folly::SingletonVault · c64d359b
      Andrii Grynenko authored
      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
      c64d359b
    • Hans Fugal's avatar
      add some benchmarks · fd81069d
      Hans Fugal authored
      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
      fd81069d
    • James Sedgwick's avatar
      subscriptions · 30d69c32
      James Sedgwick authored
      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
      30d69c32
    • Adam Simpkins's avatar
      add stringVPrintf() and stringVAppendf() · 0606460a
      Adam Simpkins authored
      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
      0606460a
    • James Sedgwick's avatar
      in thread pools, take factory as shared ptr · edbdc3d4
      James Sedgwick authored
      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
      edbdc3d4
    • James Sedgwick's avatar
      expose event base from IOThreadPoolExecutor · ca5fc366
      James Sedgwick authored
      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
      ca5fc366
  2. 30 Sep, 2014 8 commits
    • Dave Watson's avatar
      Bump version to 11:0 · 5617e556
      Dave Watson authored
      5617e556
    • Hans Fugal's avatar
      (Wangle) Make via behave more like gate · 684bae3b
      Hans Fugal authored
      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
      684bae3b
    • Andrii Grynenko's avatar
      remove get_weak from singleton · c9f0ed55
      Andrii Grynenko authored
      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
      c9f0ed55
    • Alan Frindell's avatar
      Fix compiler warning in Subprocess · eb9847c8
      Alan Frindell authored
      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
      eb9847c8
    • Haijun Zhu's avatar
      Fix bug and unit test failure in HHWheelTimer · b01aec06
      Haijun Zhu authored
      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
      b01aec06
    • Andrii Grynenko's avatar
      Use RWSpinLock for Singleton mutex · 4833e0ef
      Andrii Grynenko authored
      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
      4833e0ef
    • Nicholas Ormrod's avatar
      Malloc cannot include ScopeGuard · 23d9f0ec
      Nicholas Ormrod authored
      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
      23d9f0ec
    • Nicholas Ormrod's avatar
      Rare bug fix · b9719790
      Nicholas Ormrod authored
      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
      b9719790
  3. 26 Sep, 2014 17 commits
    • Anton Likhtarov's avatar
      Bump version to 10:0 · 0b326f2e
      Anton Likhtarov authored
      0b326f2e
    • James Sedgwick's avatar
      fix future executor test · bb8907be
      James Sedgwick authored
      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
      bb8907be
    • Jim Meyering's avatar
      folly/GroupVarint: fix a clang-caught heap-buffer-overrun · 05450621
      Jim Meyering authored
      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
      05450621
    • James Sedgwick's avatar
      RFC: future executor · 61386620
      James Sedgwick authored
      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
      61386620
    • James Sedgwick's avatar
      fix more flakey tests · fa43917d
      James Sedgwick authored
      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
      fa43917d
    • Simon Jenkins's avatar
      Set DEFAULT_CATCHUP_EVERY_N to 10 to get more accurate task expiration when busy event base · 3f54bba7
      Simon Jenkins authored
      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
      3f54bba7
    • Dave Watson's avatar
      Fix SocketAddress AF_UNIX support · 7b474bc6
      Dave Watson authored
      Summary: check external_ vs. AF_UNIX.  Also fix reset() to reset storage_.addr
      
      Test Plan: fbconfig folly/test:network_address_test; fbmake runtests
      
      Reviewed By: soren@fb.com
      
      Subscribers: tudorb, trunkagent, doug, njormrod
      
      FB internal diff: D1575098
      7b474bc6
    • Andrew Gallagher's avatar
      Add and fix BUCK files for folly · 96be3888
      Andrew Gallagher authored
      Summary:
      This gets almost all of folly building with buck.  There are a few
      cases we don't support yet (custom test wrappers for spooky test
      binaries, dlopen-enabled exception tracer binary, etc), but the
      vast majority is covered.
      
      Test Plan: built all build targets and ran all tests buck
      
      Reviewed By: sdwilsh@fb.com
      
      Subscribers: tjackson, sdwilsh, fugalh, njormrod
      
      FB internal diff: D1577256
      
      Tasks: 5055879
      96be3888
    • Marc Horowitz's avatar
      expose the class_name in an efficient way · d7364d7e
      Marc Horowitz authored
      Summary:
      Add a class_name method, which doesn't need to throw/catch
      regardless how the exception is captured.
      
      Test Plan: exception_wrapper_test
      
      Reviewed By: mshneer@fb.com
      
      Subscribers: njormrod
      
      FB internal diff: D1543596
      
      Tasks: 5025089
      
      Blame Revision:
      d7364d7e
    • Nicholas Ormrod's avatar
      Disable failing wangle test · 215076a5
      Nicholas Ormrod authored
      Summary:
      Under moderate load, this part of the test would fail. When
      there is resource contention, it is quite possible that the timeout will
      be exceeded. A further reduction of the time spread will render this
      test next to useless.
      
      Moderate load can be as little as running all of folly's tests at once,
      which is something that our testing infrastructure does quite
      frequently.
      
      fbconfig -r folly
      fbmake runtests
      
      Test Plan:
      fbconfig -r folly && fbmake runtests
      
      Reviewed By: hannesr@fb.com
      
      Subscribers: sdwilsh, fugalh, njormrod
      
      FB internal diff: D1576570
      
      Tasks: 5225808
      215076a5
    • Soren Lassen's avatar
      Fix some folly memory leaks found with clang:dev + asan · 03e99d2e
      Soren Lassen authored
      Summary:
      Reverts the revert of the small_vector and test parts of D1569246.
      I'm saving the SocketAddress fix for later, because it caused problems
      and needs more research.
      
      Test Plan:
      unit tests with clang:dev/asan
      
      Reviewed By: njormrod@fb.com
      
      Subscribers: mathieubaudet, philipp, meyering, njormrod
      
      FB internal diff: D1575574
      
      Blame Revision: D1575190
      03e99d2e
    • Dave Watson's avatar
      update thrift to use managed connection · d8e71911
      Dave Watson authored
      Summary:
      Use the new managed connection code in folly.  Cleans things up a fair bit
      
      The only catch was the current ConnectionManager code uses raw pointers, thrift expects shared_ptr so the request object can keep the channel around until the requests are finished
      
      Test Plan: fbconfig -r thrift/lib/cpp2; fbmake runtests
      
      Reviewed By: afrind@fb.com
      
      Subscribers: trunkagent, njormrod, doug, fugalh, alandau, bmatheny, dcsommer, afrind
      
      FB internal diff: D1519923
      
      Tasks: 5002343
      d8e71911
    • Danny Guo's avatar
      Revert "Fix folly memory leaks found with clang:dev + asan." · f760023d
      Danny Guo authored
      Summary: This reverts commit 2c726dcf86bb176fb3695739a3d8bca2f95c41e6.
      
      Test Plan: build it
      
      Reviewed By: soren@fb.com
      
      Subscribers: woo, mathieubaudet, njormrod
      
      FB internal diff: D1575190
      f760023d
    • James Sedgwick's avatar
      de-flake tests · 471b4b72
      James Sedgwick authored
      Summary: make these more serialized / event based so they don't get flakey under high load
      
      Test Plan:
      ran under load - caveat: i was not able to repro the flakiness @njormrod reported
      but by inspection these should be fine
      
      Reviewed By: njormrod@fb.com
      
      Subscribers: fugalh, njormrod
      
      FB internal diff: D1574640
      
      Tasks: 5225808
      471b4b72
    • Soren Lassen's avatar
      Fix folly memory leaks found with clang:dev + asan. · cae6c97a
      Soren Lassen authored
      Summary: I tried running folly/test with clang:dev and asan and found these leaks.
      
      Test Plan:
      unit tests with clang:dev/asan
      
      Reviewed By: davejwatson@fb.com
      
      Subscribers: davejwatson, trunkagent, mathieubaudet, njormrod, meyering, philipp
      
      FB internal diff: D1569246
      cae6c97a
    • James Sedgwick's avatar
      user-defined expirations · 395c7e78
      James Sedgwick authored
      Summary:
      Couple of notes:
      1. is it a bummer not to have per-task callbacks of some kind? the interfaces set up here only tell you that some task expired, not which one expired. TM calls back with the Runnable object. is that useful?
      2. std::chrono::* business is frustratingly verbose, but the safety/explicitness is nice. Not sure how I feel overall.
      3. perhaps expirations should be given in microseconds even if we don't think we can accurately accomplish that
      
      Test Plan: added unit
      
      Reviewed By: hans@fb.com
      
      Subscribers: fugalh, njormrod, bmatheny
      
      FB internal diff: D1563520
      395c7e78
    • Pavlo Kushnir's avatar
      Fix fbthrift build · 39d6939c
      Pavlo Kushnir authored
      Summary: Mcrouter open source build is failing because Codel is missing in fbthrift.
      
      Test Plan: visual
      
      Reviewed By: davejwatson@fb.com
      
      Subscribers: fugalh, njormrod
      
      FB internal diff: D1572122
      
      Blame Revision: D1566128
      39d6939c