1. 20 Dec, 2013 24 commits
    • Tudor Bosman's avatar
      Switch to folly symbolizer · 5357f0f4
      Tudor Bosman authored
      Summary:
      (not committing yet, but I want to trigger unittests)
      
      The glog symbolizer that we use has a few bugs (abort()s on certain small
      shared libraries) and doesn't allow us to distinguish between template
      specializations and function overloads (which, given that our code is more
      template-heavy than Google's, has in fact been an issue).
      
      Luckily, we have our own in folly, which doesn't have these problems and also
      supports mappings from address to file and line number.
      
      Switch translateFrames (aka the fb303 call that cpprof uses) to our symbolizer.
      Also, removed a lot of dead code in common/process.
      
      Test Plan: common/process, tested cpprof by hand
      
      Reviewed By: lucian@fb.com
      
      FB internal diff: D1090907
      
      @override-unit-failures
      5357f0f4
    • Andrei Alexandrescu's avatar
      SCOPE_SUCCESS should be able to throw, the others shouldn't · 51fea298
      Andrei Alexandrescu authored
      Summary: SCOPE_SUCCESS may legitimately throw, so changed the `noexcept` attribute to a conditional one. Also added the noexcept promise to the lambdas in the hope the compiler will check code during compilation appropriately. (It currently doesn't.)
      
      Test Plan: Added a unittest that failed, not passes.
      
      Reviewed By: simpkins@fb.com
      
      FB internal diff: D1093328
      
      @override-unit-failures
      51fea298
    • Nathan Bronson's avatar
      centralize cache-line alignment goo into CacheLocality · 9b9fb1d6
      Nathan Bronson authored
      Summary:
      The alignment requirements and details required to avoid false
      sharing belong in CacheLocality, so this diff moves them there.
      
      @override-unit-failures
      
      Test Plan: fbmake runtests
      
      Reviewed By: davejwatson@fb.com
      
      FB internal diff: D1086090
      9b9fb1d6
    • Nicholas Ormrod's avatar
      FBString iomanip fix. · 7c3790ca
      Nicholas Ormrod authored
      Summary:
      D1090936 noticed some problems with fbstring iomanip behavior.
      The root cause is that os.write(ostream, char*, size_t) is an
      UnformattedOutputFunction, so disregards setw(), setfill(), and
      left/right alignment.
      
      The use of os.write instead of os << str.data() is intentional:
      D367009 switched from the latter to the former so that strings
      containing a '\0' are printed properly.
      
      There does not seem to be a public function to write with formatting.
      Where needed in libgcc, the function __ostream_insert is used. Since
      FBString already uses such 'private' functions, __ostream_insert is an
      appropriate solution.
      
      @override-unit-failures
      
      Test Plan:
      Added test cases to FBStringTest.cpp to cover iomanip.
      fbconfig -r folly && fbmake opt && fbmake runtests_opt
      
      Reviewed By: andrei.alexandrescu@fb.com
      
      FB internal diff: D1091474
      7c3790ca
    • Tudor Bosman's avatar
      Use fixed size stack traces; unify getStackTrace · 3680f888
      Tudor Bosman authored
      Summary:
      Also, switch to the simpler unw_backtrace(), which has the nice advantage of
      actually doing IP adjustment (-1 in certain frames) correctly, unlike me :)
      
      This is in preparation for the faster backtrace in libunwind 1.1.
      
      Test Plan: folly/experimental/exception_tracer, folly/experimental/symbolizer, admarket/lib/util:memory_tracker_test
      
      Reviewed By: lucian@fb.com
      
      FB internal diff: D1088357
      3680f888
    • Nathan Bronson's avatar
      detail/CacheLocality.h - utilities for dynamic cache optimizations · 3d0c8f28
      Nathan Bronson authored
      Summary:
      CacheLocality reads cache sharing information from sysfs to
      determine how CPUs should be grouped to minimize contention, Getcpu
      provides fast access to the current CPU via __vdso_getcpu, and
      AccessSpreader uses these two to optimally spread accesses among a
      predetermined number of stripes.
      
      AccessSpreader<>::current(n) microbenchmarks at 22 nanos, which is
      substantially less than the cost of a cache miss.  This means that we
      can effectively use it to reduce cache line ping-pong on striped data
      structures such as IndexedMemPool or statistics counters.
      
      Because CacheLocality looks at all of the cache levels, it can be used for
      different levels of optimization.  AccessSpreader<>::stripeByChip.current()
      uses as few stripes as possible to avoid off-chip communication,
      AccessSpreader<>::stripeByCore.current() uses as many stripes as necessary
      to get the optimal speedup, but no more.
      
      @override-unit-failures
      
      Test Plan: new unit tests
      
      Reviewed By: davejwatson@fb.com
      
      FB internal diff: D1076718
      3d0c8f28
    • Tudor Bosman's avatar
      Revert "Change exception tracer to use per-thread caching in libunwind" · 3952281c
      Tudor Bosman authored
      Summary:
      Per-thread caching calls malloc, which means it doesn't work for malloc's
      profiling (and also it isn't async-signal-safe, despite the documentation).
      
      Test Plan: compiled
      
      Reviewed By: philipp@fb.com
      
      FB internal diff: D1085368
      
      @override-unit-failures
      3952281c
    • Tudor Bosman's avatar
      Print (2 more) if stack trace truncated · 730a0da8
      Tudor Bosman authored
      Summary:
      Also, C++ify the interface and switch to per-thread caching in libunwind as per
      D1081259
      
      Test Plan: folly/experimental/symbolizer/test
      
      Reviewed By: lucian@fb.com
      
      FB internal diff: D1081272
      730a0da8
    • bsimmers's avatar
      Fix asm constraints in folly::MicroSpinLock::cas · db56587b
      bsimmers authored
      Summary:
      If the compare part of cmpxchg fails, it writes the unexpected value
      to %rax. At certain optimization levels this was causing me to hit an
      incorrectly failed assertion in some thrift code. I also cleaned up the asm
      statement to use named operands.
      
      Test Plan: Added new test that fails before this diff.
      
      Reviewed By: delong.j@fb.com
      
      FB internal diff: D1083222
      @override-unit-failures
      db56587b
    • Adam Simpkins's avatar
      make IOBuf::gather() safe · ab4ee5fa
      Adam Simpkins authored
      Summary:
      Update IOBuf::gather() and RWCursor::gather() to check their arguments
      more carefully, and throw std::overflow_errors if the caller attempts to
      gather more data than is available.
      
      The comments for IOBuf::gather() claimed that it ensured that maxLength
      bytes would always be available when it returned.  However, if the total
      chain length was less than maxLength, it would simply coalesce the full
      chain and return successfully, even though fewer than maxLength bytes
      were available.  This fixes the code to throw a std::overflow_error
      rather than coalescing the chain.
      
      Additional, this updates RWCursor::gather() to ensure that it does not
      attempt to gather past the end of the IOBuf chain.  Previously it could
      gather past the end of the chain, coalescing the head of the chain into
      the tail.  This would free the IOBuf head, which was owned by an
      external caller.
      
      A new RWCursor::gatherAtMost() API is provided for callers that wish to
      gather as much as requested if possible, but still succeed if less than
      this is available.
      
      Test Plan:
      Updated the unit tests to test calling gather() with more bytes than
      actually available.
      
      Reviewed By: davejwatson@fb.com
      
      FB internal diff: D1081995
      ab4ee5fa
    • Adam Simpkins's avatar
      fix issues when compiling with clang · 29eecd37
      Adam Simpkins authored
      Summary:
      My previous change to re-implement IOBuf's internal storage mechanism
      introduced a build failure when compiling with clang.
      
      This fixes the new compilation error in IOBuf.cpp, as well as two
      existing build failures in some of the unit tests.
      
      Test Plan: Built the folly/io code with clang.
      
      Reviewed By: andrewjcg@fb.com
      
      FB internal diff: D1082086
      29eecd37
    • Adam Simpkins's avatar
      change the mechanism for "internal" buffer storage · cb527272
      Adam Simpkins authored
      Summary:
      This removes kFlagExt, and instead implements the internal buffer
      optimization using operator new and delete.
      
      IOBuf::createCombined() will allocate a single buffer for the IOBuf,
      SharedInfo, and the actual data itself.  Each heap allocated IOBuf now
      contains a set of flags indicating if the memory segment indicating when
      it can actually be deleted.  The delete operator won't actually free the
      storage until both the IOBuf and the data buffer are unused (the
      SharedInfo object always becomes unused at the same time as the data
      buffer).
      
      This has a couple advantages over the old mechanism:
      
      - Other IOBufs can continue to use and share the data storage space even
      after the original IOBuf associated with it is deleted.  With the old
      internal buffer mechanism, internal buffers could never be shared.
      
      - This simplifies several parts of the code, since kFlagExt no longer
      exists.  Code that previously required special handling for internal
      buffers no longer needs to be aware of them.
      
      One downside is that this can result in wasted space in some cases if
      the original IOBuf is changed to point at a different buffer, since the
      space for the data buffer cannot be freed until the IOBuf itself is also
      destroyed.  The old internal buffer mechanism also had this problem,
      which we mitigated simply by disallowing internal buffers for larger
      than ~200 bytes.  With the new mechanism we currently allocate an
      internal buffer for up to 1024 bytes by default, but we also allow
      callers to explicitly decide if they want an internal buffer or not.
      
      Test Plan:
      Added some new unit tests for the combined buffer behavior.  Also ran
      all of the existing IOBuf unit tests, with and without ASAN.  (ASAN
      performs additional memory checking, but also changes IOBuf's behavior
      slightly as usingJEMalloc() is false with ASAN.)
      
      Reviewed By: davejwatson@fb.com
      
      FB internal diff: D1072336
      
      @override-unit-failures
      cb527272
    • Tudor Bosman's avatar
      Change exception tracer to use per-thread caching in libunwind · ec61097a
      Tudor Bosman authored
      Summary: Because the global cache is slow and contends on locks.
      
      Test Plan: testinproduction
      
      Reviewed By: philipp@fb.com
      
      FB internal diff: D1081259
      ec61097a
    • Soren Lassen's avatar
      Made File::release() return the released file descriptor. · cb394cb5
      Soren Lassen authored
      Summary: It's convenient to get back the fd, similar to unique_ptr::release().
      
      Test Plan: unittest
      
      Reviewed By: tudorb@fb.com
      
      FB internal diff: D1080426
      cb394cb5
    • Tudor Bosman's avatar
      Async-signal-safe symbolizer, fatal signal handler · fd3c895f
      Tudor Bosman authored
      Test Plan: test added
      
      Reviewed By: lucian@fb.com
      
      FB internal diff: D1076170
      
      @override-unit-failures
      fd3c895f
    • Tudor Bosman's avatar
      Add async-signal-safe flavor of demangle · 35dc92a7
      Tudor Bosman authored
      Summary: To be used in the new fatal signal handler.
      
      Test Plan: test added
      
      Reviewed By: lucian@fb.com
      
      FB internal diff: D1076169
      
      @override-unit-failures
      35dc92a7
    • Tudor Bosman's avatar
      SafeAssert: async-signal-safe CHECK, DCHECK · 25b8374a
      Tudor Bosman authored
      Summary: To be used from the (new) fatal signal handler.
      
      Test Plan: test added
      
      Reviewed By: lucian@fb.com
      
      FB internal diff: D1076168
      
      @override-unit-failures
      25b8374a
    • Adam Simpkins's avatar
      update moveToFbString()'s handling of flags_ · d9298481
      Adam Simpkins authored
      Summary:
      Improve moveToFbString()'s code which determines if we actually have a
      buffer that was allocated with malloc().
      
      The old code simply checked if flags_ == kFlagExt.  This check is rather
      fragile, and relies on very specific behavior from the existing
      construction methods.  It also unnecessarily forced reallocation in some
      cases.
      
      This updates the code to have more specific checks for the flags and
      fields that actually matter.  In particular, even if we have an external
      buffer, if sharedInfo->freeFn is set then it is not managed by malloc().
      The old check happened to work because takeOwnership() was the only
      function that set set freeFn, and it also set kFlagFreeSharedInfo.
      
      This also improves the code so that it won't unnecessarily reallocate
      the buffer if kFlagMaybeShared is set but the buffer isn't really
      shared.  The code will also no longer unnecessarily reallocates the
      buffer just because kFlagFreeSharedInfo is set.
      
      Test Plan:
      Updated the moveToFbString() test to also test with buffers created
      using takeOwnership() and wrapBuffer().
      
      Reviewed By: davejwatson@fb.com
      
      FB internal diff: D1072304
      
      @override-unit-failures
      d9298481
    • Adam Simpkins's avatar
      fix IOBuf::reserve() when operating on a user-supplied buffer · 3d5106cd
      Adam Simpkins authored
      Summary:
      The IOBuf::reserveSlow() code assumed that external buffers were always
      allocated with malloc.  This would cause problems when if reserve() was
      ever used on a buffer created with IOBuf::takeOwnership().
      
      This changes the code to now check if a user-specified free function has
      been supplied.  If so, then it does not try to use realloc()/rallocm(),
      and it now frees the old buffer using the specified free function rather
      than free().
      
      User-supplied buffers also have a separately allocated SharedInfo
      object, which must be freed when we no longer need it.
      
      Test Plan:
      Added additional unit tests to check calling reserve() on IOBufs created
      with IOBuf::takeOwnership().
      
      Reviewed By: davejwatson@fb.com
      
      FB internal diff: D1072292
      3d5106cd
    • Anton Likhtarov's avatar
      sort_keys option for json · bfa6ffb5
      Anton Likhtarov authored
      Summary:
      A dumb&slow implementation. I just needed something that outputs keys in a sorted order.
      No easy to use API to discourage the use due to perf implications.
      
      Test Plan: unit test
      
      Reviewed By: delong.j@fb.com
      
      FB internal diff: D1073757
      bfa6ffb5
    • Brian Watling's avatar
      Trivial argument rename in MPMCQueue · 048a4518
      Brian Watling authored
      Summary: Rename the 'capacity' argument so g++ will not complain when -Wshadow is enabled ('capacity' is also a method on MPMCQueue)
      
      Test Plan: compiles, unit tests pass
      
      Reviewed By: ngbronson@fb.com
      
      FB internal diff: D1076220
      048a4518
    • Soren Lassen's avatar
      Make FOR_EACH* work with temporary containers. · d64c6e0d
      Soren Lassen authored
      Test Plan: unittest
      
      Reviewed By: tudorb@fb.com
      
      FB internal diff: D1071715
      d64c6e0d
    • Adam Simpkins's avatar
      fix bugs in the cursor StringOperations test · efc30713
      Adam Simpkins authored
      Summary:
      The IOBuf.StringOperations test contained two buggy tests.  According to
      the comments, they were intended to test a string spanning multiple
      buffers.  They created a chain of two buffers, but used Appender to
      write the string, which only writes in the final buffer in the chain.
      
      Additionally, the test didn't request enough space in the final buffer
      for the string data, and didn't allow Appender to grow the buffer.  (The
      code also didn't check the return value from pushAtMost() to see if it
      actually wrote all of the string data.)  The test only passed because
      IOBuf would normally allocate more capacity than the caller requested.
      
      I fixed the tests to request sufficient capacity for the string data,
      and use push() rather than pushAtMost() to ensure that all data was
      written correctly.  I also added two new tests which actually test with
      strings spanning multiple buffers.
      
      Test Plan:
      Ran this test with some changes to IOBuf that cause it to only allocate
      the requested capacity in some cases, and verified that the
      StringOperations test pass now.
      
      (The changes I was testing with use goodMallocSize() to determine the
      allocation size even for small internal buffers.  When not using
      jemalloc, goodMallocSize() returns the original size, so it only
      allocates the capacity requested by the caller.)
      
      Reviewed By: pgriess@fb.com
      
      FB internal diff: D1072283
      efc30713
    • Peter Griess's avatar
      Revert "Handle lack of <bits/c++config.h> and <bits/functexcept.h>" · f5cac66e
      Peter Griess authored
      Summary:
      - This diff introduces errors when compiling with clang in fbcode.
      Revert until they get fixed.
      
      Test Plan: .
      
      Reviewed By: tconerly@fb.com
      
      FB internal diff: D1074481
      
      Blame Revision: D998595
      f5cac66e
  2. 26 Nov, 2013 15 commits
    • Peter Griess's avatar
      Get Subprocess running for Mac OS X · 54e29e71
      Peter Griess authored
      Summary:
      - D1030008 added Subprocess to libfolly in automake builds. This
      surfaced some ambient compilation errors that slipped through in my
      prior run through porting this.
      - Mac OS X uses a nonstandard location for wait.h
      - Non-Linux platforms don't support prctl; gate that on Linux
      
      Test Plan:
      - fbmake runtests in fbcode
      - make check on Mac OS X
      
      Reviewed By: davejwatson@fb.com
      
      FB internal diff: D1066273
      
      Blame Revision: D1030008
      54e29e71
    • Peter Griess's avatar
      Revert D998590 · c807fe1f
      Peter Griess authored
      Summary:
      - It looks like Xcode 5.0.2 / clang 500.2.79 changed this behavior. The
      prior behavior was seen against Xcode 5.0.1 / clang 500.2.75. Blerg.
      If we really have to support different Xcode flavors we can come up
      with a real solution for this, but for now just take advantage of the
      fact that clang and gcc seem to agree on this.
      
      Test Plan: - make check on Mac OS X
      
      Reviewed By: andrei.alexandrescu@fb.com
      
      FB internal diff: D1066272
      
      Blame Revision: D998590
      c807fe1f
    • Peter Griess's avatar
      Remove generated .cpp files on 'make clean' · 571c2047
      Peter Griess authored
      Summary:
      - Add generated .cpp files to CLEANFILES so that they get swept up
      during 'make clean' and friends
      
      Test Plan: - autoreconf -i && ./configure && make check && make clean
      
      Reviewed By: davejwatson@fb.com
      
      FB internal diff: D1066270
      571c2047
    • Peter Griess's avatar
      Fix broken clause11_21_4_6_6 test in Apple Clang. · 1c3d7f2a
      Peter Griess authored
      Summary:
      - Prior to this fix, the test relied upon begin() being evaluated after
      the fbstring constructor (even thoug the order of evaluation of
      function arguments is un-defined). When this assumption was violated,
      begin() ended up with an iterator that was invalid since it points to
      data internal to fbstring, and the fbstring copy constructor can end
      up triggering re-allocation in the source.
      
      Test Plan:
      - fbconfig -r folly && fbmake runtests
      - ./configure && make check on Ubuntu/FC/Mac
      
      Reviewed By: andrei.alexandrescu@fb.com
      
      FB internal diff: D1014093
      1c3d7f2a
    • Peter Griess's avatar
      Handle platforms that don't support __thread. · ea10c904
      Peter Griess authored
      Summary:
      - Apple platforms either have different __thread behavior than Linux: on
      i386 __thread values are zeroed out before the destructor provided to
      pthread_key_create(3) is called; on ARM, __thread isn't supported at
      all. To handle this, use pthread_getspecific(3) to manage the array of
      IDs on these platforms only.
      
      Test Plan:
      - fbconfig -r folly && fbmake runtests
      - ./configure && make check on Ubuntu/FC/Mac
      
      Reviewed By: simpkins@fb.com
      
      FB internal diff: D1008923
      ea10c904
    • Peter Griess's avatar
      Handle lack of weak symbols on some platforms · a0fe3c18
      Peter Griess authored
      Summary:
      - It turns out that it's not always desirable to perform runtime
      resolution of weak symbols. For example, on iOS, weak symbols are
      resolved at runtime only if *all* symbol resolution is deferred util
      then, which is undesirable for othe reasons. Detect such platforms at
      configure time and use that information to populate detail/Malloc.h
      with the correct declarations: weak symbols or extern symbols with
      a value of nullptr.
      
      Test Plan:
      - fbconfig -r folly && fbmake runtests
      - ./configure && make check on Ubuntu/FC/Mac
      
      Reviewed By: andrei.alexandrescu@fb.com
      
      FB internal diff: D1002959
      a0fe3c18
    • Peter Griess's avatar
      Handle lack of <bits/c++config.h> and <bits/functexcept.h> · 758e4c77
      Peter Griess authored
      Summary:
      - Clang's libc++ doesn't provide these header files. Detect libc++ via
      the _LIBCPP_VERSION symbol (pulling it in by sourcing some header
      files earlier if necessary) and avoid using these files.
      
      Test Plan:
      - fbconfig -r folly && fbmake runtests
      - ./configure && make check on Ubuntu/FC/Mac
      
      Reviewed By: andrewjcg@fb.com
      
      FB internal diff: D998595
      758e4c77
    • Ben Maurer's avatar
      Avoid copy in folly::toJson · 23123a01
      Ben Maurer authored
      Summary: Adding const avoids a copy constuctor.
      
      Test Plan:
      Unit tests, new benchmark:
      toJson        1.83us  546.15K
      toJson        1.54us  649.98K
      
      Reviewed By: tudorb@fb.com
      
      FB internal diff: D1071781
      23123a01
    • Stephen Chen's avatar
      fix broken clang build: add missing template keyword · bc4b701a
      Stephen Chen authored
      Summary: Broken in D1054291
      
      Test Plan: build adpublisher in clang mode which was one of the two projects that failed to build.
      
      Reviewed By: bmaurer@fb.com
      
      FB internal diff: D1067652
      
      @override-unit-failures
      bc4b701a
    • Stephen Chen's avatar
      Port TimeseriesHistogram from common/stats to folly · 586dd85c
      Stephen Chen authored
      Summary:
      Port TimeseriesHistogram from common/stats to folly. Similarly to
      MultiLevelTimeSeries and Histogram classes we've converted before.
      
      Test Plan:
      Ported the old unittest for TimeseriesHistogram from common/stats to
      folly/test. The same unittest is left in tact and is still passing as well. Ran
      all tests in folly/test and common/stats
      
      Reviewed By: simpkins@fb.com
      
      FB internal diff: D1054291
      586dd85c
    • Dave Watson's avatar
      Add generated files to Makefile.am · 2d980c3d
      Dave Watson authored
      Summary: Missed these on the first pass because they're generated
      
      Test Plan: Built open source folly, links
      
      Reviewed By: pgriess@fb.com
      
      FB internal diff: D1062451
      2d980c3d
    • Tudor Bosman's avatar
      Improve CompressionTest · cda4ae2a
      Tudor Bosman authored
      Summary: Test compressing low-entropy (constant) data as well.
      
      Test Plan: ran it
      
      Reviewed By: tuomas.pelkonen@fb.com
      
      FB internal diff: D1061444
      
      @override-unit-failures
      cda4ae2a
    • Tudor Bosman's avatar
      Fix bad implementation of fnv32 · 11b4d6ba
      Tudor Bosman authored
      Summary: See https://www.facebook.com/groups/fbcode/permalink/601496126553897/
      
      Test Plan: contbuild
      
      Reviewed By: ldbrandy@fb.com
      
      FB internal diff: D1055852
      
      @override-unit-failures
      some hphp_dbg tests postponed for 1.5 days, all other hphp tests
      (including some hphp_dbg ones) are passing
      11b4d6ba
    • Tom Jackson's avatar
      Move byLine to FileGen.h, add explanatory note · 48bc9653
      Tom Jackson authored
      Summary: There's a bit of a gotcha here, let's provide at least a note on this.
      
      Test Plan: Unit tests
      
      Reviewed By: tudorb@fb.com
      
      FB internal diff: D1058244
      48bc9653
    • Daniel Marinescu's avatar
      Added SCOPE_FAIL and SCOPE_SUCCESS macros in non-portable C++. · a57c72d7
      Daniel Marinescu authored
      Summary:
      Added SCOPE_FAIL and SCOPE_SUCCESS macros in non-portable C++. The macros are similar to D's scope(failure) and scope(success).
      Currently the supported platforms are GCC and MSVC. For all others, std::uncaught_exception() is used, which will fail if the macros are used in a destructor called during stack unwinding.
      @override-unit-failures
      
      Test Plan:
      1. Added new unit test to ScopeGuardTest.cpp.
      2. Ran fbconfig -r folly && fbmake dbg
      3. Ran _build/dbg/folly/test/scope_guard_test to make sure my unit test was running and passing.
      
      Reviewed By: andrei.alexandrescu@fb.com
      
      FB internal diff: D1033621
      a57c72d7
  3. 13 Nov, 2013 1 commit
    • Yasser Ganjisaffar's avatar
      Add missing files to Makefile headers · 4962ea0c
      Yasser Ganjisaffar authored
      Summary:
      This two header files are missing from the Makefile which make the
      open source version broken.
      
      Test Plan: code using open source version compiles
      
      Reviewed By: meyering@fb.com
      
      FB internal diff: D1050326
      4962ea0c