diff --git a/folly/container/detail/F14Policy.h b/folly/container/detail/F14Policy.h index eb5bb4bb32a82f4e3754ad5b1b9ecb3be64773b5..358ed0a67fbd5ea048f28005c8285285426f5202 100644 --- a/folly/container/detail/F14Policy.h +++ b/folly/container/detail/F14Policy.h @@ -529,9 +529,13 @@ class ValueContainerPolicy : public BasePolicy< void constructValueAtItem(std::size_t /*size*/, Item* itemAddr, Args&&... args) { Alloc& a = this->alloc(); - // This assume helps GCC and MSVC avoid a null-check in the subsequent - // placement new. GCC >= 6 and clang seem to figure it out themselves. - // MSVC as of 19 2017 still has the issue. + // GCC < 6 doesn't use the fact that itemAddr came from a reference + // to avoid a null-check in the placement new. folly::assume-ing it + // here gets rid of that branch. The branch is very predictable, + // but spoils some further optimizations. All clang versions that + // compile folly seem to be okay. + // + // TODO(T31574848): clean up assume-s used to optimize placement new assume(itemAddr != nullptr); AllocTraits::construct(a, itemAddr, std::forward<Args>(args)...); } @@ -772,9 +776,11 @@ class NodeContainerPolicy void constructValueAtItem(std::size_t /*size*/, Item* itemAddr, Args&&... args) { Alloc& a = this->alloc(); + // TODO(T31574848): clean up assume-s used to optimize placement new assume(itemAddr != nullptr); new (itemAddr) Item{AllocTraits::allocate(a, 1)}; auto p = std::addressof(**itemAddr); + // TODO(T31574848): clean up assume-s used to optimize placement new assume(p != nullptr); AllocTraits::construct(a, p, std::forward<Args>(args)...); } @@ -782,6 +788,7 @@ class NodeContainerPolicy void moveItemDuringRehash(Item* itemAddr, Item& src) { // This is basically *itemAddr = src; src = nullptr, but allowing // for fancy pointers. + // TODO(T31574848): clean up assume-s used to optimize placement new assume(itemAddr != nullptr); new (itemAddr) Item{std::move(src)}; src = nullptr; @@ -1110,6 +1117,7 @@ class VectorContainerPolicy : public BasePolicy< Alloc& a = this->alloc(); *itemAddr = size; auto dst = std::addressof(values_[size]); + // TODO(T31574848): clean up assume-s used to optimize placement new assume(dst != nullptr); AllocTraits::construct(a, dst, std::forward<Args>(args)...); } @@ -1142,6 +1150,7 @@ class VectorContainerPolicy : public BasePolicy< std::memcpy(dst, src, n * sizeof(Value)); } else { for (std::size_t i = 0; i < n; ++i, ++src, ++dst) { + // TODO(T31574848): clean up assume-s used to optimize placement new assume(dst != nullptr); AllocTraits::construct(a, dst, Super::moveValue(*src)); if (kIsMap) { @@ -1167,6 +1176,7 @@ class VectorContainerPolicy : public BasePolicy< } else { for (std::size_t i = 0; i < size; ++i, ++src, ++dst) { try { + // TODO(T31574848): clean up assume-s used to optimize placement new assume(dst != nullptr); AllocTraits::construct(a, dst, constructorArgFor(*src)); } catch (...) { diff --git a/folly/container/detail/F14Table.h b/folly/container/detail/F14Table.h index ed524c810bf9a97a50785e66d56f02197083a5ac..93cf106832b14d370ab545a9129452edbbe5e43e 100644 --- a/folly/container/detail/F14Table.h +++ b/folly/container/detail/F14Table.h @@ -1526,7 +1526,6 @@ class F14Table : public Policy { void insertAtBlank(ItemIter pos, HashPair hp, Args&&... args) { try { auto dst = pos.itemAddr(); - assume(dst != nullptr); this->constructValueAtItem(size(), dst, std::forward<Args>(args)...); } catch (...) { eraseBlank(pos, hp); @@ -1628,7 +1627,6 @@ class F14Table : public Policy { auto&& srcArg = std::forward<T>(src).buildArgForItem(srcChunk->item(srcI)); auto dst = dstChunk->itemAddr(dstI); - assume(dst != nullptr); this->constructValueAtItem( 0, dst, std::forward<decltype(srcArg)>(srcArg)); dstChunk->setTag(dstI, srcChunk->tag(srcI));