1. 21 Jun, 2017 4 commits
    • Adam Simpkins's avatar
      logging: reduce the amount of code emitted for log statements · fc91e303
      Adam Simpkins authored
      Summary:
      This refactors the logging code with the aim of reducing the amount of assembly
      code emitted for each log statement, particularly for `XLOG()` statements.
      Ideally it should be possible to put debug `XLOG()` statements throughout your
      code without having to worry about the performance overhead.  Therefore we
      should attempt to make sure that `XLOG()` statements do not generate a lot of
      assembly and hurt icache performance.
      
      This diff does not have any code behavior changes--it just moves code around a
      bit.  The high-level summary is:
      
      - Move as much code as possible into the LogStreamProcessor constructors and
        destructor.  Make sure these methods are defined in LogStreamProcessor.cpp to
        avoid having them be emitted inline at each log statement.
      - Move some of the XLOG level checking logic into separate non-inline functions
        defined in xlog.cpp
      - Pass xlog category information around as a pair of (categoryName,
        isOverridden) parameters.  If isOverridden is true then the categoryName
        parameter should be used as the category name directly.  If isOverridden is
        false, then categoryName is a filename that needs to go through filename to
        category name string processing.  This allows the category name processing to
        be done in non-inlined code.
      
      Reviewed By: wez
      
      Differential Revision: D5269976
      
      fbshipit-source-id: 7a7877ddfed66cd27ed82f052330b6aa2be4b37b
      fc91e303
    • Adam Simpkins's avatar
      logging: add printf-style logging macros · cf984921
      Adam Simpkins authored
      Summary:
      Add new `FB_LOGC()` and `XLOGC()` macros that accept C-style printf format
      strings.  (The `FB_LOGF()` and `XLOGF()` macro names are already used for
      `folly::format()` style formatting.)
      
      This will make it easier for users to update existing printf-style code to use
      this new logging library.
      
      These are in a separate `printf.h` header file that must be explicitly included
      to have access to these macros.  The intent is to encourage users to use one of
      the other APIs (streaming, append-style, or `folly::format()`) instead of these
      printf-like APIs in new code.
      
      Reviewed By: omry
      
      Differential Revision: D5269974
      
      fbshipit-source-id: 56e55f9642bb00806d9b4c762fb6a91778ef6ad3
      cf984921
    • Adam Simpkins's avatar
      logging: make XLOG_GET_CATEGORY() safe for all callers · e38fcb3a
      Adam Simpkins authored
      Summary:
      The `XLOG_GET_CATEGORY()` macro was previously written assuming it was only
      used inside `XLOG()` statement.  When used inside the main translation unit
      being compiled (e.g., a .cpp file and not a header file), the code assumed that
      the file-scope category had already been initialized, since a level check had
      presumably already been performed.
      
      However, it is useful in some places for external users (outside of the logging
      library itself) to call `XLOG_GET_CATEGORY()`.  In these cases a log level
      check may not have been performed yet, so the file-scope category may not be
      initialized yet.
      
      This diff renames the existing `XLOG_GET_CATEGORY()` macro to
      `XLOG_GET_CATEGORY_INTERNAL()` and adds a new `XLOG_GET_CATEGORY()` macro that
      is slower (it always looks up the category by name) but always safe to use.
      
      This also adds a new `XLOG_GET_CATEGORY_NAME()` macro, and renames the existing
      `XLOG_SET_CATEGORY()` macro to `XLOG_SET_CATEGORY_NAME()` for API consistency.
      
      Reviewed By: wez
      
      Differential Revision: D5269975
      
      fbshipit-source-id: 373805255823855282fa7a8d4a4d232ba06367f6
      e38fcb3a
    • Adam Simpkins's avatar
      logging: improve the AsyncFileWriterTest discard test · 440cddaf
      Adam Simpkins authored
      Summary:
      This improves the test that exercises the AsyncFileWriter message discarding
      logic.
      
      Previously each writer thread wrote a fixed number of small messages.  This was
      fairly slow, and wasn't always guaranteed to reliably produce discards.
      Now each writer thread writes larger messages, which produce discards faster.
      The test also automatically stops after 10 separate discard events, so that it
      finishes faster (typically a few hundred milliseconds, rather than 5+ seconds).
      
      This also updates the test to use XLOG() internally rather than using fprintf()
      to print to stderr, now that the XLOG() diffs have landed.
      
      Reviewed By: wez
      
      Differential Revision: D5261059
      
      fbshipit-source-id: 120224706fee36948ef76efbeb579ccc56400c51
      440cddaf
  2. 20 Jun, 2017 11 commits
    • Adam Simpkins's avatar
      logging: implement FATAL and DFATAL log levels · 61fbd669
      Adam Simpkins authored
      Summary:
      Add new `FATAL` and `DFATAL` log levels.
      
      Any log message with a level of `FATAL` always crashes the program.  Log
      messages with a level of `DFATAL` crash the program in debug build modes.
      
      Before crashing, the code makes sure to flush all LogHandlers, so that the
      fatal message is not lost if some of the LogHandlers process messages
      asynchronously.  If no LogHandlers were configured, the message is printed to
      stderr.
      
      Reviewed By: wez
      
      Differential Revision: D5189497
      
      fbshipit-source-id: c45dbd582fb1c3a962d00effb2967737ef97cc8b
      61fbd669
    • Maged Michael's avatar
      Update hazard pointer interface to standard proposal P0233R4 · eeae0d90
      Maged Michael authored
      Summary:
      Updated to the interface to be in synch with the latest standard proposal in P0233R4 as follows:
      - Renamed hazptr_owner as hazptr_holder.
      - Combined hazptr_holder member functions set() and clear() as reset().
      - Replaced the template parameter A for hazptr_holder member function templates try_protect() and get_protected with atomic<T*>.
      - Moved the template parameter T from the class hazptr_holder to its member functions try_protect(), get_protected(), and reset().
      - Added a non-template overload of hazptr_holder::reset() with an optional nullptr_t parameter.
      - Removed the template parameter T from the free function swap() as hazptr_holder is no longer a template.
      
      Reviewed By: davidtgoldblatt
      
      Differential Revision: D5283863
      
      fbshipit-source-id: 2bc1a09f4f844aa72d9b7dff9c450540bbe09972
      eeae0d90
    • Qi Zhou's avatar
      fix folly::FunctionScheduler.cancelFunctionAndWait() hanging issue · b3ef1edc
      Qi Zhou authored
      Summary:
      When
      - only one function is scheduled in FunctionScheduler; and
      - the function is running while cancelFunctionAndWait() is being called;
      FunctionScheduler.cancelFunctionAndWait() will hang forever.  The root cause is that the condition in cancelFunctionAndWait() is incorrect:
      
      ```
      runningCondvar_.wait(l, [currentFunction, this]() {
        return currentFunction != currentFunction_;
      });
      ```
      
      because currentFunction will not be changed if only one function is in the scheduler.
      
      The fix here is to
      - clear currentFunction as nullptr.  This also makes the internal behaviors of cancelFunction() and cancelFunctionAndWait() consistent.
      - introduces additional variable to indicate the state of cancelling current function.  After running the function, the background thread will detect cancellation of current function and clear the variable.
      - cancelFunctionAndWait() condition variable will wait for the variable to be cleared.
      
      Similarly, cancelAllFunctionsAndWait() also suffers from the same issue.
      
      Unit tests are added to reproduce the issue.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D5271664
      
      fbshipit-source-id: acb223304d3eab23129907ce9ff5e57e55f1e909
      b3ef1edc
    • Adam Simpkins's avatar
      logging: add LoggerDB::flushAllHandlers() · a5766338
      Adam Simpkins authored
      Summary:
      Add a method to flush all LogHandler objects.
      
      This will be necessary to implement FB_LOG(FATAL), as we will want to flush all
      outstanding messages before crashing.
      
      Reviewed By: wez
      
      Differential Revision: D5189501
      
      fbshipit-source-id: faf260b8e71e5dfed4a3b1c1aee32f072bd7b764
      a5766338
    • Adam Simpkins's avatar
      logging: add a LogHandler::flush() call · d65439dd
      Adam Simpkins authored
      Summary:
      Add a flush() call to the LogHandler interface.  This is needed to implement
      `FB_LOG(FATAL)` so that we can flush all LogHandlers before aborting the
      program.
      
      Reviewed By: wez
      
      Differential Revision: D5189499
      
      fbshipit-source-id: 75fa4d7e75ea26de5b7383bf7e8d073fb37e9309
      d65439dd
    • Adam Simpkins's avatar
      logging: add a NEVER_DISCARD flag to LogWriter · 84458670
      Adam Simpkins authored
      Summary:
      Add a flag to the LogWriter so we can ensure that particular messages are never
      discarded, even when the LogWriter is throttling messages.
      
      This functionality will be necessary to implement `FB_LOG(FATAL)` to that we
      can avoid discarding the reason for crashing.
      
      Reviewed By: wez
      
      Differential Revision: D5189498
      
      fbshipit-source-id: dc4322ea5ba449a341cdbdc32afb0ed466019801
      84458670
    • Adam Simpkins's avatar
      logging: add a small example program · 1b5a7dbf
      Adam Simpkins authored
      Summary:
      This adds a small example program which demonstrates using the logging library.
      
      This gives a very basic example of how the library is intended to be used, and
      can also be used to play around with controlling the log levels from the
      command line argument.
      
      Reviewed By: wez
      
      Differential Revision: D5083104
      
      fbshipit-source-id: ab09c6c88db33065f6e39f35b28014f2a6153cef
      1b5a7dbf
    • Christopher Dykes's avatar
      Delete folly/futures/OpaqueCallbackShunt.h · ac6751e6
      Christopher Dykes authored
      Summary: It is not used anywhere, not even in tests, so kill it.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D5280800
      
      fbshipit-source-id: 7e6a308bf09198548b77dcc1bfacc0ee95eb4887
      ac6751e6
    • Christopher Dykes's avatar
      Revert D5278412: [Folly] Merge StringBase.cpp and String.cpp · a6ad67f7
      Christopher Dykes authored
      Summary: This reverts commit 07639e155421f31a6cc7ed16cba2034750e44325
      
      Differential Revision: D5278412
      
      fbshipit-source-id: 4b4d60f1f472ebe2e68e8eea1a31f42bc55c60ed
      a6ad67f7
    • Christopher Dykes's avatar
      Merge StringBase.cpp and String.cpp · 76775588
      Christopher Dykes authored
      Summary: The only reason these were split was because of the use of `folly::format`
      
      Reviewed By: yfeldblum
      
      Differential Revision: D5278412
      
      fbshipit-source-id: 07639e155421f31a6cc7ed16cba2034750e44325
      76775588
    • Christopher Dykes's avatar
      Switch pid_t to be defined as int rather than void* · 04429ca1
      Christopher Dykes authored
      Summary: Because an `int` plays much much nicer with code already written for Windows than `void*` does. It takes quite a bit of hackery to make it possible with the pthread implementation we support using, but it is possible and is worth the effort.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D5276968
      
      fbshipit-source-id: 4cd0d3120a4f19976e76853ef4b337e96d5005a8
      04429ca1
  3. 19 Jun, 2017 5 commits
  4. 18 Jun, 2017 1 commit
    • Christopher Dykes's avatar
      Fix a few exception_wrapper tests under MSVC · dd707e05
      Christopher Dykes authored
      Summary: They were incorrectly comparing against string literals rather than the actual demangled names. MSVC includes `class/struct` as part of the mangled name, so they also appear in the demangled name, in contrast to GCC/Clang, which don't.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D5271087
      
      fbshipit-source-id: 41fa0cebe098b1b935e1f8b1af5882c412cf6254
      dd707e05
  5. 17 Jun, 2017 1 commit
  6. 16 Jun, 2017 9 commits
    • Christopher Dykes's avatar
      Revert D4567233: [Folly] Drop support for Clang < 3.9 · 59d010b5
      Christopher Dykes authored
      Summary: This reverts commit cd94c711f0390d249b6736ab292721a477022b02
      
      Differential Revision: D4567233
      
      fbshipit-source-id: a10aff6b4c231666aec9f40ab65e881c3cd870dc
      59d010b5
    • Peter DeLong's avatar
      Improve get_fiber_manager_map_*() error reporting · 030e254e
      Peter DeLong authored
      Summary:
      get_fiber_manager_map_vevb() and get_fiber_manager_map_evb() don't
      provide very useful feedback when called with an empty map (just the former) or
      with a program that doesn't have debug symbols (both)
      
      Reviewed By: andriigrynenko
      
      Differential Revision: D5260114
      
      fbshipit-source-id: c44b4e279e5c88dc08507b969339a4befc23d79b
      030e254e
    • Uladzislau Paulovich's avatar
      Fix class member shadowing in folly::ProducerConsumerQueue · a60bf0bb
      Uladzislau Paulovich authored
      Summary: Previous version failed to compile with "-Werror=shadow" flag, this commit fixes the problem.
      
      Reviewed By: WillerZ
      
      Differential Revision: D5255814
      
      fbshipit-source-id: c1252474ed5415b47759022bcbabc78c1639e10a
      a60bf0bb
    • Christopher Dykes's avatar
      Remove the relative comparison operators on LogLevel · ff607e1b
      Christopher Dykes authored
      Summary: MSVC, GCC, and Clang define the relative comparison operators by default on `enum class` values, however under MSVC defining them explicitly causes the compiler to fail to resolve which operator to use, so just remove them entirely, as they aren't needed.
      
      Reviewed By: simpkins
      
      Differential Revision: D5259835
      
      fbshipit-source-id: 1b91db5de0fc47320daabfdd7132ae910887ff0f
      ff607e1b
    • Christopher Dykes's avatar
      Don't declare caught exceptions when not actually used · ab9a4d71
      Christopher Dykes authored
      Summary: There are a couple of places where the caught exception was being given a name that was never used, generating a warning under MSVC.
      
      Reviewed By: simpkins
      
      Differential Revision: D5260131
      
      fbshipit-source-id: f82c6bd1266f6a4c916594ec3ac94edc9a2e48fe
      ab9a4d71
    • Christopher Dykes's avatar
      Don't try to use _CrtDbgReport in the logging framework · cd02eca6
      Christopher Dykes authored
      Summary: Glog doesn't try to, so there's no real reason to try and be smart. Just do the same thing as every other platform for now until we have reason to do otherwise.
      
      Reviewed By: simpkins
      
      Differential Revision: D5259435
      
      fbshipit-source-id: dcdf55f51f2e13945afd351cb45e9c1a22f56218
      cd02eca6
    • Christopher Dykes's avatar
      Ensure LogWriter::writeMessage(std::string&&) is in the overload set of ImmediateFileWriter · 19859f96
      Christopher Dykes authored
      Summary: As MSVC correctly warns via C4266, the overload sets of functions in base classes are not part of the overload set in derived classes where you've declared an overload (or override) unless you've explicitly imported it.
      
      Reviewed By: simpkins
      
      Differential Revision: D5260056
      
      fbshipit-source-id: bbbeeea3c13201a3a6eba6e62cfa5a49a9470d43
      19859f96
    • Christopher Dykes's avatar
      Include the time portability header in GlogStyleFormatter.cpp · 2bb2c015
      Christopher Dykes authored
      Summary: It is needed on Windows for the `localtime_r` function.
      
      Reviewed By: simpkins
      
      Differential Revision: D5259944
      
      fbshipit-source-id: e380ecf42252ecabec168cf87668ace3ff4c0a9f
      2bb2c015
    • Christopher Dykes's avatar
      Explicitly initialize the base class of LogStream · a219d071
      Christopher Dykes authored
      Summary:
      The standard does not define that `std::ostream` should have a default constructor, so this was invalid and was an error on Windows.
      This explicitly initializes the base class by passing a `nullptr`.
      
      Reviewed By: simpkins
      
      Differential Revision: D5259690
      
      fbshipit-source-id: b8914d73ff2682e1a4447b8338860259778b2247
      a219d071
  7. 15 Jun, 2017 9 commits
    • Andrii Grynenko's avatar
      Fix Observable to not trigger unneccessary refresh if the value didn't change · 3e010905
      Andrii Grynenko authored
      Differential Revision: D5251218
      
      fbshipit-source-id: 1ceb37dd727e8ac2fd842e2c437cdaa9017221c8
      3e010905
    • Adam Simpkins's avatar
      fix issues with the open source tests · 5dd16dc0
      Adam Simpkins authored
      Summary:
      Fix build problems in the open source Makefiles that cause issues when running
      `make check`
      
      - The test subdirectory needs to be processed before experimental, since some
        tests in experimental/ depend on libfollytestmain from test/
      - The stats/test Makefile had an incorrectly copy-and-pasted line for
        libgtest_la_SOURCES but didn't actually build a libgtest.la library.
      - The test/ Makefile defined thread_id_test but forgot to add it to TESTS so
        that it would actually be run as part of "make check"
      
      Reviewed By: Orvid
      
      Differential Revision: D5249132
      
      fbshipit-source-id: 5a71e1f72a39d5407b843a5876891c67238ec006
      5dd16dc0
    • Adam Simpkins's avatar
      logging: add initialization convenience functions · f47a8a52
      Adam Simpkins authored
      Summary:
      Add a logging/Init.h header file with a couple convenience functions for
      initializing log levels and log handlers.
      
      This is pretty basic for now, but simplifies usage for small programs that just
      want to easily initialize the logging library.
      
      Reviewed By: wez
      
      Differential Revision: D5083106
      
      fbshipit-source-id: 73c1fd00df2eaf506b9c1485d6afd12570412a0f
      f47a8a52
    • Adam Simpkins's avatar
      logging: add GlogStyleFormatter · 48b88f01
      Adam Simpkins authored
      Summary:
      Add a LogFormatter implementation that logs messages using a similar format to
      the glog library.
      
      Reviewed By: wez
      
      Differential Revision: D5083108
      
      fbshipit-source-id: 75f0a6b78ce5406b4557d6c4394f033d5e019f71
      48b88f01
    • Adam Simpkins's avatar
      logging: add AsyncFileWriter · 82b71ca3
      Adam Simpkins authored
      Summary:
      Add an AsyncFileWriter class that implements the LogWriter interface using a
      separate I/O thread to write the log messages to a file descriptor.
      
      This LogWriter implementation ensures that normal process threads will never
      block due to logging I/O.  By default it will buffer up to 1MB of data.  If
      log messages are generated faster than they can be written to the file, log
      messages will be discarded once the buffer limit is exceeded.  The LogWriter
      will emit a message into the file recording how many messages were discarded
      where the dropped messages should have been.
      
      The downside of this class is that unwritten log messages still in the buffer
      will be lost when the program crashes.
      
      Reviewed By: wez
      
      Differential Revision: D5083107
      
      fbshipit-source-id: c67226f4d0726675d480b03eae83a29c5c3431b2
      82b71ca3
    • Adam Simpkins's avatar
      logging: add ImmediateFileWriter · cb238d29
      Adam Simpkins authored
      Summary:
      Add an ImmediateFileWriter class that implements the LogWriter interface by
      immediately writing data it receives to a file descriptor.
      
      This LogWriter implementation can be used by users that want to ensure log
      messages are immediately flushed to a file as soon as they are written, at the
      expense of possibly having logging I/O block their normal process threads.
      
      Reviewed By: wez
      
      Differential Revision: D5083105
      
      fbshipit-source-id: 15cd071834e1784fe50e1d6d6ce403b9bc907047
      cb238d29
    • Adam Simpkins's avatar
      logging: add a LoggerDB::internalWarning() function · 78307e3f
      Adam Simpkins authored
      Summary:
      A few places in the logging code need to report issues when something goes
      wrong in the logging code itself.  This adds a new LoggerDB::internalWarning()
      method for use in these places.
      
      This provides a default implementation that logs these warnings with
      _CrtDbgReport() on Windows, and writes to stderr on non-Windows platforms.
      The default implementation can also be overridden for programs that want some
      alternative behavior.
      
      Reviewed By: wez
      
      Differential Revision: D5157731
      
      fbshipit-source-id: e27f5587d58b3941fca8d5b38c8c3edbcb75d7f3
      78307e3f
    • Adam Simpkins's avatar
      logging: add new RateLimiter helper class · dc37f8ea
      Adam Simpkins authored
      Summary:
      Add a new RateLimiter API to the logging library, to support rate limiting
      messages in the future.  I have included a single IntervalRateLimiter
      implementation in this diff.  In the future we can add more implementations if
      necessary, to mimic the functionality available with our older logging code, to
      make it easier for users to convert to the new framework.
      
      Note that RateLimiter is inside a `folly::logging` namespace, unlike most of
      the other code in the logging library that lives directly in the `folly`
      namespace.  I intentionally chose this since RateLimiter is a fairly generic
      class name, and I wanted to distinguish it from other possible generic class
      names in folly.  On the other hand, most of the other class names already start
      with `Log`, so there seems to be no need to put them in a `logging` namespace.
      
      Nothing is using this new API yet, but I will use it for some internal logging
      APIs in an upcoming diff.  Later on I also plan to use it to implement
      per-LogCategory rate limiting, and possibly per-LogHandler rate limiting.
      
      Reviewed By: wez
      
      Differential Revision: D5162805
      
      fbshipit-source-id: 9b81c2f4544006cd392152a768296bce0c5daaa1
      dc37f8ea
    • Adam Simpkins's avatar
      logging: add LogFormatter and LogWriter interfaces · 9578ff20
      Adam Simpkins authored
      Summary:
      This simplifies the LogHandler interface to a single generic `handleMessage()`,
      and adds a `StandardLogHandler` implementation that defers to separate
      `LogFormatter` and `LogWriter` objects.
      
      The `LogFormatter` class is responsible for serializing the `LogMessage` object
      into a string, and `LogWriter` is responsible for then doing something with
      the serialized string.
      
      This will make it possible in the future to have separate `LogWriter`
      implementations that all share the same log formatting code.  For example, this
      will allow separate `LogWriter` implementations for performing file I/O
      immediately versus performing I/O asynchronously in a separate thread.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D5083103
      
      fbshipit-source-id: e3f5ece25e260c825d49a5eb30e942973d6b68bf
      9578ff20