Benchmark specific fixes

Summary:
benchmark is necessary to get the tests running
primarily replacing a few inline asm items with MSVC intrinsics and do not optimize tricks
also a fix for use of the gcc specific ## with __VAR_ARGS__
although that is primarily intended as a workaround for trailing commas, gcc apparently cuts off all items in the macro afterwards
this was being used for a clever/dirty trick to do one or none for a macro
replaced instead with a version that will work for all other compilers (might need to be expanded for more args, but for now it's working with all current usage)

@override-unit-failures

also fixed a use of max without specifying a template type that was making msvc barf - specifying it (as in the min case) made it compile cleanly

Test Plan: fbconfig -r folly && fbmake runtests

Reviewed By: andrei.alexandrescu@fb.com

FB internal diff: D1313609

race in Future destructor

Summary:
@mnd wrote some Wangle code that would trip up with a `bad_function_call` exception. This Shouldn't Happen™ but the exception comes from trying to call a `std::function` which is null. We pretty thoroughly examined his usage and didn't find any problems, and this patch seems to make the error go away. See #4207781 for more details.

And reasoning about it, it makes sense. Inline comments explain the race.

Test Plan: Alas, I haven't been able to get a minimal repro and therefore a regression unit test. It's a hard race to trigger. I still don't understand why Matt's code does it.

Reviewed By: davejwatson@fb.com

FB internal diff: D1304001

printf format checking for msvc

Summary:
sal annotations can be used to do similar (not exact) checking to the functionality provided by the format attribute in gcc
the annotations are done by prefixing the format string with a value which makes the macro definitions a bit messy

@override-unit-failures

Test Plan: fbconfig -r folly && fbmake runtests

Reviewed By: delong.j@fb.com

FB internal diff: D1313653

Use winpthreads clock_gettime implementations

Summary:
No need to reinvent the wheel - winpthreads has the appropriate posix layer in place for time functionality
and we're already using it for pthreads on windows - so just include the right headers to make sure defines are set up when using time functionality

@override-unit-failures

Test Plan: fbconfig -r folly && fbmake runtests

Reviewed By: delong.j@fb.com

FB internal diff: D1313600

Adding some msvc specific defines

Summary:
msvc puts ssize_t in a stupidly odd place and names it weirdly too
this also takes care of snprintf missing (the semantics are slightly off in the msvc version regarding the return value, but usage in folly is limited and does NOT do the double snprintf call madness so this is safe)
funcsig and pretty function give you roughtly the same thing in compiler specific implementations
strerror_s is msvc's thread save strerror

@override-unit-failures

Test Plan: fbmake runtests

Reviewed By: delong.j@fb.com

FB internal diff: D1291542

folly::join takes advantage of StringPiece::size()

Summary: folly::join should take advantage of StringPiece::size() when joining StringPieces. This avoids unnecessary resizes when appending values to the output string.

Test Plan: Reran folly unit tests for strings

Reviewed By: philipp@fb.com

FB internal diff: D1313009

@override-unit-failures

Handle event_base_new failure when out of file descriptors.

Summary:
Both event_base_new and event_init return nullptr when out of file descriptors.
Using null event_base will result in segfault.

Test Plan: (ulimit -n 50000 && _build/opt/sigma/service/sigma_server --instance_name=si_sigma_push --min_scribe_log_level=0 --allow_status_port_fallback=true --minloglevel=1 --v=0 --feature_objects_limit=1000000 --hbase_default_timeout_ms=250 --max_total_connections_per_region_server=10 --max_retained_connections_per_region_server=10 --tao_default_timeout_ms=5000 --enable_writes_scribe_si_floop=false --enable_writes_all=false --arena_size_limit=268435456 --run_fxl=true)

Reviewed By: davejwatson@fb.com

FB internal diff: D1311855

ignore non-existent mount points in readHugePageSizes

Summary:
Some of mount points may not exist (for example, if
application is running from chroot and unshare hasn't been used).

@override-unit-failures

Test Plan: unicorn canary

Reviewed By: tudorb@fb.com

FB internal diff: D1311374

Add defaulted() in Format.h to avoid throwing on missing keys; add string-y dynamic constructors

Test Plan: folly/test

Reviewed By: simpkins@fb.com

FB internal diff: D1303911

ExceptionWrapper comments

Summary: Comments to discuss motivation

Test Plan: its comments

Reviewed By: delong.j@fb.com

FB internal diff: D1308259

@override-unit-failures

Move setPosixThreadName & friends to folly.

Summary: This was previously in thrift (and copied and pasted in several other places, including folly itself). Other potential open-source projects want this basic functionality so lets centralize it in folly instead of having potentially awkward dependencies on thrift (or copy/paste everywhere).

Test Plan: Build all the things. Run the tests.

Reviewed By: delong.j@fb.com

FB internal diff: D1297972

Fix for Android importing folly/Random.cpp

Summary: ext/random causes issues building on android

Test Plan: compiled on fbandroid

Reviewed By: bmaurer@fb.com

FB internal diff: D1308224

exception_wrapper: now without undefined behavior

Summary:
Converting from std::exception* to void* to T* (where T is not std::exception
but a derived type) is undefined behavior (and will break with multiple or
virtual inheritance). Luckily, there's no need for void* there at all.

Also, don't force make_exception_wrapper to capture by value.

Test Plan: exception_wrapper_test

Reviewed By: marccelani@fb.com

FB internal diff: D1308251

@override-unit-failures

MemoryMapping changes: automatically detect huge pages, no more WritableMemoryMapping

Summary: "writable" is now an option to the MemoryMapping constructor.

Test Plan: folly/test, thrift/lib/util/test:FrozenUtilTest, others

Reviewed By: kma@fb.com

FB internal diff: D1307044

@override-unit-failures

Add missing files to Makefile.am

Summary:
As title, add newly added files.  fbthrift's jenkins build is failing due to missing futex and lifosem.  Added other ones that looked new
http://ci-builds.fb.com/job/fbthrift/

Test Plan: Built folly on ubuntu.  Builds.

Reviewed By: pgriess@fb.com

FB internal diff: D1306401

exception wrapper

Summary:
folly::exception_wrapper is a different take on std::exception_ptr to
suit a specific use case.

The good: std::exception_ptr is not templated, so it can easily be used in
different classes without template creep. You can pass errors around between
threads or simply between modules. Rethrowing the exception throws the *proper*
derived class exception, not some base class.

The bad: Getting access to the exception requires throwing, which is expensive.
Many users of popular frameworks that take advantage of std::exception_ptr
check if the exception is set, and if so do some error handling without actually
knowing the type of the exception or logging its message, just to avoid the cost
of rethrowing the exception.

The ugly: Creating an exception_ptr requires throwing the exception as least
once. This is bad in the performance sensitive case where users will not even
inspect the exception.

This class takes advantage of the good while avoiding the requirement to throw.
By using a templated deleter and thrower function, we can create an
exception_wrapper which is properly managed, can be thrown, and can be retrieved
as a void* with a get() function. Users that previously caught exceptions are
now able to dynamically cast to different exception types they formerly caught
to avoid the unwind cost while still getting details about the error.

Test Plan: unit test

Reviewed By: davejwatson@fb.com

FB internal diff: D1305470

@override-unit-failures

Fix Chatty subprocess test, call callback on hangup

Summary:
The Chatty subprocess test incorrectly assumed that we saw EOF on the last
read from the child (that is, read() returned 0). That's not the case for two
reasons: 1. the child is allowed to stall right before it closes its stdout, in
which case we get EAGAIN, and 2. Subprocess::communicate would close the fd
without calling the read callback anyway. Fix both such things.

Test Plan: ran test

Reviewed By: njormrod@fb.com

FB internal diff: D1303215

QueueBenchmark set max read at once

Summary: Makes a fair comparison between asox queue and notification queue.  THe updated benchmark is just noise in this diff, notificationqueue isn't fast enough (yet) to make a difference.

Test Plan:
fbconfig common/concurrency; fbmake opt
./common/concurrency/QueueBenchmark.sh

Reviewed By: afrind@fb.com

FB internal diff: D1272859

Use readNoInt/writeNoInt in folly::Subprocess

Summary: We have these helper methods, but are not using them. What sadness.

Test Plan:
fbconfig -r folly
fbmake runtests

Reviewed By: tudorb@fb.com

FB internal diff: D1299720

Include <random> in folly/Random.h

Summary:
- Its methods are being used; include the header

@override-unit-failures

Test Plan: - Build for iOS

Reviewed By: tudorb@fb.com

FB internal diff: D1299440

Default to using folly::LifoSem

Summary:
Unless a service is overloaded, it should be able to clear
its queue frequently. When this happens, threads fall asleep until
more work is available in the queue. Waking up threads in LIFO
order gives us a lot of benefits. First, threads that were most
recently active are more likely to be mapped to the same cpu core
and thereby experience better L1 cache hit rate. Second, we can
madvise away jemalloc arenas on very idle threads. If we wake up
threads in FIFO order, we will never get a thread to remain idle
long enough for this to be worthwhile.

folly::LifoSem does just that. Benchmark in which the queue is
allowed to drain show that we get a substantial increase in
throughput by waking up threads in LIFO order.

Test Plan:
QueueBenchmark results summary:
As expected, benchmarks run faster in the case where the queue is
able to frequently drain itself, particularly in cases where the
number of threads is large. Benchmarks run slower when the
consumers cannot keep up with the producers, particularly when we
reach the queue capacity and we need to synchronize between
producers and consumers. However, in this case I think we care
less about the overhead of the queue itself and more about how
quickly we can shed the actual underlying load.

Reviewed By: davejwatson@fb.com

FB internal diff: D1298343

fixing split_step documentation

Summary: example results were backwards

Test Plan: unit tests

Reviewed By: brg@fb.com

FB internal diff: D1262425

SymbolizedFrame::name is already null-terminated

Summary:
... as it comes from Elf::getSymbolName, which returns null-terminated C
strings. So there's no need to copy it into a fixed-size buffer (and have a
buffer overflow, ouch).

Test Plan: folly/experimental/symbolizer_test, see what else "arc unit" gets us

Reviewed By: tconerly@fb.com

FB internal diff: D1286348

std::move-able MemoryMapping

Test Plan: unit tests

Reviewed By: lucian@fb.com

FB internal diff: D1290632

Support addValueAggregated() for histogram

Summary:
Support addValueAggregated() for histogram to add multiple samples for
one bucket at one time.

Test Plan: Have a client to call the new API and test in production.

Reviewed By: simpkins@fb.com

FB internal diff: D1286528

Better <bits/c++config.h> detection

Summary:
When worrying about only 2 cpp libraries, detection of "is this libstdc++ or libc++" works, but when throwing in a third option (msvc) this becomes a very brittle way of detecting features.  Also depending on how clang is used it may or may not have this header about

Changed to instead use configuration value for "do we have this header file" for including bits/c++config.h and changed the logic to be identical in all three use cases

Test Plan: fbmake runtests

Reviewed By: njormrod@fb.com

FB internal diff: D1291547

msvc packed attribute translation

Summary:
Provide translations for gcc packed

MSVC has a very very ugly syntax for packing using pragmas which is very different from the gcc attribute
this makes it very hard to macro around, but lots of ifdefs are also painful - cleanest of the hackery is to simply have push, pop, and attribute macros for packed that define properly depending on the compiler

Test Plan: fbmake runtests

Reviewed By: njormrod@fb.com

FB internal diff: D1279966

Future API for CacheClientCommon

Summary:
Allow mutable functors in Promise::fulfil.

Facebook:
Works both with EventBase and without it (thanks to wangle::Future thread-safety). CacheClientImpl lifetime is guaranteed using shared_ptr.

Test Plan: unit test

Reviewed By: stepan@fb.com

FB internal diff: D1291024

fix off-by-one error in IPAddress::longestCommonPrefix()

Summary:
Fix an off-by-one error causing an ASAN abort.  When calling
longestCommonPrefix() with a full mask, it would compare one byte past
the end of the address data, and would end up writing past the end of
the ba array on the stack.

Test Plan:
Built with ASAN, ran the unit tests, and verified the ASAN failure was
gone.

Reviewed By: jasmeetbagga@fb.com

FB internal diff: D1284750

Move common/network/IPAddress.h and related to folly/

Summary:
Moving our internal IP/Mac address libraries to folly/

Facebook:
We want to get rid of common/ dependencies in Mcrouter since we're going to open source it. Also looking at the original commit, seems like it's been the intention all along, so I just did it.

I tried to keep dependencies intact as much as possible. Changing projects to use this directly should be in separate diffs.

Test Plan:
Run folly/network and common/network unit tests.

Generate the list of targets with:

```
fbgs /$FILE.h | cut -f1 -d: | xargs -L1 dirname | cut -f2- -d/ | sort | uniq
```

Then fbconfig + fbmake. Will fix contbuild failures.

Revert Plan:

Reviewed By: simpkins@fb.com

FB internal diff: D1261089

Fix type mismatch in IOBuf::getIov()

Summary: - On iOS, iovec::iov_len is a size_t, which is a uint32_t; convert

Test Plan:
- fbconfig -r folly && fbmake runtests
- Builds in fbobjc

Reviewed By: simpkins@fb.com

FB internal diff: D1284931

Fix header ordering lint failures

Summary:
Headers whose includes are not sufficient cause problems when included
from other files. An easy way to prevent this is to include the header
first in its associated cpp file. In fact, we have a lint rule for this.
However, there are places where this rule is ignored. This diff fixes
many of them.

Note that a few top-level directories were excluded.

Reviewed By: robbert@fb.com

FB internal diff: D1281032

x64 detection missing an include

Summary: rwspinlock.h was missing the portability.h header which made the x64 detection go wonky, but only shows up when used with isolated rwspinlock use

Test Plan: fbmake runtests

Reviewed By: andrei.alexandrescu@fb.com

FB internal diff: D1284387

Blame Revision: D1282140

macro for cross platform x64 detection

Summary: Another cross platform fun macro - this one detects x64 status and requires some portability.h includes (which should only define portability stuff and not actually include anything)

Test Plan: fbmake runtests

Reviewed By: delong.j@fb.com

FB internal diff: D1282140

Fix flag naming

Summary: doh
@override-unit-failures

Test Plan: compiled under android

Reviewed By: subodh@fb.com

FB internal diff: D1283177

fbstring likely/unlikley ifdef for stuff that doesn't support it

Summary: the fbstring specific unlikely/likely defs were missing the ifdef blocks to avoid breaking things without builtin_expect

Test Plan: fbmake runtests

Reviewed By: njormrod@fb.com

FB internal diff: D1282166

(Folly/Gen) Make ranges and sequences take a stepping functor

Summary:
The functor allows to create ranges and sequences that advance the
iterator by more than 1 per iteration.

Test Plan: Unit test.

Reviewed By: tjackson@fb.com

FB internal diff: D1228065

(Folly/Gen) Fix compilation with clang

Summary:
Clang chokes because it tries to instantiate both versions of `from<const int*>`, one of which calls `std::begin(const int*)`, which doesn't work. By casting the intializer list to the right type, it can pick the overload.

Clang, because it makes debugging these templates so much better.

Test Plan: `fbconfig --clang folly/gen/test && fbmake runtests_dbg`

Reviewed By: tjackson@fb.com

FB internal diff: D1280888

abstract thread_local support

Summary:
change from using __thread to using FOLLY_THREAD_LOCAL macro, this will allow abstraction over gcc and msvc implementations of thread local (__thread and __declspec(thread)) which have the same semantices and will also allow drop in replacement of thread_local when compiler support for the feature is complete  This doesn't do anything about apple, however, which still has broken __thread support

This doesn't actually change any implementation for now, simply allows for correct compilation

Test Plan: fbmake runtests

Reviewed By: delong.j@fb.com

FB internal diff: D1278726

Add missing includes for msvc

Summary: MSVC is a stricter compiler and needs additional includes explicitly declared

Test Plan: fbmake runtests

Reviewed By: njormrod@fb.com

FB internal diff: D1277004

parallel(pipeline)

Summary:
Adding `... | parallel(my | pipe | line) | ...` for parallelizing a portion of a generator pipeline.

```lang=cpp
auto factored = from(values)
| parallel(filter(isEven) | map(square) | sub(count))
| sum;
```

Work is divided evenly among a fixed number of threads using a `MPMCQueue`.

Test Plan: Unit tests and benchmarks testing for a variety of workloads and performance characteristics, including sub-linear (blocking) workloads, linear (mostly math) workloads, and superlinear (sleeping) workloads to simulate real-world use.

Reviewed By: lucian@fb.com

FB internal diff: D638551

Introduce LifoSem

Summary: A LIFO semaphore is useful for building a thread pool because we can keep cpu caches warmer by continually reusing the last used thread. This diff introduces a LIFO semaphore to folly.

Test Plan: unit tests

Reviewed By: davejwatson@fb.com

FB internal diff: D1280405

Fix folly Uri::host() return value for IPv6 address

Summary: folly Uri::host() returns leading and trailing brackets for IPv6 host, for example, for URI "http:://[::1]:8080/index.html", host() method returns "[::1]". But square brackets are not part of host in fact, this diff is going to remove brackets for IPv6 host.

Test Plan: fbconfig -r folly/test && fbmake runtests_dbg

Reviewed By: chip@fb.com

FB internal diff: D1276513

MSVC translation of noreturn attribute

Summary:
Provide translations for gcc noreturn attribute

__attribute__((noreturn)) is gcc specific, clang understands it on nix systems, but for msvc __declspec(noreturn) is the compiler specific version, which clang will imitate/use on windows.  There was already a FOLLY_NORETURN in portability.h, however because of __declspec limitations it must prefix the declaration, not postfix.  The gcc noreturn attribute does not have this limitation and will work prefixed OR postfixed, so to keep from turning code into spaghetti nonsense, the macro was moved to the front of declations where it is currently used.  This will allow it to work with the proper definitions on clang, clang on windows, gcc, and msvc

Test Plan: fbmake runtests

Reviewed By: delong.j@fb.com

FB internal diff: D1279466

MSVC always_inline and noinline __attributes__ translation

Summary:
Provide translations for gcc always_inline and noinline attribute

Change to using a macro from portability.h for attributes for using always_inline and noinline for cross platform support

Test Plan: fbmake runtests

Reviewed By: delong.j@fb.com

FB internal diff: D1279652

Use StringPiece instead of String

Summary:
Using StringPiece makes it easier than std::string

Facebook: Motivation for this diff is found in D1253595

Test Plan:
Added new unittest

fbconfig folly/io/test && fbmake runtests_opt

Reviewed By: simpkins@fb.com

FB internal diff: D1276185

uint isn't portable and makes some compilers angry, change to unsigned int

Summary: As part of the windows port, uint is not defined as a type and is not standard, it makes msvc choke, a simple change to unsigned int fixes the issue

Test Plan: fbmake runtests

Reviewed By: delong.j@fb.com

FB internal diff: D1278487

Add non-const operator[] to TypedIOBuf

Test Plan: used it

Reviewed By: lucian@fb.com

FB internal diff: D1277063

@override-unit-failures

folly: improve ProducerConsumerQueueBenchmark (misplaced doNotOptimizeAway)

Summary:
as per discussion from https://phabricator.fb.com/D1274352

After changing the queue length to be a small value (100) I didn't see
improvement from cache alignment of atomics (too much noise). Dropping
that and just fixing the benchmark.

Test Plan: n/a

Reviewed By: ngbronson@fb.com

FB internal diff: D1276591

Fix apparent race in SubprocessTest

Summary:
The code originally contained the line

EXPECT_EQ((rcount == lineCount), r); // EOF iff at lineCount

Which asserted that, on the last iteration, EOF was being read (and thus
r was true).

This line has been causing errors in contbuild. There seem to be two
issues at play.

First, the left hand side of the EXPECT_EQ is always false. rcount is
incremented three lines down, and the (implicit) loop ends when rcount
equals lineCount. This means that, on the last iteration, rcount is
actually one less than lineCount. Hence the check is effectively
asserting that r is false.

The second issue is that r is rarely true. Empirically, a necessary but
not sufficient condition is that there be lots of other processes
running at the same time (such as when running fbconfig -r folly &&
fbmake runtests_opt). This would explain why the test passes during
development but fails in contbuild.

I am guessing that, when there are few processes running, Subprocess is
predictably ceasing to read before whichever other process yields EOF,
and that this does not always happen when there is resource contention.

As an immediate fix, I am asserting that r may only be true on the last
iteration. Could someone (cc @tudorb) who is more familiar with the
intricacies of Subprocess please check to see if this is an idiosyncrasy
of the test, or if there is actually an underlying bug.

Test Plan:
Before the change, run fbconfig -r folly && fbmake runtests_opt
repeatedly. Notice that most of the iterations yield an error from line
376 (which is consistent with contbuild error logs).

After the change, notice that the error disappears.

Reviewed By: andrewjcg@fb.com

FB internal diff: D1269124

folly: gen: pmap: parallel version of map

Summary:
same as map, but runs it's argument in parallel over a number of threads.

@override-unit-failures

Test Plan: added test

Reviewed By: tjackson@fb.com

FB internal diff: D1258364

Change child's working directory

Summary: Changing the parent's WD is prone to race conditions of various sorts, and needlessly alters parent state. Python's subprocess also has this feature.

Test Plan: fbmake dbg _bin/folly/test/subprocess_test ; ../_bin/folly/test/subprocess_test

Reviewed By: agoder@fb.com

FB internal diff: D1269526

folly: allow to build with clang.dev (flex arrays vs. lambda)

Summary:
* folly/test/CacheLocalityTest.cpp (contentionAtWidth): Work around
clang's prohibition against using flexible arrays in a lambda.

Test Plan:
fbconfig --clang --with-project-version clang:dev folly/test \
&& fbmake runtests

Reviewed By: ngbronson@fb.com

FB internal diff: D1263620

configurable alignment for Arena

Summary: Allow specifying custom alignment.

Test Plan: fbconfig -r folly/test && fbmake opt -j32

@override-unit-failures

Reviewed By: lucian@fb.com

FB internal diff: D1264851

Make header compatible with warp

fix ConcurrentSkipList::Recycler layout

Summary:
D1261546 introduced regression in `sizeof(ConcurrentSkipList)`.
In case of `NodeAlloc = folly::SysAlloc`, size of an empty `NodeAlloc` struct
is 1 byte, due to the alignment requirements of the next field, it got
translated into wasting 8 bytes.

Test Plan: fbconfig -r folly/test && fbmake opt -j32

@override-unit-failures

Reviewed By: lucian@fb.com

FB internal diff: D1264730

folly: accommodate fact that clang's strlen is not constexpr

Summary:
* folly/folly-config.h (FOLLY_HAVE_CONSTEXPR_STRLEN): Define to
zero when compiling with clang, since clang's strlen is not constexpr.

Test Plan:
Run this:
fbconfig --clang --with-project-version clang:dev folly/test && fbconfig dbg
and observe that the following no longer strikes:
folly/test/RangeTest.cpp:304:25: error: constexpr variable 'hello1' must be initialized by a constant expression

I also tried using clang-3.4 (the current default),
fbconfig --clang folly/test && fbmake dbg
with these results:
https://phabricator.fb.com/P8583443

Reviewed By: pgriess@fb.com

FB internal diff: D1262840

Fix make check

Summary: Fix make check.  Benchmark.cpp was included twice, eventfd doesn't exist anymore.

Test Plan: autoreconf -i; ./configure; make check on ubuntu

Reviewed By: pgriess@fb.com

FB internal diff: D1259706

fix automake

Summary:
Automake complains there are two Malloc.cpp rules - rename one of them.

Facebook: Actually this file doesn't even build in fbconfig - is it still necessary?

Test Plan: autoreconf -i on ubuntu doens't fail

Reviewed By: pgriess@fb.com

FB internal diff: D1257372

(wangle) codegen then() variant tests

Summary:
When all the variants I am dreaming of work, there's a little more than
100 tests generated.

Test Plan: fbmake runtests

Reviewed By: hannesr@fb.com

FB internal diff: D1258754

(Folly/FBString) Fix compilation time of test with opt

Summary: Now compiles in 18 seconds. Took two minutes before.

Test Plan: `fbmake runtests_opt`

Reviewed By: smith@fb.com

FB internal diff: D1243689

configurable node allocator for ConcurrentSkipList

Summary:
Allow specifying `SimpleAllocator` to use for node allocation.
Added optimizations for arena allocators.

Test Plan: fbconfig -r folly/test && fbmake opt -j32

Reviewed By: soren@fb.com

FB internal diff: D1261546

make ConcurrentSkipList ctor public

Summary:
There is no reason to force heap allocation for
`ConcurrentSkipList`.

Test Plan: fbconfig -r unicorn/test && fbmake opt -j32

@override-unit-failures

Reviewed By: lucian@fb.com

FB internal diff: D1261183

Adding support for signed integers

Summary: For use with Thrift::Frozen, especially.

Test Plan: Unit tests

Reviewed By: philipp@fb.com

FB internal diff: D1255257

Bugfix in iterator emplace

Summary:
Subtle bugfix to fbvector

Reproduction requirements:
- emplace into a vector at a non-end position
- the vector has enough capacity for the extra elements
- the value type's internal state is poisoned after a move

What should happen, explained by picture:

INITIAL SETUP

abc1234XY____
^
want to insert two elements here

FIRST STEP: uninitialized move-construction
x and y are initialized, but could be in an invalid state

abc1234xyXY__

SECOND STEP: move-assignment, in reverse order

abc123xy4XY__
abc12xy34XY__
abc1xy234XY__
abcxy1234XY__

What actually happens:
An indexing error causes the entire tail (##1234xy##) to be
move-assigned, instead of the intended head of the tail (##1234##).

If the data type's move operator does not invalidate its own data (i.e.
move is essentially a copy, as is the case with atomic types) then the
move assignment of ##y## onto ##Y## and ##x## onto ##X## is basically a
no-op. However, for types that do invalidate their own data when being
the source of a move (e.g. non-empty vectors and fbvectors), then the
end result will have bad data in place of ##X## and ##Y##.

Detection:
This bug has lain dormant for a while.

Very few people emplace into a vector, and those few who do could be
saved by using a copy-movable type like int.

The original test case did not use a data type which poisoned itself
after being the source of a move, so the tests did not notice any
oddities.

A new testsuite, which does poison itself, did notice.

Test Plan:
re-enable the test/stl_test mega-test. Run it. All tests
pass.

fbconfig -r folly && fbmake runtests_opt

Run fbvector through the new test suite (which initially caught the
error). It now passes.

Reviewed By: robbert@fb.com

FB internal diff: D1257258

A few missing includes

Summary: These were required to build folly on OSX.

Test Plan: build

Reviewed By: davejwatson@fb.com

FB internal diff: D1256856

(wangle) Use an atomic_flag to make FutureObject threadsafe

Summary:
There are two operations on `FutureObject` (the backing object that both `Future` and `Promise` front), which are not threadsafe currently. Those are `setContinuation` and `fulfil`. They're not threadsafe because they have a race condition between checking to see whether they should run the continuation and the other side making its modification. This diff introduces a `std::atomic_flag` variable to avoid the race and make these threadsafe.

NB Making `Future`/`Promise` threadsafe does not make this pattern safe:

f1.via(...).then(...).then(...)

(In a hypothetical world where `Future::via` is another name for `executeWithSameThread`)
There is a race between the future from `via` and the call of `then` (and between the first and second `then`s), so the `then`s could execute in the far thread as intended, or they could execute in the current thread (probably not what was intended). This diff does not solve that semantic problem of which thread this runs in. But it does make it safer, in that all those continuations will always execute, just maybe not in the thread you had intended.

You may ask what is the benefit if that semantic problem still exists? That's up for debate. I think the safety is worth it: at least everything a n00b throws at it will *work*. The question of thread semantics can iterate. If we end up with thread semantics that really don't need locks (e.g. maybe because we move to an Rx/Later style "launch" at the end of a chain) we can always toss this atomic.

How does this affect performance? I'm doing some experiments and going to write up my findings. Early results indicate that outlier performance is impacted but not by a lot, but don't count those chickens until I hatch them. Part of the reason for sending this diff out is to run windtunnel experiments.

Test Plan: tests, benchmark, close inspection

Reviewed By: davejwatson@fb.com

FB internal diff: D1241822

Fix TIMED_SYNCHRONIZED_CONST bug

Summary:
TIMED_SYNCHRONIZED_CONST is acquiring a read-write lock but releasing a shared lock.  Chaos ensues.
@override-unit-failures

Test Plan: Add unit test, test times out before fix, passes after fix.

Reviewed By: andrei.alexandrescu@fb.com

FB internal diff: D1251518

Blame Revision: D327492

Add ability to silence callbacks for Subprocess::communicate

Summary:
Subprocess::communicate callbacks are level-triggered, which makes writing
"chatty" communication protocols difficult -- you often want to silence the
write callback until you read the expected message. Add
Subprocess::enableNotifications() for this purpose.

Test Plan: test added

Reviewed By: lucian@fb.com

FB internal diff: D1251564

@override-unit-failures

Add loopOnce() method to folly::EventBase

Summary: Add loopOnce() method which provides functionality similar to event_base_loop(base, EVLOOP_ONCE);

Test Plan: fbmake

Reviewed By: simpkins@fb.com

FB internal diff: D1249753

Utility that converts from prettyPrint format back to double (e.g. 10M = 10 000 000)

Summary: Utility is doing reverse of prettyPrint. This is a first sketch.

Test Plan:
1) We can reverse all tests from prettyPrint test and use for our function. For now I just reversed several of them
2) We can also test prettyPrint and prettyToDouble together by checking whether prettyToDouble(prettyPrint(X)) == X. This is implemented.

Reviewed By: marcelo.juchem@fb.com

FB internal diff: D1159879

(Folly/FBString) Use a lambda to throw an exception for NULL input

Summary:
This should be standard compliant. `__PRETTY_FUNCTION__` seems
unnecessary, it just prints `basic_fbstring`, might as well use that and
simplify the code.

Test Plan:
`fbconfig --platform-all=gcc-4.8.1-glibc-2.17-fb folly/test`
`fbmake runtests_dbg`
`fbmake runtests_opt`

`fbconfig --platform-all=gcc-4.7.1-glibc-2.14.1 folly/test`
`fbmake runtests_dbg`
`fbmake runtests_opt`

`fbconfig --clang folly/test`
`...`

Reviewed By: andrei.alexandrescu@fb.com

FB internal diff: D1243670

NULL -> nullptr

Summary:
Change NULLs to nullptrs.

Facebook:
I was tired of seeing lint errors for NULL -> nullptr, so I
fixed it.

How: modified flint's tokenizer to output lines of the form

sed -i '__LINE__s|\<NULL\>|nullptr|g' __FILE__

for each NULL token encountered.

Ran the sed lines, restricted to filepaths with extensions
h,hpp,cc,cpp,tcc. Did not apply to FacebookUpdate, due to weird
formatting; hphp, to be done in a separate diff; payment, due to a large
number of generated text which didn't play well with my flow; and
memcache, which has lots of .h's that cannot contain nullptr's because
they are included from .c's.

@bypass-lint

Test Plan:
fbconfig -r common folly && fbmake opt && fbmake runtests_opt
^ of the 4k+ test cases, about two dozen are failing. Slightly more
failed in master than with my diff applied.

arc unit

Reviewed By: andrei.alexandrescu@fb.com

FB internal diff: D1234261

Range::contains(x)

Test Plan: Unit tests

Reviewed By: marcelo.juchem@fb.com

FB internal diff: D1236630

Optionally specify page size in MemoryMapping ctor

Summary:
For proper rounding, and also so memOpInChunks doesn't try to mlock / munlock /
madvise / munmap in chunks smaller than the page size.

Test Plan: memory_mapping_test

Reviewed By: lucian@fb.com

FB internal diff: D1232153

(Wangle) Sanity check for the small_vector test

Summary:
Let's double check that `small_vector` is doing the right thing. So
it doesn't just accidentally work.

Test Plan:
`fbconfig folly/wangle && fbmake runtests`
`fbconfig --clang folly/wangle && fbmake runtests`

Reviewed By: marccelani@fb.com

FB internal diff: D1231360

EventBase cleanup

Summary: Moar c++11 features.  marccelani's comments in D1195393, but in a separate diff.

Test Plan:
contbuild
fbconfig thrift/lib/cpp/test/
fbmake runtests

Reviewed By: rajat@fb.com

FB internal diff: D1219158

Performance Improvement for AsciiCaseInsensitive

Summary:
This implementation improves the performance of AsciiCaseInsensitive.
AsciiCaseInsensitive uses toupper, and it's slow.
Since toupper only works for ascii characters and this function does exactly the same thing over the range of [0..256), we can gain 3x performance here.

Test Plan: unit test and benchmark

Reviewed By: lucian@fb.com

FB internal diff: D1217596

@override-unit-failures

(Wangle) Fix tests for clang

Summary: `std::async` doesn't play nicely with our clang. Replace it with `std::thread`.

Test Plan: `fbconfig --clang folly/wangle && fbmake runtests`

Reviewed By: marccelani@fb.com

FB internal diff: D1230197

Harden usingJEMalloc()

Summary:
In a shared library, we're using whichever memory allocator the main program
uses, but we may still believe we're using jemalloc if we load libjemalloc.so
(because rallocm is defined). This means we'll pass to rallocm pointers that
were not allocated by jemalloc, which is bad.

Test Plan: tests added

Reviewed By: ngbronson@fb.com

FB internal diff: D1228483

range(), for making Range<T*> from arrays and std::vector

Summary: So people can more easily write functions which take contiguous values from memory.

Test Plan: Unit tests

Reviewed By: tudorb@fb.com

FB internal diff: D1217809

Add constexpr specifier for small_vector::max_size

Summary: Now we could use small_vector::max_size in constant expressions

Test Plan: unittest

Reviewed By: delong.j@fb.com

FB internal diff: D1224274

Unbreak Symbolizer with small ElfCache

Test Plan: fbconfig -r folly/experimental/symbolizer && fbmake runtests_opt && fbmake runtests

Reviewed By: simpkins@fb.com

FB internal diff: D1224552

@override-unit-failures

Move commited bits out of folly

Summary: Preparing to land the other diffs. No code changes here, just moving files around.

Test Plan: fbconfig bistro/common/cron/test bistro/common/cron/utils/test ; fbmake runtests

Reviewed By: yinwang@fb.com

FB internal diff: D1225159

sorted_vector_types have move inserts

Summary: sorted_vector_types were missing move inserts that come with std::map in c++11. This prevents me from using move iterators in folly::merge use case.

Test Plan: unit tests

Reviewed By: delong.j@fb.com

FB internal diff: D1223426

folly::merge() - std::merge() with stronger guarantees (probably same implementation in practice)

Summary:
std::merge() does not guarantee the ordering when equal elements belong in two ranges(comparator(it_a, it_b) == comparator(it_b, it_a) == 0). For maps, it is important that we can specify the ordering (see array_merge in php, where we guarantee which array's value will be present in the output if a key is present in both inputs).

Also removes folly::merge that is specfic for sorted_vector_map since this will not be needed. NOTE: I expect this to break feed, will fix in a separate non-folly diff.

Test Plan: This implementation is directly ripped from cppreference.com, but unit tests added none-the-less. Specifically, one is added where the output is a std::map to demonstrate its usefulness.

Reviewed By: delong.j@fb.com

FB internal diff: D1223401

@override-unit-failures

sorted_vector_types does not work with std::inserter

Summary:
The signature of insert(hint, const val& v) is wrong. It should return an iterator, not pair<iterator, bool>. This breaks std::inserter, making it impossible to use this for std::merge. I plan to create folly::merge (std::merge with stronger semantics when equal values are present in the two input ranges), so I need this fixed.

Given that the documentation claims this is a "drop in replacement" for std::map, I figure we should fix this.

Test Plan: reran unit tests

Reviewed By: delong.j@fb.com

FB internal diff: D1223396

Introduce a merge() function for folly::sorted_vector_map

Summary:
Feed spends a non trivial amount of time merging two sorted_vector_maps. Writing code for this efficiently is actually a little tricky, and its much easier to maintain the simpler code. This adds merge() to folly so that its easy for feed and fast. Benchmarks with large input sizes show this is a gigantic win (moving from O(n^2) to O(n).

Test Plan: unit tests, benchmark

Reviewed By: delong.j@fb.com

FB internal diff: D1221725

@override-unit-failures

Make sure folly::dynamic arrays can be used with folly::gen

Summary:
Define a value_type for folly::dynamic to make arrays work with
folly::gen::from.

Test Plan:
- added new unit test
- fbconfig folly/test && fbmake runtests

Reviewed By: tjackson@fb.com

FB internal diff: D1216358

Move thrift/lib/cpp/async to folly.

Summary:
Move the minimum amount of stuff and still have everything compile.  Would like to move TAsyncSocket/ServerSocket/SSL/UDP eventually, but not this round.

thrift async is used very widely now: thrift, proxygen, newer mysql async interface, even trying it out in memcache.  A common complaint is that it doesn't get wide enough notice under thrift/, so let's move it to folly/.  After moving TAsyncSocket, both HHVM and proxygen could avoid a dep on thrift as well.

Changes:
* mv files to folly/io/async
* remove 'T' prefix on classes/filenames
* change namespace to 'folly'
* remove any thrift references.

Tried this before in D470080, this time doesn't attempt to abstract libevent

@override-unit-failures

Test Plan:
fbconfig -r thrift; fbmake dev.
Will iterate on any other contbuild failures

Reviewed By: pgriess@fb.com

FB internal diff: D1195393

update FOLLY_IS_TRIVIALLY_COPYABLE

Summary:
Let's make sure that boost version is intented to mean exactly
the same as `std::is_trivially_copyable` (see 9 [class] / 6) to avoid any
confusion (current boost path is more close to
`std::is_trivially_copy_constructible` than to
`std::is_trivially_copyable`;  UPD: unfortunately, old versions of boost
don't support `boost::has_trivial_move_constructor` and friends, so I
can't completely mimic `std::is_trivially_copyable` here).

As mentioned in the original comment, `__has_trivial_copy` returns 1 in
case of deleted copy ctor in clang (contradicts 12.8 [class.copy] / 13 +
8.4.2 [dcl.fct.def.default] / 4), which makes `FOLLY_IS_TRIVIALLY_COPYABLE`
accept `std::unique_ptr<>`; using `boost::has_trivial_destructor` would
at least save us from the problems in this case.

Alternative "solution" may be to rename `FOLLY_IS_TRIVIALLY_COPYABLE` to
`FOLLY_IS_TRIVIALLY_COPY_CONSTRUCTIBLE`, and make sure
`folly::small_vector` won't call dtor when memcopies the data around.

Test Plan: fbconfig --clang folly/test:small_vector_test && fbmake runtests_opt (fails in trunk, passes with this diff)

Reviewed By: pgriess@fb.com

FB internal diff: D1216158

Add control of floating point serialization to JSON serialization

Summary: Title.  Note that this is a no-op with default parameters because folly::toAppend(double, &dest) calls folly::toAppend(double, &dest, DtoaMode, numDigits) with DtoaMode = SHORTEST and numDigits = 0.

Test Plan: Tested new functionality in D1212547.

Reviewed By: kelarini@fb.com

FB internal diff: D1212617

fix Cursor's copy constructor

Summary:
The Cursor class didn't provide a copy constructor accepting a
"const Cursor&".  The existing constructor only accepted references to
non-const Cursor.

This updates the constructor to also accept "const Cursor&".  I also
made the template parameter checking more strict, so that it only
accepts other CursorBase classes.  This prevents the compiler from
thinking that a Cursor can be constructed from any other possible type
when performing overload resolution.

Test Plan: Ran all of the folly cursor tests.

Reviewed By: davejwatson@fb.com

FB internal diff: D1208428

Support numeric types as targets for folly::split

Summary:
This extends the fixed version of folly::split to support numeric types
as targets in addition to string pieces. I was almost certain this was
already possible when I recently reviewed some code that did

folly::StringPiece source, target, strFrequency;
int frequency;
if (folly::split('\t', line, source, target, strFrequency) &&
(frequency = folly::to<unsigned int>(strFrequency)) > 0)

and was about to suggest changing the above into:

folly::StringPiece source, target;
int frequency;
if (folly::split('\t', line, source, target, frequency) && frequency > 0)

I double checked and saw that only splitting to string pieces was supported.

In the meantime I came across this pattern again and decided to just make
it work because it's really convenient.

The implementation should be fully backwards compatible.

Test Plan:
- New unit tests
- fbconfig -r folly && fbmake runtests
- Applied to github release, ./configure && make check

Reviewed By: andrei.alexandrescu@fb.com

FB internal diff: D1187004

Add setPort() to folly::URI class.

Summary:
Add setPort() method to URI class for easy port replacement in
URIs.

Facebook:
Perform port replacement in WriteProcessor.cpp in a way that
supports URI schemes. Add setPort() method to URI class to replace
the port number in the authority portion of the URI.

Test Plan: Test in debugger for dfsrouter changes.

Reviewed By: tudorb@fb.com

FB internal diff: D1209120

folly/experimental/Bits.h + ASan

Summary: Allow disabling ASan for `folly::Bits`.

Test Plan: [fb-only]

Reviewed By: soren@fb.com

FB internal diff: D1208584

Fix comment

Summary: This should be 'docs' not 'doc'. Too bad now it exceeds 80 chars :(.

Test Plan: n/a

Reviewed By: simpkins@fb.com

FB internal diff: D1209738

@override-unit-failures

(Wangle) Support .then(function pointer) syntax

Summary:
Support `.then(&static_function)`, `.then(&static_member_function)`, `.then(&Class::static_member_function)`, `.then(instance, &Class::member_function)` in Wangle.

C++ does not allow `.then(instance, &member_function)`, sadly.

This implementation just creates the closure for you and defers to the existing `then` implementations.

Test Plan: Added a test.

Reviewed By: hans@fb.com

FB internal diff: D1204954

use 0666 as the default permissions for new files

Summary: Seems like there is no real need to restrict permissions by default?

Test Plan: fbconfig -r folly/test && fbmake runtests_opt

@override-unit-failures

Reviewed By: tudorb@fb.com

FB internal diff: D1205527

(Wangle) Remove some early returns in tests

Summary:
I guess they sneaked in when testing.

Also remove an `EXPECT_NE` that depended on `std::move` actually moving.

Test Plan: All the tests.

Reviewed By: hans@fb.com

FB internal diff: D1204971