1. 16 Sep, 2016 5 commits
    • Dominik Gabi's avatar
      escape `{` in cmd for deprecated `Substring(std::string...)` · d9749c81
      Dominik Gabi authored
      Summary:
      Since `shellify` interprets the command as a format string we need to escape
      `{` and `}`.
      
      Reviewed By: ldemailly, simpkins
      
      Differential Revision: D3874605
      
      fbshipit-source-id: f47db387c3a44a3ba1c0c1d4726b7212fcb5ef3e
      d9749c81
    • Adam Simpkins's avatar
      update stats APIs to use TimePoint vs Duration correctly · 42ca5613
      Adam Simpkins authored
      Summary:
      Update the stats APIs to correcly distinguish between TimePoint and Duration
      types.
      
      This does leave addValue() and update() APIs in place that accept Duration
      values, for backwards compatibility.  These should eventually be removed once
      all code has been converted to call the new APIs.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D3808805
      
      fbshipit-source-id: 36d6574ba4a09db7eb9f1a35e47addd3e07f8461
      42ca5613
    • Tom Jackson's avatar
      Return rvalue references from &&-qualified members of dynamic · 864eb2a6
      Tom Jackson authored
      Summary: Let the caller do a move, don't force one.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D3873129
      
      fbshipit-source-id: 40c6bf564bcbf794830c99ea1248a9c1bb30e9b0
      864eb2a6
    • Vladimir Slaykovskiy's avatar
      Reduce footprint of ScribeClient · 2adb6eb0
      Vladimir Slaykovskiy authored
      Summary:
      I've found all code paths that contributed to the number of threads in new ScribeClient. Here's a list of flags that reduce the number of thread:
        --default_tls_thread_count=1 # was 32
        --sr2_event_base_pool_size=1
        --has_thrift_dispatcher_reporter=0 # creates a thrift server with a few extra threads
        --observer_manager_pool_size=1 # default is 4
      
      Optimizations in this diff:
      - Don't initialize OBCClient object if it is not used
      - Added FLAG_observer_manager_pool_size to control size of the pool in ObserverManager
      
      Currently OBC counters are switched off globally in ScribeCliean, it means that new code path that creates SR instances is disabled, but eventually we plan to switch it on. Clients that don't use SR and want to keep their RSS/threads profile low, should pass flags listed above. I'll announce this in ScribeUsers group as well.
      
      Differential Revision: D3870704
      
      fbshipit-source-id: 0efad6b3dc43c072ab11cac7e9461c09532ea11c
      2adb6eb0
    • Eric Niebler's avatar
      work around LLVM#30305 in folly::Expected, use unified initialization syntax for extra goodness · a610249b
      Eric Niebler authored
      Summary: The behavior of inheriting constructors changed in clang 3.9 (see https://llvm.org/bugs/show_bug.cgi?id=30305), and folly::Expected needs to change accordingly. As a drive-by improvement, change all invocations of default constructors to use unified initialization syntax.
      
      Reviewed By: igorsugak
      
      Differential Revision: D3872994
      
      fbshipit-source-id: fdaea8b35980df21b8522e2c3d5a8c3be1d84efa
      a610249b
  2. 15 Sep, 2016 7 commits
    • Jim Meyering's avatar
      folly/.../ExceptionTracerLib.cpp: provide less DOF fodder (avoid heap use-after-free) · 101b2ad7
      Jim Meyering authored
      Summary:
      Before this change, an application would fail consistently with a shutdown-time heap use-after-free (i.e., destructor-order-fiasco when one of the following symbols was used after its static-destruction-triggered free):
      
        DECLARE_CALLBACK(CxaThrow);
        DECLARE_CALLBACK(CxaBeginCatch);
        DECLARE_CALLBACK(CxaRethrow);
        DECLARE_CALLBACK(CxaEndCatch);
        DECLARE_CALLBACK(RethrowException);
      
      Each of those would define a classic meyers singleton.
      Since each destructor is trivial, we can fix this by making each a pseudo-leaky (indestructible) meyers singleton instead. Since each static value is never destroyed,
      it can never cause a DOF error again.
      
      Differential Revision: D3870740
      
      fbshipit-source-id: 625f5d5268768ca0e4125bed72bc66c53618be29
      101b2ad7
    • Christopher Dykes's avatar
      Kill unneeded dependency on portability/Memory.h · 6848287d
      Christopher Dykes authored
      Summary: This dependency was added but isn't actually needed, so get rid of it.
      
      Reviewed By: mzlee
      
      Differential Revision: D3863923
      
      fbshipit-source-id: 407d2b3db759e30c37d876dde133fc962daeaeae
      6848287d
    • Neel Goyal's avatar
      Add flag to indicate a session resumption attempt · fd032019
      Neel Goyal authored
      Summary: Track when session resumption was attempted during connect
      
      Reviewed By: shamdor-fb
      
      Differential Revision: D3861028
      
      fbshipit-source-id: 26ca41084faeeb64666b6833896b9510ef2b4c25
      fd032019
    • Adam Simpkins's avatar
      a simple first step towards using clocks properly in the stats code · 8b030a19
      Adam Simpkins authored
      Summary:
      Update the timeseries and histogram classes to accept a clock parameter as a
      template parameter, instead of a time duration type.
      
      This is a first step towards transitioning the code to correctly distinguishing
      between time_point and duration types.  This defines TimePoint and Duration
      type aliases, but does not start using them yet.
      
      In upcoming diffs I will start converting more APIs to correctly use TimePoint
      instead of just Duration.
      
      For now the default clock type is folly::LegacyStatsClock, which still uses
      std::chrono::seconds as the default duration.  At the moment the stats code is
      optimized for second granularity--the addValue() code has a fast path when
      called in the same second as the last update.  When using finer granularity
      durations this fast path can't be used as often.  I will also send out
      subsequent diffs to make the code optimized for updates within the same bucket,
      rather than just updates with the exact same time value.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D3807715
      
      fbshipit-source-id: 77696c4f44a8d85e4d6ff84d7656fe7a9709797c
      8b030a19
    • Dominik Gabi's avatar
      ignore `$SHELL` in `shellify` · 9637519e
      Dominik Gabi authored
      Summary:
      Why `$SHELL` is a bad idea:
      
      - getenv() is not thread safe. (In practice it should work if other threads aren't calling setenv(), but still seems undesirable if we can avoid it.)
      - It seems confusing for the program to have different behavior for different developers.
      - Other shells besides /bin/sh may have different quoting behaviors. For instance, I don't think csh allows unescaped newlines inside single quotes.
      - SHELL might be set to other non-shell-like programs in some cases. (Say, if this gets run from inside a git or mercurial hook it might end up being set to the restricted git or hg shell that only lets you run specific commands.)
      Anyway, this isn't related to your diff, so nothing needs to be done for now,
      but I would vote for changing this to always use /bin/sh in a future diff.
      
      Both the C system() call and python's subprocess module appear to
      always use /bin/sh and ignore $SHELL.
      
      Reviewed By: simpkins
      
      Differential Revision: D3867047
      
      fbshipit-source-id: dab0e6afbe1c20ff13d9a52f212df95af425dd77
      9637519e
    • Dominik Gabi's avatar
      formatting support for `Subprocess::shellify` · e69e359c
      Dominik Gabi authored
      Summary:
      Adding formatting support with automatic escaping of parameters. Will
      subsequentyl try and make it harder for people to use non-statically determined
      format strings for this.
      
      Reviewed By: simpkins
      
      Differential Revision: D3866174
      
      fbshipit-source-id: 80bb5886386692643876116fbf338ca0d6986e6a
      e69e359c
    • Dominik Gabi's avatar
      deprecate `folly::Subprocess(std::string, ...)` · 8f27d856
      Dominik Gabi authored
      Summary:
      introducing `Subprocess::shellify` to get around this. Still thinking
      about how to make sure people escape arguments passing into that function. I'd
      like to have something along the lines of Phabricator's `csprintf` here.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D3857827
      
      fbshipit-source-id: 8afbc9f1c62c62e0fc91782e11b808145b370933
      8f27d856
  3. 14 Sep, 2016 3 commits
    • Mark Williams's avatar
      Make ElfCache follow .gnu_debuglink · 3272dfdb
      Mark Williams authored
      Summary:
      If you split out debug info into a separate file, folly::Symbolizer
      didn't find it, and failed to include file and line number info in
      backtraces.
      
      This adds a new open mode which follows the .gnu_debuginfo link, and
      uses it from ElfCache and SignalSafeElfCache.
      
      Reviewed By: meyering, luciang
      
      Differential Revision: D3852311
      
      fbshipit-source-id: fec9e57378ae59fa1b82d41a163bb9cfcf9ca23c
      3272dfdb
    • Christopher Dykes's avatar
      Swap a newly added include of gtest.h with portability/GTest.h · 9f838616
      Christopher Dykes authored
      Summary: Because it's needed with the way the portability layer works.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D3863954
      
      fbshipit-source-id: 3b682dcc2d2799f17c61fe16a4391fc9f0c2b127
      9f838616
    • Phil Willoughby's avatar
      Test for folly::SingletonThreadLocal · 953890fe
      Phil Willoughby authored
      Summary: Validates that we create a distinct singleton for each thread and that each such singleton is destroyed at its owning-thread's exit.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D3849146
      
      fbshipit-source-id: af878b32ecfc6c82b866d7a805e1385d74a8a5f5
      953890fe
  4. 13 Sep, 2016 5 commits
  5. 12 Sep, 2016 1 commit
    • Giuseppe Ottaviano's avatar
      Do not mess with NDEBUG in fbstring · fd01e2bf
      Giuseppe Ottaviano authored
      Summary: Temporarily overriding `NDEBUG` can have unexpected side-effects depending on the implementation of `<assert.h>`. Better use an explicit macro for `assert()` that we can explicitly disable.
      
      Reviewed By: Gownta
      
      Differential Revision: D3850717
      
      fbshipit-source-id: b1e7fce89ab4120b0224e9c6bf668d983b3d2d31
      fd01e2bf
  6. 10 Sep, 2016 3 commits
    • Christopher Dykes's avatar
      Use the GTest portability headers · 6a6ac91e
      Christopher Dykes authored
      Summary:
      Switch all of the uses of `<gtest/gtest.h>` to `<folly/portability/GTest.h>`.
      
      This is painful but necessary to get the tests to compile nicely under MSVC.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D3837300
      
      fbshipit-source-id: 7ae43a5d5ba94c8c24fa23a485f18546416e7191
      6a6ac91e
    • Christopher Dykes's avatar
      Use the GMock portability header · 90989a73
      Christopher Dykes authored
      Summary:
      Switch all of the uses of <gmock/gmock.h> to <folly/portability/GMock.h>.
      
      This is painful but necessary to get the tests to compile nicely under MSVC.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D3837529
      
      fbshipit-source-id: 7221dfea8f2a880919690b5b0601ca91642991ae
      90989a73
    • Christopher Dykes's avatar
      Add portability headers for GTest and GMock · af5dbcc3
      Christopher Dykes authored
      Summary:
      Both the gtest and gmock headers include `<io.h>` on Windows, which conflicts with how the portability headers include it, so a specific include order is required before either of those headers can be included. As that's brittle and messy, create a pair of portability headers for them instead, so that those can be included instead.
      
      This only adds the headers. The switch to using them will be done in a later pair of diffs.
      
      Reviewed By: mzlee
      
      Differential Revision: D3837221
      
      fbshipit-source-id: 82a273485cdd4536f1153d958c171bfe3ec32e0b
      af5dbcc3
  7. 09 Sep, 2016 5 commits
    • Eric Niebler's avatar
      Refactor basic_fbstring · dbd70eed
      Eric Niebler authored
      Summary: Move fbstring_core and basic_fbstring member functions from in-situ to out-of-class inlines and refactor for readability. Remove superfluous dependence on scope_guard.
      
      Reviewed By: ot, luciang
      
      Differential Revision: D3795250
      
      fbshipit-source-id: f6edca25d4771181faff9e0a4339bbaffd71a370
      dbd70eed
    • Yedidya Feldblum's avatar
      Nomenclature in Synchronized - prefer read to shared · 2e8a121a
      Yedidya Feldblum authored
      Summary:
      [Folly] Nomenclature in `Synchronized` - prefer `read` to `shared`.
      
      `folly::Synchronized` is a higher-level abstraction, so it will use higher-level nomenclature than will lower-level tools like `std::mutex` and `folly::LockTraits`. `shared` describes the lock state, and is used in the C++ libraries. `read` describes the class of operations that calling code is permitted to perform.
      
      Reviewed By: simpkins
      
      Differential Revision: D3840060
      
      fbshipit-source-id: 4b23eaa391cb59d1eca2bfacf72db89d3c7c591e
      2e8a121a
    • Daniel Sommermann's avatar
      Add forwarding gather() function to IOBufQueue · 2f028f63
      Daniel Sommermann authored
      Summary:
      I'm working with a parser that requires a certain number of
      contiguous bytes to be able to make forward progress. I'm also using
      IOBufQueue to receive data from an AsyncReader::ReadCallback with
      pre/postallocate. So, I need to call gather() to ensure that the queue's
      front IOBuf has the right number of contiguous bytes available.
      
      Reviewed By: djwatson
      
      Differential Revision: D3838079
      
      fbshipit-source-id: 9f1ec5c86895eb1b2b109f9f145ca42d2dbba4c6
      2f028f63
    • Anirudh Ramachandran's avatar
      Make folly::PasswordCollector::getPassword const · 0c723a60
      Anirudh Ramachandran authored
      Summary: As in title
      
      Reviewed By: siyengar
      
      Differential Revision: D3794648
      
      fbshipit-source-id: f0b7052f34ecce65cf4e21d546d08c7a6b0a8ee3
      0c723a60
    • Michael Lee's avatar
      #if FOLLY_HAVE_INT128_T rather than #ifdef · 6b3b5def
      Michael Lee authored
      Summary: Switch from #ifdef to #if so the configuration can define the macro to 0
      
      Reviewed By: yfeldblum, Orvid
      
      Differential Revision: D3838748
      
      fbshipit-source-id: e287b07f0fdfdc86c882538e96f2078795b85bfd
      6b3b5def
  8. 08 Sep, 2016 6 commits
    • Denis Samoylov's avatar
      move AsyncSSLSocket logging level for errors to vlog · f4962102
      Denis Samoylov authored
      Summary: current log entries ("E0906 AsyncSSLSocket.cpp:117] TCP connect failed: AsyncSocketException: connect failed, type = Socket not open, errno = 111 (Connection refused)") are not very helpful to debug due lack of details and can spam logs of application that uses async library without ability to disable logging
      
      Reviewed By: djwatson
      
      Differential Revision: D3825048
      
      fbshipit-source-id: 1c97f14e1ea3f1b276d04bb12483d42372a0d186
      f4962102
    • Maged Michael's avatar
      Dynamic expansion of folly MPMC queue · 59ea1768
      Maged Michael authored
      Summary:
      This diff allows queues to start with small capacity and expand as needed up to the specified capacity.
      The main additions and changes:
      - Extra template parameter `Dynamic` that enables dynamic expansion (`default 'false').
      - `ClosedArray` type.
      - Extra members:
        -- `dstate_`: a packed 64 bit unsigned int that contains a seqlock (which implicitly indicates the number of expansions and the lowest ticket for the current `dslots_/dcapacity_/dstride_` configuration.
       -- `dcapacity_`: current dynamic capacity.
       -- `dslots_`: current dynamic slots array. (in anonymous union with `slots_`)
       -- `dstride_`: current dynamic stride. (in anonymous union with `stride_`)
       -- `closed_` a logarithmic-sized array of ClosedArray to hold information about earlier smaller queue arrays for use by lagging consumers.
      
      Design sketch:
      - Reallocate a new larger array on expansion
      - Expansion uses a seqlock. The common case critical path includes a seqlock read-only section.
      - Lagging consumers and lagging blocking producers use a logarithmic-sized array for info about closed arrays
      - Tickets are adjusted by an offset (to accounts for the tickets associated with earlier closed arrays) in order to calculate appropriate index and turn.
      - The synching of `pushTicket_` with the ticket offset packed in `dstate_` is tricky. `pushTicket_` is accessed outside `dstate_`'s seqlock.
      
      Reviewed By: djwatson
      
      Differential Revision: D3462592
      
      fbshipit-source-id: d442a7694190cca3c33753409ffac941d7463f83
      59ea1768
    • Andrii Grynenko's avatar
      Remove runAfterDrain() · 1cd90b1d
      Andrii Grynenko authored
      Reviewed By: djwatson
      
      Differential Revision: D3832835
      
      fbshipit-source-id: b5cabc00758fd80b314424e4224458e2d50ddb5c
      1cd90b1d
    • Zonr Chang's avatar
      Explicitly include <random> to use std::mt19937 in MathBenchmark. · 83b4f372
      Zonr Chang authored
      Summary:
      This fixes build with Clang.
      Closes https://github.com/facebook/folly/pull/470
      
      Differential Revision: D3834865
      
      Pulled By: nbronson
      
      fbshipit-source-id: 02c1ca192c6b6af2cc0a8fdaa0a854510cb0bca4
      83b4f372
    • Tom Jackson's avatar
      prvalues from get_ref_default()'s default functor · cf784212
      Tom Jackson authored
      Summary: This previously allowed `get_ref_default(map, 4, []{ return 6; })`, even though this would form a reference to a temporary, then **use that invalid reference**.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D3802707
      
      fbshipit-source-id: 384d965f69c9d7b6bd3f011c8eff7fe55be7023a
      cf784212
    • Christopher Dykes's avatar
      Move the alignment attribute in TokenBucket to a place where MSVC supports it · dd6fff71
      Christopher Dykes authored
      Summary: MSVC doesn't support the aligment attribute being after the field name.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D3832314
      
      fbshipit-source-id: 6a56db245e01922ede6b9b93eb1c8aecc835e2bf
      dd6fff71
  9. 07 Sep, 2016 2 commits
    • Nathan Bronson's avatar
      integer division with controlled rounding in Math.h · 1d9d5cbe
      Nathan Bronson authored
      Summary:
      C++'s integer division performs truncation, but it is fairly
      common that people actually want to round up.  There are actually
      four rounding modes that make some sense: toward negative infinity
      (floor), toward positive infinity (ceil), toward zero (truncation),
      and away from zero (?).  It is pretty common that code that wants ceil
      actually writes (a + b - 1) / b, which doesn't work at all for negative
      values (and has a potential overflow issue).  This diff adds 4 templated
      functions for performing integer division: divFloor, divCeil, divTrunc,
      and divRoundAway.  They are not subject to unnecessary internal underflow
      or overflow, and they work correctly across their entire input domain.
      
      I did a bit of benchmarking across x86_64, arm64, and 32-bit ARM.
      Only 32-bit ARM was different.  That's not surprising since it doesn't
      have an integer division instruction, and the function that implements
      integer division doesn't produce the remainder for free.  On 32-bit ARM
      a branchful version that doesn't need the modulus is used.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D3806743
      
      fbshipit-source-id: c14c56717e96f135321920e64acbfe9dcb1fe039
      1d9d5cbe
    • Maged Michael's avatar
      Expand DSched interface for managing auxiliary functions · f6b5f78b
      Maged Michael authored
      Summary:
      Changed DSched interface for managing auxiliary functions to allow separate auxiliary functions for single actions (applicable to the next shared access by a specific thread) and repeating actions (applicable to all subsequent shared accesses).
      
      [Note: I have a dependent diff that depends on both this diff and the diff for dynamic MPMCQueue (/D3462592). I don't think I can submit a diff that depends on multiple diffs that haven't landed yet. So, I'll wait until this one lands.]
      
      Reviewed By: djwatson
      
      Differential Revision: D3792669
      
      fbshipit-source-id: 52654fffda2dc905b19ff91f4459f15da11f7735
      f6b5f78b
  10. 06 Sep, 2016 3 commits
    • Christopher Dykes's avatar
      Rework the de-allocation guard on the munmap implementation · 18914ab5
      Christopher Dykes authored
      Summary:
      The previous version assumed that `RegionSize` would always be the full size of the allocation done by `VirtualAlloc`. However, `RegionSize` actually only includes the following pages that have the same attributes, so, if you change the access permissions via `mprotect`, the `RegionSize` would exclude that region, which is not what was intended.
      This instead stores the length and a dummy magic value after the end of the requested allocation.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D3812949
      
      fbshipit-source-id: 53bbbcc371accbed08adaffa82fc082ec44f316f
      18914ab5
    • Chip Turner's avatar
      Eliminate a string allocation in folly::SocketAddr and bug in char[] conversion · e64edf80
      Chip Turner authored
      Summary:
      We were doing an unnecessary and wasteful string conversion in the
      SocketAddr codepath.  This eliminates it.  I also noticed we had an off-by-one
      in the "convert string to char buffer" code path, so I added a test to confirm
      the bug and fixed it.
      
      Reviewed By: yfeldblum, meyering
      
      Differential Revision: D3817959
      
      fbshipit-source-id: 51fed8331ab23c0888a3d1f9e0cc9cea5ea8329b
      e64edf80
    • Christopher Dykes's avatar
      Implement more of the sockets API · bf77be6e
      Christopher Dykes authored
      Summary:
      This gets the socket portability layer functional enough that most of the socket tests are passing, the few that are left are depending on specific error messages.
      This also switches all of the additional overloads for some of the socket functions to forward to a single implementation, to make adjustments to these functions easier in the future. (most of the functions already needed adjustments and would have had to change regardless)
      
      Reviewed By: yfeldblum
      
      Differential Revision: D3814011
      
      fbshipit-source-id: c6793ee74a91d9e164775a2d52c96f54b28b9f24
      bf77be6e