From 4d73eae51d0310546d1aa9f0a6e1fb2da9e45684 Mon Sep 17 00:00:00 2001 From: Enrico Guiraud Date: Thu, 12 Nov 2020 00:11:45 +0100 Subject: [PATCH 01/51] [VecOps] Remove testing of std::vector> RVec 2.0 will not be based on RAdoptAllocator anymore. --- math/vecops/test/vecops_rvec.cxx | 103 ------------------------------- 1 file changed, 103 deletions(-) diff --git a/math/vecops/test/vecops_rvec.cxx b/math/vecops/test/vecops_rvec.cxx index 8f23ac9f4f787..76caee7cfd9d4 100644 --- a/math/vecops/test/vecops_rvec.cxx +++ b/math/vecops/test/vecops_rvec.cxx @@ -496,109 +496,6 @@ void CheckEq(const T0 &v, const T0 &ref) } } -TEST(VecOps, InputOutputImpl) -{ - auto filename = "vecops_inputoutput_impl.root"; - auto treename = "t"; - - const ROOT::VecOps::RVec::Impl_t dref {1., 2., 3.}; - const ROOT::VecOps::RVec::Impl_t fref {1.f, 2.f, 3.f}; - const ROOT::VecOps::RVec::Impl_t uiref {1, 2, 3}; - const ROOT::VecOps::RVec::Impl_t ulref {1UL, 2UL, 3UL}; - const ROOT::VecOps::RVec::Impl_t ullref {1ULL, 2ULL, 3ULL}; - const ROOT::VecOps::RVec::Impl_t usref {1, 2, 3}; - const ROOT::VecOps::RVec::Impl_t ucref {1, 2, 3}; - const ROOT::VecOps::RVec::Impl_t iref {1, 2, 3};; - const ROOT::VecOps::RVec::Impl_t lref {1UL, 2UL, 3UL};; - const ROOT::VecOps::RVec::Impl_t llref {1ULL, 2ULL, 3ULL}; - const ROOT::VecOps::RVec::Impl_t sref {1, 2, 3}; - const ROOT::VecOps::RVec::Impl_t cref {1, 2, 3}; - const ROOT::VecOps::RVec::Impl_t bref {true, false, true}; - - { - auto d = dref; - auto f = fref; - auto ui = uiref; - auto ul = ulref; - auto ull = ullref; - auto us = usref; - auto uc = ucref; - auto i = iref; - auto l = lref; - auto ll = llref; - auto s = sref; - auto c = cref; - auto b = bref; - TFile file(filename, "RECREATE"); - TTree t(treename, treename); - t.Branch("d", &d); - t.Branch("f", &f); - t.Branch("ui", &ui); - t.Branch("ul", &ul); - t.Branch("ull", &ull); - t.Branch("us", &us); - t.Branch("uc", &uc); - t.Branch("i", &i); - t.Branch("l", &l); - t.Branch("ll", &ll); - t.Branch("s", &s); - t.Branch("c", &c); - t.Branch("b", &b); - t.Fill(); - t.Write(); - } - - auto d = new ROOT::VecOps::RVec::Impl_t(); - auto f = new ROOT::VecOps::RVec::Impl_t; - auto ui = new ROOT::VecOps::RVec::Impl_t(); - auto ul = new ROOT::VecOps::RVec::Impl_t(); - auto ull = new ROOT::VecOps::RVec::Impl_t(); - auto us = new ROOT::VecOps::RVec::Impl_t(); - auto uc = new ROOT::VecOps::RVec::Impl_t(); - auto i = new ROOT::VecOps::RVec::Impl_t(); - auto l = new ROOT::VecOps::RVec::Impl_t(); - auto ll = new ROOT::VecOps::RVec::Impl_t(); - auto s = new ROOT::VecOps::RVec::Impl_t(); - auto c = new ROOT::VecOps::RVec::Impl_t(); - auto b = new ROOT::VecOps::RVec::Impl_t(); - - TFile file(filename); - TTree *tp; - file.GetObject(treename, tp); - auto &t = *tp; - - t.SetBranchAddress("d", &d); - t.SetBranchAddress("f", &f); - t.SetBranchAddress("ui", &ui); - t.SetBranchAddress("ul", &ul); - t.SetBranchAddress("ull", &ull); - t.SetBranchAddress("us", &us); - t.SetBranchAddress("uc", &uc); - t.SetBranchAddress("i", &i); - t.SetBranchAddress("l", &l); - t.SetBranchAddress("ll", &ll); - t.SetBranchAddress("s", &s); - t.SetBranchAddress("c", &c); - t.SetBranchAddress("b", &b); - - t.GetEntry(0); - CheckEq(*d, dref); - CheckEq(*f, fref); - CheckEq(*d, dref); - CheckEq(*f, fref); - CheckEq(*d, dref); - CheckEq(*f, fref); - CheckEq(*d, dref); - CheckEq(*f, fref); - CheckEq(*d, dref); - CheckEq(*f, fref); - CheckEq(*b, bref); - - gSystem->Unlink(filename); - -} - - TEST(VecOps, InputOutput) { auto filename = "vecops_inputoutput.root"; From 366c5f63444dfc22275cdd300502aec0f310f28d Mon Sep 17 00:00:00 2001 From: Enrico Guiraud Date: Thu, 12 Nov 2020 00:55:54 +0100 Subject: [PATCH 02/51] [VecOps] Do not build dicts for vector> RVec 2.0 will not be implemented in terms of RAdoptAllocator. --- math/vecops/inc/LinkDef.h | 19 +------------------ 1 file changed, 1 insertion(+), 18 deletions(-) diff --git a/math/vecops/inc/LinkDef.h b/math/vecops/inc/LinkDef.h index 2f1d652e29a13..5a03297c31c9c 100644 --- a/math/vecops/inc/LinkDef.h +++ b/math/vecops/inc/LinkDef.h @@ -1,5 +1,5 @@ /************************************************************************* - * Copyright (C) 1995-2018, Rene Brun and Fons Rademakers. * + * Copyright (C) 1995-2020, Rene Brun and Fons Rademakers. * * All rights reserved. * * * * Author: Danilo Piparo - CERN * @@ -28,21 +28,4 @@ #pragma link C++ class ROOT::VecOps::RVec-; #pragma link C++ class ROOT::VecOps::RVec-; -#pragma link C++ class ROOT::VecOps::RVec::Impl_t+; - -#pragma link C++ class ROOT::VecOps::RVec::Impl_t+; -#pragma link C++ class ROOT::VecOps::RVec::Impl_t+; - -#pragma link C++ class ROOT::VecOps::RVec::Impl_t+; -#pragma link C++ class ROOT::VecOps::RVec::Impl_t+; -#pragma link C++ class ROOT::VecOps::RVec::Impl_t+; -#pragma link C++ class ROOT::VecOps::RVec::Impl_t+; -#pragma link C++ class ROOT::VecOps::RVec::Impl_t+; - -#pragma link C++ class ROOT::VecOps::RVec::Impl_t+; -#pragma link C++ class ROOT::VecOps::RVec::Impl_t+; -#pragma link C++ class ROOT::VecOps::RVec::Impl_t+; -#pragma link C++ class ROOT::VecOps::RVec::Impl_t+; -#pragma link C++ class ROOT::VecOps::RVec::Impl_t+; - #endif From 43608a2d056eacf75a6ea21ac5421e79b121c825 Mon Sep 17 00:00:00 2001 From: Enrico Guiraud Date: Thu, 12 Nov 2020 00:11:08 +0100 Subject: [PATCH 03/51] [VecOps] Base RVec implementation on LLVM's SmallVector The new implementation introduces a small buffer optimization and is not based on RAdoptAllocator anymore, but rather on LLVM's SmallVector. On top of SmallVector, RVec adds (or better keeps) the option to adopt an existing memory buffer. Some std::vector methods are also added to SmallVector for backwards compatibility of RVec. This commit also removes the deprecated `RVec::AsVector`, which we cannot implement with the same zero-copy semantics now that RVec is not implemented in terms of a std::vector. --- math/vecops/inc/ROOT/RVec.hxx | 1181 +++++++++++++++++++++++++++++---- math/vecops/src/RVec.cxx | 92 +++ 2 files changed, 1143 insertions(+), 130 deletions(-) diff --git a/math/vecops/inc/ROOT/RVec.hxx b/math/vecops/inc/ROOT/RVec.hxx index cf91e525d1112..5ecfc3db9dd0d 100644 --- a/math/vecops/inc/ROOT/RVec.hxx +++ b/math/vecops/inc/ROOT/RVec.hxx @@ -1,16 +1,33 @@ -// Author: Enrico Guiraud, Enric Tejedor, Danilo Piparo CERN 01/2018 +// Author: Enrico Guiraud, Enric Tejedor, Danilo Piparo CERN 04/2021 /************************************************************************* - * Copyright (C) 1995-2018, Rene Brun and Fons Rademakers. * + * Copyright (C) 1995-2021, Rene Brun and Fons Rademakers. * * All rights reserved. * * * * For the licensing terms see $ROOTSYS/LICENSE. * * For the list of contributors see $ROOTSYS/README/CREDITS. * *************************************************************************/ +/* + * The implementation of RVec is based in LLVM's SmallVector. + * + * The ability to adopt an existing memory buffer has been added, + * as well as other minor patches for backward compatibility with + * the previous implementation of RVec (which did not have a small buffer + * optimization and was based on std::vector with a custom allocator) and + * for compatibility with cppyy. + * RVec also adds a vectorized operator[] and many "numpy-like" helper functions. + */ + #ifndef ROOT_RVEC #define ROOT_RVEC +#if __cplusplus > 201402L +#define RVEC_NODISCARD [[nodiscard]] +#else +#define RVEC_NODISCARD +#endif + #ifdef _WIN32 #ifndef M_PI #ifndef _USE_MATH_DEFINES @@ -31,7 +48,10 @@ #include #include +#include +#include #include // for inner_product +#include #include #include #include @@ -97,27 +117,6 @@ auto MapFromTuple(Tuple_t &&t, std::index_sequence) } } -namespace Internal { -namespace VecOps { - -// We use this helper to workaround a limitation of compilers such as -// gcc 4.8 amd clang on osx 10.14 for which std::vector::emplace_back -// is not defined. -template -void EmplaceBack(T &v, Args &&... args) -{ - v.emplace_back(std::forward(args)...); -} - -template -void EmplaceBack(std::vector &v, Args &&... args) -{ - v.push_back(std::forward(args)...); -} - -} // End of VecOps NS -} // End of Internal NS - namespace VecOps { // Note that we open here with @{ the Doxygen group vecops and it is @@ -280,160 +279,1082 @@ hpt->Draw(); **/ // clang-format on + +//FIXME put things in the namespace they belong to + +/// Returns the next power of two (in 64-bits) that is strictly greater than A. +/// Returns zero on overflow. +inline uint64_t NextPowerOf2(uint64_t A) +{ + A |= (A >> 1); + A |= (A >> 2); + A |= (A >> 4); + A |= (A >> 8); + A |= (A >> 16); + A |= (A >> 32); + return A + 1; +} + +/// This is all the stuff common to all SmallVectors. +/// +/// The template parameter specifies the type which should be used to hold the +/// Size and Capacity of the SmallVector, so it can be adjusted. +/// Using 32 bit size is desirable to shrink the size of the SmallVector. +/// Using 64 bit size is desirable for cases like SmallVector, where a +/// 32 bit size would limit the vector to ~4GB. SmallVectors are used for +/// buffering bitcode output - which can exceed 4GB. +template +class SmallVectorBase { +protected: + void *BeginX; + Size_T fSize = 0, Capacity; + bool Owns = true; + + /// The maximum value of the Size_T used. + static constexpr size_t SizeTypeMax() { return std::numeric_limits::max(); } + + SmallVectorBase() = delete; + SmallVectorBase(void *FirstEl, size_t TotalCapacity) : BeginX(FirstEl), Capacity(TotalCapacity) {} + + /// This is an implementation of the grow() method which only works + /// on POD-like data types and is out of line to reduce code duplication. + /// This function will report a fatal error if it cannot increase capacity. + void grow_pod(void *FirstEl, size_t MinSize, size_t TSize); + + /// Report that MinSize doesn't fit into this vector's size type. Throws + /// std::length_error or calls report_fatal_error. + static void report_size_overflow(size_t MinSize); + /// Report that this vector is already at maximum capacity. Throws + /// std::length_error or calls report_fatal_error. + static void report_at_maximum_capacity(); + +public: + size_t size() const { return fSize; } + size_t capacity() const { return Capacity; } + + RVEC_NODISCARD bool empty() const { return !fSize; } + + /// Set the array size to \p N, which the current array must have enough + /// capacity for. + /// + /// This does not construct or destroy any elements in the vector. + /// + /// Clients can use this in conjunction with capacity() to write past the end + /// of the buffer when they know that more elements are available, and only + /// update the size later. This avoids the cost of value initializing elements + /// which will only be overwritten. + void set_size(size_t N) + { + assert(N <= capacity()); + fSize = N; + } +}; + +template +using SmallVectorSizeType = typename std::conditional= 8, uint64_t, uint32_t>::type; + +/// Figure out the offset of the first element. +template +struct SmallVectorAlignmentAndSize { + alignas(SmallVectorBase>) char Base[sizeof(SmallVectorBase>)]; + alignas(T) char FirstEl[sizeof(T)]; +}; + +/// This is the part of SmallVectorTemplateBase which does not depend on whether +/// the type T is a POD. The extra dummy template argument is used by ArrayRef +/// to avoid unnecessarily requiring T to be complete. +template +class SmallVectorTemplateCommon : public SmallVectorBase> { + using Base = SmallVectorBase>; + + /// Find the address of the first element. For this pointer math to be valid + /// with small-size of 0 for T with lots of alignment, it's important that + /// SmallVectorStorage is properly-aligned even for small-size of 0. + void *getFirstEl() const + { + return const_cast(reinterpret_cast(reinterpret_cast(this) + + offsetof(SmallVectorAlignmentAndSize, FirstEl))); + } + // Space after 'FirstEl' is clobbered, do not add any instance vars after it. + +protected: + SmallVectorTemplateCommon(size_t Size) : Base(getFirstEl(), Size) {} + + void grow_pod(size_t MinSize, size_t TSize) { Base::grow_pod(getFirstEl(), MinSize, TSize); } + + /// Return true if this is a smallvector which has not had dynamic + /// memory allocated for it. + bool isSmall() const { return this->BeginX == getFirstEl(); } + + /// Put this vector in a state of being small. + void resetToSmall() + { + this->BeginX = getFirstEl(); + // from the original LLVM implementation: + // FIXME: Setting Capacity to 0 is suspect. + this->fSize = this->Capacity = 0; + } + +public: + using size_type = size_t; + using difference_type = ptrdiff_t; + using value_type = T; + using iterator = T *; + using const_iterator = const T *; + + using const_reverse_iterator = std::reverse_iterator; + using reverse_iterator = std::reverse_iterator; + + using reference = T &; + using const_reference = const T &; + using pointer = T *; + using const_pointer = const T *; + + using Base::capacity; + using Base::empty; + using Base::size; + + // forward iterator creation methods. + iterator begin() { return (iterator)this->BeginX; } + const_iterator begin() const { return (const_iterator)this->BeginX; } + iterator end() { return begin() + size(); } + const_iterator end() const { return begin() + size(); } + + // reverse iterator creation methods. + reverse_iterator rbegin() { return reverse_iterator(end()); } + const_reverse_iterator rbegin() const { return const_reverse_iterator(end()); } + reverse_iterator rend() { return reverse_iterator(begin()); } + const_reverse_iterator rend() const { return const_reverse_iterator(begin()); } + + size_type size_in_bytes() const { return size() * sizeof(T); } + size_type max_size() const { return std::min(this->SizeTypeMax(), size_type(-1) / sizeof(T)); } + + size_t capacity_in_bytes() const { return capacity() * sizeof(T); } + + /// Return a pointer to the vector's buffer, even if empty(). + pointer data() { return pointer(begin()); } + /// Return a pointer to the vector's buffer, even if empty(). + const_pointer data() const { return const_pointer(begin()); } + + reference operator[](size_type idx) + { + assert(idx < size()); + return begin()[idx]; + } + const_reference operator[](size_type idx) const + { + assert(idx < size()); + return begin()[idx]; + } + + reference front() + { + assert(!empty()); + return begin()[0]; + } + const_reference front() const + { + assert(!empty()); + return begin()[0]; + } + + reference back() + { + assert(!empty()); + return end()[-1]; + } + const_reference back() const + { + assert(!empty()); + return end()[-1]; + } +}; + +/// SmallVectorTemplateBase - This is where we put +/// method implementations that are designed to work with non-trivial T's. +/// +/// We approximate is_trivially_copyable with trivial move/copy construction and +/// trivial destruction. While the standard doesn't specify that you're allowed +/// copy these types with memcpy, there is no way for the type to observe this. +/// This catches the important case of std::pair, which is not +/// trivially assignable. +template ::value) && + (std::is_trivially_move_constructible::value) && + std::is_trivially_destructible::value> +class SmallVectorTemplateBase : public SmallVectorTemplateCommon { +protected: + SmallVectorTemplateBase(size_t Size) : SmallVectorTemplateCommon(Size) {} + + static void destroy_range(T *S, T *E) + { + while (S != E) { + --E; + E->~T(); + } + } + + /// Move the range [I, E) into the uninitialized memory starting with "Dest", + /// constructing elements as needed. + template + static void uninitialized_move(It1 I, It1 E, It2 Dest) + { + std::uninitialized_copy(std::make_move_iterator(I), std::make_move_iterator(E), Dest); + } + + /// Copy the range [I, E) onto the uninitialized memory starting with "Dest", + /// constructing elements as needed. + template + static void uninitialized_copy(It1 I, It1 E, It2 Dest) + { + std::uninitialized_copy(I, E, Dest); + } + + /// Grow the allocated memory (without initializing new elements), doubling + /// the size of the allocated memory. Guarantees space for at least one more + /// element, or MinSize more elements if specified. + void grow(size_t MinSize = 0); + +public: + void push_back(const T &Elt) + { + if (R__unlikely(this->size() >= this->capacity())) + this->grow(); + ::new ((void *)this->end()) T(Elt); + this->set_size(this->size() + 1); + } + + void push_back(T &&Elt) + { + if (R__unlikely(this->size() >= this->capacity())) + this->grow(); + ::new ((void *)this->end()) T(::std::move(Elt)); + this->set_size(this->size() + 1); + } + + void pop_back() + { + this->set_size(this->size() - 1); + this->end()->~T(); + } +}; + +// Define this out-of-line to dissuade the C++ compiler from inlining it. +template +void SmallVectorTemplateBase::grow(size_t MinSize) +{ + // Ensure we can fit the new capacity. + // This is only going to be applicable when the capacity is 32 bit. + if (MinSize > this->SizeTypeMax()) + this->report_size_overflow(MinSize); + + // Ensure we can meet the guarantee of space for at least one more element. + // The above check alone will not catch the case where grow is called with a + // default MinSize of 0, but the current capacity cannot be increased. + // This is only going to be applicable when the capacity is 32 bit. + if (this->capacity() == this->SizeTypeMax()) + this->report_at_maximum_capacity(); + + // Always grow, even from zero. + size_t NewCapacity = size_t(NextPowerOf2(this->capacity() + 2)); + NewCapacity = std::min(std::max(NewCapacity, MinSize), this->SizeTypeMax()); + T *NewElts = static_cast(malloc(NewCapacity * sizeof(T))); + + // Move the elements over. + this->uninitialized_move(this->begin(), this->end(), NewElts); + + if (this->Owns) { + // Destroy the original elements. + destroy_range(this->begin(), this->end()); + + // If this wasn't grown from the inline copy, deallocate the old space. + if (!this->isSmall()) + free(this->begin()); + } + + this->BeginX = NewElts; + this->Capacity = NewCapacity; + this->Owns = true; +} + +/// SmallVectorTemplateBase - This is where we put +/// method implementations that are designed to work with trivially copyable +/// T's. This allows using memcpy in place of copy/move construction and +/// skipping destruction. template -class RVec { - // Here we check if T is a bool. This is done in order to decide what type - // to use as a storage. If T is anything but bool, we use a vector>. - // If T is a bool, we opt for a plain vector otherwise we'll not be able - // to write the data type given the shortcomings of TCollectionProxy design. - static constexpr const auto IsVecBool = std::is_same::value; +class SmallVectorTemplateBase : public SmallVectorTemplateCommon { +protected: + SmallVectorTemplateBase(size_t Size) : SmallVectorTemplateCommon(Size) {} + + // No need to do a destroy loop for POD's. + static void destroy_range(T *, T *) {} + + /// Move the range [I, E) onto the uninitialized memory + /// starting with "Dest", constructing elements into it as needed. + template + static void uninitialized_move(It1 I, It1 E, It2 Dest) + { + // Just do a copy. + uninitialized_copy(I, E, Dest); + } + + /// Copy the range [I, E) onto the uninitialized memory + /// starting with "Dest", constructing elements into it as needed. + template + static void uninitialized_copy(It1 I, It1 E, It2 Dest) + { + // Arbitrary iterator types; just use the basic implementation. + std::uninitialized_copy(I, E, Dest); + } + + /// Copy the range [I, E) onto the uninitialized memory + /// starting with "Dest", constructing elements into it as needed. + template + static void uninitialized_copy( + T1 *I, T1 *E, T2 *Dest, + typename std::enable_if::type, T2>::value>::type * = nullptr) + { + // Use memcpy for PODs iterated by pointers (which includes SmallVector + // iterators): std::uninitialized_copy optimizes to memmove, but we can + // use memcpy here. Note that I and E are iterators and thus might be + // invalid for memcpy if they are equal. + if (I != E) + memcpy(reinterpret_cast(Dest), I, (E - I) * sizeof(T)); + } + + /// Double the size of the allocated memory, guaranteeing space for at + /// least one more element or MinSize if specified. + void grow(size_t MinSize = 0) + { + this->grow_pod(MinSize, sizeof(T)); + this->Owns = true; + } + +public: + void push_back(const T &Elt) + { + if (R__unlikely(this->size() >= this->capacity())) + this->grow(); + memcpy(reinterpret_cast(this->end()), &Elt, sizeof(T)); + this->set_size(this->size() + 1); + } + + void pop_back() { this->set_size(this->size() - 1); } +}; + +/// This class consists of common code factored out of the SmallVector class to +/// reduce code duplication based on the SmallVector 'N' template parameter. +template +class SmallVectorImpl : public SmallVectorTemplateBase { + using SuperClass = SmallVectorTemplateBase; + public: - using Impl_t = typename std::conditional, std::vector>>::type; - using value_type = typename Impl_t::value_type; - using size_type = typename Impl_t::size_type; - using difference_type = typename Impl_t::difference_type; - using reference = typename Impl_t::reference; - using const_reference = typename Impl_t::const_reference; - using pointer = typename Impl_t::pointer; - using const_pointer = typename Impl_t::const_pointer; - // The data_t and const_data_t types are chosen to be void in case T is a bool. - // This way we can as elegantly as in the STL return void upon calling the data() method. - using data_t = typename std::conditional::type; - using const_data_t = typename std::conditional::type; - using iterator = typename Impl_t::iterator; - using const_iterator = typename Impl_t::const_iterator; - using reverse_iterator = typename Impl_t::reverse_iterator; - using const_reverse_iterator = typename Impl_t::const_reverse_iterator; - -private: - Impl_t fData; + using iterator = typename SuperClass::iterator; + using const_iterator = typename SuperClass::const_iterator; + using reference = typename SuperClass::reference; + using size_type = typename SuperClass::size_type; + +protected: + // Default ctor - Initialize to empty. + explicit SmallVectorImpl(unsigned N) : SmallVectorTemplateBase(N) {} public: - // constructors - RVec() {} + SmallVectorImpl(const SmallVectorImpl &) = delete; + + ~SmallVectorImpl() + { + // Subclass has already destructed this vector's elements. + // If this wasn't grown from the inline copy, deallocate the old space. + if (!this->isSmall() && this->Owns) + free(this->begin()); + } + + // also give up adopted memory if applicable + void clear() + { + if (this->Owns) { + this->destroy_range(this->begin(), this->end()); + this->fSize = 0; + } else { + this->resetToSmall(); + } + } + + void resize(size_type N) + { + if (N < this->size()) { + if (this->Owns) + this->destroy_range(this->begin() + N, this->end()); + this->set_size(N); + } else if (N > this->size()) { + if (this->capacity() < N) + this->grow(N); + for (auto I = this->end(), E = this->begin() + N; I != E; ++I) + new (&*I) T(); + this->set_size(N); + } + } + + void resize(size_type N, const T &NV) + { + if (N < this->size()) { + if (this->Owns) + this->destroy_range(this->begin() + N, this->end()); + this->set_size(N); + } else if (N > this->size()) { + if (this->capacity() < N) + this->grow(N); + std::uninitialized_fill(this->end(), this->begin() + N, NV); + this->set_size(N); + } + } + + void reserve(size_type N) + { + if (this->capacity() < N) + this->grow(N); + } + + void pop_back_n(size_type NumItems) + { + assert(this->size() >= NumItems); + if (this->Owns) + this->destroy_range(this->end() - NumItems, this->end()); + this->set_size(this->size() - NumItems); + } - explicit RVec(size_type count) : fData(count) {} + RVEC_NODISCARD T pop_back_val() + { + T Result = ::std::move(this->back()); + this->pop_back(); + return Result; + } - RVec(size_type count, const T &value) : fData(count, value) {} + void swap(SmallVectorImpl &RHS); - RVec(const RVec &v) : fData(v.fData) {} + /// Add the specified range to the end of the SmallVector. + template ::iterator_category, std::input_iterator_tag>::value>::type> + void append(in_iter in_start, in_iter in_end) + { + size_type NumInputs = std::distance(in_start, in_end); + if (NumInputs > this->capacity() - this->size()) + this->grow(this->size() + NumInputs); - RVec(RVec &&v) : fData(std::move(v.fData)) {} + this->uninitialized_copy(in_start, in_end, this->end()); + this->set_size(this->size() + NumInputs); + } - RVec(const std::vector &v) : fData(v.cbegin(), v.cend()) {} + /// Append \p NumInputs copies of \p Elt to the end. + void append(size_type NumInputs, const T &Elt) + { + if (NumInputs > this->capacity() - this->size()) + this->grow(this->size() + NumInputs); - RVec(pointer p, size_type n) : fData(n, T(), ROOT::Detail::VecOps::RAdoptAllocator(p)) {} + std::uninitialized_fill_n(this->end(), NumInputs, Elt); + this->set_size(this->size() + NumInputs); + } - template - RVec(InputIt first, InputIt last) : fData(first, last) {} + void append(std::initializer_list IL) { append(IL.begin(), IL.end()); } - RVec(std::initializer_list init) : fData(init) {} + // from the original LLVM implementation: + // FIXME: Consider assigning over existing elements, rather than clearing & + // re-initializing them - for all assign(...) variants. - // assignment - RVec &operator=(const RVec &v) + void assign(size_type NumElts, const T &Elt) { - fData = v.fData; + clear(); + if (this->capacity() < NumElts) + this->grow(NumElts); + this->set_size(NumElts); + std::uninitialized_fill(this->begin(), this->end(), Elt); + } + + template ::iterator_category, std::input_iterator_tag>::value>::type> + void assign(in_iter in_start, in_iter in_end) + { + clear(); + append(in_start, in_end); + } + + void assign(std::initializer_list IL) + { + clear(); + append(IL); + } + + iterator erase(const_iterator CI) + { + // Just cast away constness because this is a non-const member function. + iterator I = const_cast(CI); + + assert(I >= this->begin() && "Iterator to erase is out of bounds."); + assert(I < this->end() && "Erasing at past-the-end iterator."); + + iterator N = I; + // Shift all elts down one. + std::move(I + 1, this->end(), I); + // Drop the last elt. + this->pop_back(); + return (N); + } + + iterator erase(const_iterator CS, const_iterator CE) + { + // Just cast away constness because this is a non-const member function. + iterator S = const_cast(CS); + iterator E = const_cast(CE); + + assert(S >= this->begin() && "Range to erase is out of bounds."); + assert(S <= E && "Trying to erase invalid range."); + assert(E <= this->end() && "Trying to erase past the end."); + + iterator N = S; + // Shift all elts down. + iterator I = std::move(E, this->end(), S); + // Drop the last elts. + if (this->Owns) + this->destroy_range(I, this->end()); + this->set_size(I - this->begin()); + return (N); + } + + iterator insert(iterator I, T &&Elt) + { + if (I == this->end()) { // Important special case for empty vector. + this->push_back(::std::move(Elt)); + return this->end() - 1; + } + + assert(I >= this->begin() && "Insertion iterator is out of bounds."); + assert(I <= this->end() && "Inserting past the end of the vector."); + + if (this->size() >= this->capacity()) { + size_t EltNo = I - this->begin(); + this->grow(); + I = this->begin() + EltNo; + } + + ::new ((void *)this->end()) T(::std::move(this->back())); + // Push everything else over. + std::move_backward(I, this->end() - 1, this->end()); + this->set_size(this->size() + 1); + + // If we just moved the element we're inserting, be sure to update + // the reference. + T *EltPtr = &Elt; + if (I <= EltPtr && EltPtr < this->end()) + ++EltPtr; + + *I = ::std::move(*EltPtr); + return I; + } + + iterator insert(iterator I, const T &Elt) + { + if (I == this->end()) { // Important special case for empty vector. + this->push_back(Elt); + return this->end() - 1; + } + + assert(I >= this->begin() && "Insertion iterator is out of bounds."); + assert(I <= this->end() && "Inserting past the end of the vector."); + + if (this->size() >= this->capacity()) { + size_t EltNo = I - this->begin(); + this->grow(); + I = this->begin() + EltNo; + } + ::new ((void *)this->end()) T(std::move(this->back())); + // Push everything else over. + std::move_backward(I, this->end() - 1, this->end()); + this->set_size(this->size() + 1); + + // If we just moved the element we're inserting, be sure to update + // the reference. + const T *EltPtr = &Elt; + if (I <= EltPtr && EltPtr < this->end()) + ++EltPtr; + + *I = *EltPtr; + return I; + } + + iterator insert(iterator I, size_type NumToInsert, const T &Elt) + { + // Convert iterator to elt# to avoid invalidating iterator when we reserve() + size_t InsertElt = I - this->begin(); + + if (I == this->end()) { // Important special case for empty vector. + append(NumToInsert, Elt); + return this->begin() + InsertElt; + } + + assert(I >= this->begin() && "Insertion iterator is out of bounds."); + assert(I <= this->end() && "Inserting past the end of the vector."); + + // Ensure there is enough space. + reserve(this->size() + NumToInsert); + + // Uninvalidate the iterator. + I = this->begin() + InsertElt; + + // If there are more elements between the insertion point and the end of the + // range than there are being inserted, we can use a simple approach to + // insertion. Since we already reserved space, we know that this won't + // reallocate the vector. + if (size_t(this->end() - I) >= NumToInsert) { + T *OldEnd = this->end(); + append(std::move_iterator(this->end() - NumToInsert), std::move_iterator(this->end())); + + // Copy the existing elements that get replaced. + std::move_backward(I, OldEnd - NumToInsert, OldEnd); + + std::fill_n(I, NumToInsert, Elt); + return I; + } + + // Otherwise, we're inserting more elements than exist already, and we're + // not inserting at the end. + + // Move over the elements that we're about to overwrite. + T *OldEnd = this->end(); + this->set_size(this->size() + NumToInsert); + size_t NumOverwritten = OldEnd - I; + this->uninitialized_move(I, OldEnd, this->end() - NumOverwritten); + + // Replace the overwritten part. + std::fill_n(I, NumOverwritten, Elt); + + // Insert the non-overwritten middle part. + std::uninitialized_fill_n(OldEnd, NumToInsert - NumOverwritten, Elt); + return I; + } + + template ::iterator_category, std::input_iterator_tag>::value>::type> + iterator insert(iterator I, ItTy From, ItTy To) + { + // Convert iterator to elt# to avoid invalidating iterator when we reserve() + size_t InsertElt = I - this->begin(); + + if (I == this->end()) { // Important special case for empty vector. + append(From, To); + return this->begin() + InsertElt; + } + + assert(I >= this->begin() && "Insertion iterator is out of bounds."); + assert(I <= this->end() && "Inserting past the end of the vector."); + + size_t NumToInsert = std::distance(From, To); + + // Ensure there is enough space. + reserve(this->size() + NumToInsert); + + // Uninvalidate the iterator. + I = this->begin() + InsertElt; + + // If there are more elements between the insertion point and the end of the + // range than there are being inserted, we can use a simple approach to + // insertion. Since we already reserved space, we know that this won't + // reallocate the vector. + if (size_t(this->end() - I) >= NumToInsert) { + T *OldEnd = this->end(); + append(std::move_iterator(this->end() - NumToInsert), std::move_iterator(this->end())); + + // Copy the existing elements that get replaced. + std::move_backward(I, OldEnd - NumToInsert, OldEnd); + + std::copy(From, To, I); + return I; + } + + // Otherwise, we're inserting more elements than exist already, and we're + // not inserting at the end. + + // Move over the elements that we're about to overwrite. + T *OldEnd = this->end(); + this->set_size(this->size() + NumToInsert); + size_t NumOverwritten = OldEnd - I; + this->uninitialized_move(I, OldEnd, this->end() - NumOverwritten); + + // Replace the overwritten part. + for (T *J = I; NumOverwritten > 0; --NumOverwritten) { + *J = *From; + ++J; + ++From; + } + + // Insert the non-overwritten middle part. + this->uninitialized_copy(From, To, OldEnd); + return I; + } + + void insert(iterator I, std::initializer_list IL) { insert(I, IL.begin(), IL.end()); } + + template + reference emplace_back(ArgTypes &&... Args) + { + if (R__unlikely(this->size() >= this->capacity())) + this->grow(); + ::new ((void *)this->end()) T(std::forward(Args)...); + this->set_size(this->size() + 1); + return this->back(); + } + + SmallVectorImpl &operator=(const SmallVectorImpl &RHS); + + SmallVectorImpl &operator=(SmallVectorImpl &&RHS); + + bool operator==(const SmallVectorImpl &RHS) const + { + if (this->size() != RHS.size()) + return false; + return std::equal(this->begin(), this->end(), RHS.begin()); + } + bool operator!=(const SmallVectorImpl &RHS) const { return !(*this == RHS); } + + bool operator<(const SmallVectorImpl &RHS) const + { + return std::lexicographical_compare(this->begin(), this->end(), RHS.begin(), RHS.end()); + } +}; + +template +void SmallVectorImpl::swap(SmallVectorImpl &RHS) +{ + if (this == &RHS) + return; + + // We can only avoid copying elements if neither vector is small. + if (!this->isSmall() && !RHS.isSmall()) { + std::swap(this->BeginX, RHS.BeginX); + std::swap(this->fSize, RHS.fSize); + std::swap(this->Capacity, RHS.Capacity); + std::swap(this->Owns, RHS.Owns); + return; + } + if (RHS.size() > this->capacity()) + this->grow(RHS.size()); + if (this->size() > RHS.capacity()) + RHS.grow(this->size()); + + // Swap the shared elements. + size_t NumShared = this->size(); + if (NumShared > RHS.size()) + NumShared = RHS.size(); + for (size_type i = 0; i != NumShared; ++i) + std::swap((*this)[i], RHS[i]); + + // Copy over the extra elts. + if (this->size() > RHS.size()) { + size_t EltDiff = this->size() - RHS.size(); + this->uninitialized_copy(this->begin() + NumShared, this->end(), RHS.end()); + RHS.set_size(RHS.size() + EltDiff); + if (this->Owns) + this->destroy_range(this->begin() + NumShared, this->end()); + this->set_size(NumShared); + } else if (RHS.size() > this->size()) { + size_t EltDiff = RHS.size() - this->size(); + this->uninitialized_copy(RHS.begin() + NumShared, RHS.end(), this->end()); + this->set_size(this->size() + EltDiff); + if (RHS.Owns) + this->destroy_range(RHS.begin() + NumShared, RHS.end()); + RHS.set_size(NumShared); + } +} + +template +SmallVectorImpl &SmallVectorImpl::operator=(const SmallVectorImpl &RHS) +{ + // Avoid self-assignment. + if (this == &RHS) + return *this; + + // If we already have sufficient space, assign the common elements, then + // destroy any excess. + size_t RHSSize = RHS.size(); + size_t CurSize = this->size(); + if (CurSize >= RHSSize) { + // Assign common elements. + iterator NewEnd; + if (RHSSize) + NewEnd = std::copy(RHS.begin(), RHS.begin() + RHSSize, this->begin()); + else + NewEnd = this->begin(); + + // Destroy excess elements. + if (this->Owns) + this->destroy_range(NewEnd, this->end()); + + // Trim. + this->set_size(RHSSize); return *this; } - RVec &operator=(RVec &&v) + // If we have to grow to have enough elements, destroy the current elements. + // This allows us to avoid copying them during the grow. + // From the original LLVM implementation: + // FIXME: don't do this if they're efficiently moveable. + if (this->capacity() < RHSSize) { + if (this->Owns) { + // Destroy current elements. + this->destroy_range(this->begin(), this->end()); + } + this->set_size(0); + CurSize = 0; + this->grow(RHSSize); + } else if (CurSize) { + // Otherwise, use assignment for the already-constructed elements. + std::copy(RHS.begin(), RHS.begin() + CurSize, this->begin()); + } + + // Copy construct the new elements in place. + this->uninitialized_copy(RHS.begin() + CurSize, RHS.end(), this->begin() + CurSize); + + // Set end. + this->set_size(RHSSize); + return *this; +} + +template +SmallVectorImpl &SmallVectorImpl::operator=(SmallVectorImpl &&RHS) +{ + // Avoid self-assignment. + if (this == &RHS) + return *this; + + // If the RHS isn't small, clear this vector and then steal its buffer. + if (!RHS.isSmall()) { + if (this->Owns) { + this->destroy_range(this->begin(), this->end()); + if (!this->isSmall()) + free(this->begin()); + } + this->BeginX = RHS.BeginX; + this->fSize = RHS.fSize; + this->Capacity = RHS.Capacity; + this->Owns = RHS.Owns; + RHS.resetToSmall(); + return *this; + } + + // If we already have sufficient space, assign the common elements, then + // destroy any excess. + size_t RHSSize = RHS.size(); + size_t CurSize = this->size(); + if (CurSize >= RHSSize) { + // Assign common elements. + iterator NewEnd = this->begin(); + if (RHSSize) + NewEnd = std::move(RHS.begin(), RHS.end(), NewEnd); + + // Destroy excess elements and trim the bounds. + if (this->Owns) + this->destroy_range(NewEnd, this->end()); + this->set_size(RHSSize); + + // Clear the RHS. + RHS.clear(); + + return *this; + } + + // If we have to grow to have enough elements, destroy the current elements. + // This allows us to avoid copying them during the grow. + // From the original LLVM implementation: + // FIXME: this may not actually make any sense if we can efficiently move + // elements. + if (this->capacity() < RHSSize) { + if (this->Owns) { + // Destroy current elements. + this->destroy_range(this->begin(), this->end()); + } + this->set_size(0); + CurSize = 0; + this->grow(RHSSize); + } else if (CurSize) { + // Otherwise, use assignment for the already-constructed elements. + std::move(RHS.begin(), RHS.begin() + CurSize, this->begin()); + } + + // Move-construct the new elements in place. + this->uninitialized_move(RHS.begin() + CurSize, RHS.end(), this->begin() + CurSize); + + // Set end. + this->set_size(RHSSize); + + RHS.clear(); + return *this; +} + +/// Storage for the SmallVector elements. This is specialized for the N=0 case +/// to avoid allocating unnecessary storage. +template +struct SmallVectorStorage { + alignas(T) char InlineElts[N * sizeof(T)]; +}; + +/// We need the storage to be properly aligned even for small-size of 0 so that +/// the pointer math in \a SmallVectorTemplateCommon::getFirstEl() is +/// well-defined. +template +struct alignas(T) SmallVectorStorage { +}; + +/// This is a 'vector' (really, a variable-sized array), optimized +/// for the case when the array is small. It contains some number of elements +/// in-place, which allows it to avoid heap allocation when the actual number of +/// elements is below that threshold. This allows normal "small" cases to be +/// fast without losing generality for large inputs. +/// +/// Note that this does not attempt to be exception safe. +/// +template +class RVec : public SmallVectorImpl, SmallVectorStorage { +public: + static constexpr unsigned N = 8; + RVec() : SmallVectorImpl(N) {} + + ~RVec() + { + if (this->Owns) { + // Destroy the constructed elements in the vector. + this->destroy_range(this->begin(), this->end()); + } + } + + explicit RVec(size_t Size, const T &Value = T()) : SmallVectorImpl(N) { this->assign(Size, Value); } + + template ::iterator_category, std::input_iterator_tag>::value>::type> + RVec(ItTy S, ItTy E) : SmallVectorImpl(N) + { + this->append(S, E); + } + + RVec(std::initializer_list IL) : SmallVectorImpl(N) { this->assign(IL); } + + RVec(const RVec &RHS) : SmallVectorImpl(N) + { + if (!RHS.empty()) + SmallVectorImpl::operator=(RHS); + } + + RVec &operator=(const RVec &RHS) { - std::swap(fData, v.fData); + SmallVectorImpl::operator=(RHS); return *this; } - RVec &operator=(std::initializer_list ilist) + RVec(RVec &&RHS) : SmallVectorImpl(N) { - fData = ilist; + if (!RHS.empty()) + SmallVectorImpl::operator=(::std::move(RHS)); + } + + RVec(SmallVectorImpl &&RHS) : SmallVectorImpl(N) + { + if (!RHS.empty()) + SmallVectorImpl::operator=(::std::move(RHS)); + } + + RVec(const std::vector &RHS) : RVec(RHS.begin(), RHS.end()) {} + + RVec &operator=(RVec &&RHS) + { + SmallVectorImpl::operator=(::std::move(RHS)); return *this; } - // conversion - template ::value>> - operator RVec() const + RVec(typename RVec::pointer p, typename RVec::size_type n) : SmallVectorImpl(N) { - RVec ret(size()); - std::copy(begin(), end(), ret.begin()); - return ret; + this->BeginX = p; + this->fSize = n; + this->Capacity = n; + this->Owns = false; } - // accessors - reference at(size_type pos) { return fData.at(pos); } - const_reference at(size_type pos) const { return fData.at(pos); } - /// No exception thrown. The user specifies the desired value in case the RVec is shorter than `pos`. - value_type at(size_type pos, value_type fallback) { return pos < fData.size() ? fData[pos] : fallback; } - /// No exception thrown. The user specifies the desired value in case the RVec is shorter than `pos`. - value_type at(size_type pos, value_type fallback) const { return pos < fData.size() ? fData[pos] : fallback; } - reference operator[](size_type pos) { return fData[pos]; } - const_reference operator[](size_type pos) const { return fData[pos]; } + RVec &operator=(SmallVectorImpl &&RHS) + { + SmallVectorImpl::operator=(::std::move(RHS)); + return *this; + } + + RVec &operator=(std::initializer_list IL) + { + this->assign(IL); + return *this; + } + + using SmallVectorTemplateCommon::operator[]; template ::value>> RVec operator[](const RVec &conds) const { - const size_type n = conds.size(); + const auto n = conds.size(); - if (n != size()) + if (n != this->size()) throw std::runtime_error("Cannot index RVec with condition vector of different size"); RVec ret; ret.reserve(n); - for (size_type i = 0; i < n; ++i) + for (auto i = 0u; i < n; ++i) if (conds[i]) - ret.emplace_back(fData[i]); + ret.emplace_back(this->operator[](i)); return ret; } - reference front() { return fData.front(); } - const_reference front() const { return fData.front(); } - reference back() { return fData.back(); } - const_reference back() const { return fData.back(); } - data_t data() noexcept { return fData.data(); } - const_data_t data() const noexcept { return fData.data(); } - // iterators - iterator begin() noexcept { return fData.begin(); } - const_iterator begin() const noexcept { return fData.begin(); } - const_iterator cbegin() const noexcept { return fData.cbegin(); } - iterator end() noexcept { return fData.end(); } - const_iterator end() const noexcept { return fData.end(); } - const_iterator cend() const noexcept { return fData.cend(); } - reverse_iterator rbegin() noexcept { return fData.rbegin(); } - const_reverse_iterator rbegin() const noexcept { return fData.rbegin(); } - const_reverse_iterator crbegin() const noexcept { return fData.crbegin(); } - reverse_iterator rend() noexcept { return fData.rend(); } - const_reverse_iterator rend() const noexcept { return fData.rend(); } - const_reverse_iterator crend() const noexcept { return fData.crend(); } - // capacity - bool empty() const noexcept { return fData.empty(); } - size_type size() const noexcept { return fData.size(); } - size_type max_size() const noexcept { return fData.size(); } - void reserve(size_type new_cap) { fData.reserve(new_cap); } - size_type capacity() const noexcept { return fData.capacity(); } - void shrink_to_fit() { fData.shrink_to_fit(); }; - // modifiers - void clear() noexcept { fData.clear(); } - iterator erase(iterator pos) { return fData.erase(pos); } - iterator erase(iterator first, iterator last) { return fData.erase(first, last); } - void push_back(T &&value) { fData.push_back(std::forward(value)); } - void push_back(const value_type& value) { fData.push_back(value); }; - template - reference emplace_back(Args &&... args) + // conversion + template ::value>> + operator RVec() const + { + return RVec(this->begin(), this->end()); + } + + // from the original LLVM implementation: + // FIXME find a less verbose way + typename SmallVectorTemplateCommon::reference at(typename SmallVectorTemplateCommon::size_type pos) { - ROOT::Internal::VecOps::EmplaceBack(fData, std::forward(args)...); - return fData.back(); + if (pos >= this->fSize) + throw std::out_of_range("RVec"); + return this->operator[](pos); } - /// This method is intended only for arithmetic types unlike the std::vector - /// corresponding one which is generic. - template::value, int>* = nullptr> - iterator emplace(const_iterator pos, U value) + typename SmallVectorTemplateCommon::const_reference at(typename SmallVectorTemplateCommon::size_type pos) const { - return fData.emplace(pos, value); + if (pos >= this->fSize) + throw std::out_of_range("RVec"); + return this->operator[](pos); } - void pop_back() { fData.pop_back(); } - void resize(size_type count) { fData.resize(count); } - void resize(size_type count, const value_type &value) { fData.resize(count, value); } - void swap(RVec &other) { std::swap(fData, other.fData); } + /// No exception thrown. The user specifies the desired value in case the RVec is shorter than `pos`. + typename SmallVectorTemplateCommon::value_type + at(typename SmallVectorTemplateCommon::size_type pos, typename SmallVectorTemplateCommon::value_type fallback) + { + if (pos >= this->fSize) + return fallback; + return this->operator[](pos); + } + /// No exception thrown. The user specifies the desired value in case the RVec is shorter than `pos`. + typename SmallVectorTemplateCommon::value_type + at(typename SmallVectorTemplateCommon::size_type pos, typename SmallVectorTemplateCommon::value_type fallback) const + { + if (pos >= this->fSize) + return fallback; + return this->operator[](pos); + } + }; +template +inline size_t capacity_in_bytes(const RVec &X) +{ + return X.capacity_in_bytes(); +} + ///@name RVec Unary Arithmetic Operators ///@{ diff --git a/math/vecops/src/RVec.cxx b/math/vecops/src/RVec.cxx index 29e167adb4c9f..d1b73341f3ec7 100644 --- a/math/vecops/src/RVec.cxx +++ b/math/vecops/src/RVec.cxx @@ -1,4 +1,96 @@ #include "ROOT/RVec.hxx" +using namespace ROOT::VecOps; + +/***** FIXME the extra boolean breaks this assert and the two below -- maybe use unused bits in pointer? +// Check that no bytes are wasted and everything is well-aligned. +namespace { +struct Struct16B { + alignas(16) void *X; +}; +struct Struct32B { + alignas(32) void *X; +}; +} // namespace +static_assert(sizeof(SmallVector) == sizeof(unsigned) * 2 + sizeof(void *), + "wasted space in RVec size 0"); +static_assert(alignof(SmallVector) >= alignof(Struct16B), "wrong alignment for 16-byte aligned T"); +static_assert(alignof(SmallVector) >= alignof(Struct32B), "wrong alignment for 32-byte aligned T"); +static_assert(sizeof(SmallVector) >= alignof(Struct16B), "missing padding for 16-byte aligned T"); +static_assert(sizeof(SmallVector) >= alignof(Struct32B), "missing padding for 32-byte aligned T"); +static_assert(sizeof(SmallVector) == sizeof(unsigned) * 2 + sizeof(void *) * 2, + "wasted space in SmallVector size 1"); + +static_assert(sizeof(SmallVector) == sizeof(void *) * 2 + sizeof(void *), + "1 byte elements have word-sized type for size and capacity"); +************/ + +template +void SmallVectorBase::report_size_overflow(size_t MinSize) +{ + std::string Reason = "RVec unable to grow. Requested capacity (" + std::to_string(MinSize) + + ") is larger than maximum value for size type (" + std::to_string(SizeTypeMax()) + ")"; + throw std::length_error(Reason); +} + +template +void SmallVectorBase::report_at_maximum_capacity() +{ + std::string Reason = "RVec capacity unable to grow. Already at maximum size " + std::to_string(SizeTypeMax()); + throw std::length_error(Reason); +} + +// Note: Moving this function into the header may cause performance regression. +template +void SmallVectorBase::grow_pod(void *FirstEl, size_t MinSize, size_t TSize) +{ + // Ensure we can fit the new capacity. + // This is only going to be applicable when the capacity is 32 bit. + if (MinSize > SizeTypeMax()) + report_size_overflow(MinSize); + + // Ensure we can meet the guarantee of space for at least one more element. + // The above check alone will not catch the case where grow is called with a + // default MinSize of 0, but the current capacity cannot be increased. + // This is only going to be applicable when the capacity is 32 bit. + if (capacity() == SizeTypeMax()) + report_at_maximum_capacity(); + + // In theory 2*capacity can overflow if the capacity is 64 bit, but the + // original capacity would never be large enough for this to be a problem. + size_t NewCapacity = 2 * capacity() + 1; // Always grow. + NewCapacity = std::min(std::max(NewCapacity, MinSize), SizeTypeMax()); + + void *NewElts; + if (BeginX == FirstEl) { + NewElts = malloc(NewCapacity * TSize); + + // Copy the elements over. No need to run dtors on PODs. + memcpy(NewElts, this->BeginX, size() * TSize); + } else { + // If this wasn't grown from the inline copy, grow the allocated space. + NewElts = realloc(this->BeginX, NewCapacity * TSize); + } + + this->BeginX = NewElts; + this->Capacity = NewCapacity; +} + +template class ROOT::VecOps::SmallVectorBase; + +// Disable the uint64_t instantiation for 32-bit builds. +// Both uint32_t and uint64_t instantations are needed for 64-bit builds. +// This instantiation will never be used in 32-bit builds, and will cause +// warnings when sizeof(Size_T) > sizeof(size_t). +#if SIZE_MAX > UINT32_MAX +template class ROOT::VecOps::SmallVectorBase; + +// Assertions to ensure this #if stays in sync with SmallVectorSizeType. +static_assert(sizeof(SmallVectorSizeType) == sizeof(uint64_t), + "Expected SmallVectorBase variant to be in use."); +#else +static_assert(sizeof(SmallVectorSizeType) == sizeof(uint32_t), + "Expected SmallVectorBase variant to be in use."); +#endif #if (_VECOPS_USE_EXTERN_TEMPLATES) From 018a3de79a6d06647e32dc0168dd23f557b90589 Mon Sep 17 00:00:00 2001 From: Enrico Guiraud Date: Wed, 17 Feb 2021 10:29:20 +0100 Subject: [PATCH 04/51] [VecOps] Fix RVec I/O after change in class layout In particular, now that RVec is not implemented in terms of std::vector anymore, we do not need to apply the related tricks when performing I/O of RVec. --- core/cont/inc/TCollectionProxyInfo.h | 4 ---- io/io/src/TGenCollectionProxy.cxx | 3 ++- io/io/src/TStreamerInfoActions.cxx | 2 -- 3 files changed, 2 insertions(+), 7 deletions(-) diff --git a/core/cont/inc/TCollectionProxyInfo.h b/core/cont/inc/TCollectionProxyInfo.h index 3787d1311a8df..79a73b1b9a43c 100644 --- a/core/cont/inc/TCollectionProxyInfo.h +++ b/core/cont/inc/TCollectionProxyInfo.h @@ -752,10 +752,6 @@ namespace Detail { } }; - // TODO this can/should go away when we move to new RVec - template <> - struct TCollectionProxyInfo::Type> : TCollectionProxyInfo::Type> {}; - template struct TCollectionProxyInfo::Type > : public TCollectionProxyInfo::Address { typedef Bitset_t Cont_t; diff --git a/io/io/src/TGenCollectionProxy.cxx b/io/io/src/TGenCollectionProxy.cxx index 83ee325ed50c3..945ef6c981c64 100644 --- a/io/io/src/TGenCollectionProxy.cxx +++ b/io/io/src/TGenCollectionProxy.cxx @@ -1043,13 +1043,14 @@ void* TGenCollectionProxy::At(UInt_t idx) if ( fEnv && fEnv->fObject ) { switch (fSTL_type) { case ROOT::kSTLvector: - case ROOT::kROOTRVec: // TODO will be unnecessary with RVec 2.0: RVec won't be a special case if ((*fValue).fKind == kBool_t) { auto vec = (std::vector *)(fEnv->fObject); fEnv->fLastValueVecBool = (*vec)[idx]; fEnv->fIdx = idx; return &(fEnv->fLastValueVecBool); } + // intentional fall through + case ROOT::kROOTRVec: fEnv->fIdx = idx; switch( idx ) { case 0: diff --git a/io/io/src/TStreamerInfoActions.cxx b/io/io/src/TStreamerInfoActions.cxx index 7b61dab789af2..0afc89b3e7c46 100644 --- a/io/io/src/TStreamerInfoActions.cxx +++ b/io/io/src/TStreamerInfoActions.cxx @@ -1704,8 +1704,6 @@ namespace TStreamerInfoActions || proxy.GetCollectionType() == ROOT::kSTLunorderedmap || proxy.GetCollectionType() == ROOT::kSTLunorderedmultimap || proxy.GetCollectionType() == ROOT::kSTLbitset) { return kAssociativeLooper; - } else if (proxy.GetCollectionType() == ROOT::kROOTRVec && proxy.GetType() == EDataType::kBool_t) { - return kVectorLooper; } else { return kGenericLooper; } From 32d22f390d40bdc7c488adb7aea0a602a6a16263 Mon Sep 17 00:00:00 2001 From: Stefan Wunsch Date: Thu, 25 Mar 2021 12:31:47 +0100 Subject: [PATCH 05/51] [VecOps][PyROOT] Add workarounds for PyROOT issues PyROOT does not pick up correctly methods such as `operator[]` or `size` from the parent classes and therefore fails to pythonize the proxies in Python correctly. Raising the respective methods in the hierarchy is a workaround for these issues, which are tracked in #7686. --- math/vecops/inc/ROOT/RVec.hxx | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/math/vecops/inc/ROOT/RVec.hxx b/math/vecops/inc/ROOT/RVec.hxx index 5ecfc3db9dd0d..36770dbc46508 100644 --- a/math/vecops/inc/ROOT/RVec.hxx +++ b/math/vecops/inc/ROOT/RVec.hxx @@ -436,17 +436,6 @@ public: /// Return a pointer to the vector's buffer, even if empty(). const_pointer data() const { return const_pointer(begin()); } - reference operator[](size_type idx) - { - assert(idx < size()); - return begin()[idx]; - } - const_reference operator[](size_type idx) const - { - assert(idx < size()); - return begin()[idx]; - } - reference front() { assert(!empty()); @@ -1291,7 +1280,22 @@ public: return *this; } - using SmallVectorTemplateCommon::operator[]; + using reference = typename SmallVectorTemplateCommon::reference; + using const_reference = typename SmallVectorTemplateCommon::const_reference; + using size_type = typename SmallVectorTemplateCommon::size_type; + using SmallVectorTemplateCommon::begin; + using SmallVectorTemplateCommon::size; + + reference operator[](size_type idx) + { + assert(idx < size()); + return begin()[idx]; + } + const_reference operator[](size_type idx) const + { + assert(idx < size()); + return begin()[idx]; + } template ::value>> RVec operator[](const RVec &conds) const From 17bcf4cff3054e26cd0e99b84185810ea275c8a2 Mon Sep 17 00:00:00 2001 From: Enrico Guiraud Date: Tue, 13 Apr 2021 17:40:10 +0200 Subject: [PATCH 06/51] [VecOps] RVec refactorings: - move SmallVector classes to internal namespace - add noexcept specifiers where RVec had them - add deprecated `shrink_to_fit` and `emplace` implementations --- math/vecops/inc/ROOT/RVec.hxx | 434 ++++++++++++++++++---------------- math/vecops/src/RVec.cxx | 14 +- 2 files changed, 234 insertions(+), 214 deletions(-) diff --git a/math/vecops/inc/ROOT/RVec.hxx b/math/vecops/inc/ROOT/RVec.hxx index 36770dbc46508..ada158cbc1ec7 100644 --- a/math/vecops/inc/ROOT/RVec.hxx +++ b/math/vecops/inc/ROOT/RVec.hxx @@ -117,173 +117,10 @@ auto MapFromTuple(Tuple_t &&t, std::index_sequence) } } +namespace Internal { namespace VecOps { - -// Note that we open here with @{ the Doxygen group vecops and it is -// closed again at the end of the C++ namespace VecOps -/** - * \defgroup vecops VecOps - * A "std::vector"-like collection of values implementing handy operation to analyse them - * @{ -*/ - -// clang-format off -/** -\class ROOT::VecOps::RVec -\brief A "std::vector"-like collection of values implementing handy operation to analyse them -\tparam T The type of the contained objects - -A RVec is a container designed to make analysis of values' collections fast and easy. -Its storage is contiguous in memory and its interface is designed such to resemble to the one -of the stl vector. In addition the interface features methods and external functions to ease -the manipulation and analysis of the data in the RVec. - -\htmlonly -DOI -\endhtmlonly - -## Table of Contents -- [Example](\ref example) -- [Owning and adopting memory](\ref owningandadoptingmemory) -- [Sorting and manipulation of indices](\ref sorting) -- [Usage in combination with RDataFrame](\ref usagetdataframe) -- [Reference for the RVec class](\ref RVecdoxyref) - -Also see the [reference for RVec helper functions](https://root.cern/doc/master/namespaceROOT_1_1VecOps.html). - -\anchor example -## Example -Suppose to have an event featuring a collection of muons with a certain pseudorapidity, -momentum and charge, e.g.: -~~~{.cpp} -std::vector mu_charge {1, 1, -1, -1, -1, 1, 1, -1}; -std::vector mu_pt {56, 45, 32, 24, 12, 8, 7, 6.2}; -std::vector mu_eta {3.1, -.2, -1.1, 1, 4.1, 1.6, 2.4, -.5}; -~~~ -Suppose you want to extract the transverse momenta of the muons satisfying certain -criteria, for example consider only negatively charged muons with a pseudorapidity -smaller or equal to 2 and with a transverse momentum greater than 10 GeV. -Such a selection would require, among the other things, the management of an explicit -loop, for example: -~~~{.cpp} -std::vector goodMuons_pt; -const auto size = mu_charge.size(); -for (size_t i=0; i < size; ++i) { - if (mu_pt[i] > 10 && abs(mu_eta[i]) <= 2. && mu_charge[i] == -1) { - goodMuons_pt.emplace_back(mu_pt[i]); - } -} -~~~ -These operations become straightforward with RVec - we just need to *write what -we mean*: -~~~{.cpp} -auto goodMuons_pt = mu_pt[ (mu_pt > 10.f && abs(mu_eta) <= 2.f && mu_charge == -1) ] -~~~ -Now the clean collection of transverse momenta can be used within the rest of the data analysis, for -example to fill a histogram. - -\anchor owningandadoptingmemory -## Owning and adopting memory -RVec has contiguous memory associated to it. It can own it or simply adopt it. In the latter case, -it can be constructed with the address of the memory associated to it and its length. For example: -~~~{.cpp} -std::vector myStlVec {1,2,3}; -RVec myRVec(myStlVec.data(), myStlVec.size()); -~~~ -In this case, the memory associated to myStlVec and myRVec is the same, myRVec simply "adopted it". -If any method which implies a re-allocation is called, e.g. *emplace_back* or *resize*, the adopted -memory is released and new one is allocated. The previous content is copied in the new memory and -preserved. - -\anchor sorting -## Sorting and manipulation of indices - -### Sorting -RVec complies to the STL interfaces when it comes to iterations. As a result, standard algorithms -can be used, for example sorting: -~~~{.cpp} -RVec v{6., 4., 5.}; -std::sort(v.begin(), v.end()); -~~~ - -For convinience, helpers are provided too: -~~~{.cpp} -auto sorted_v = Sort(v); -auto reversed_v = Reverse(v); -~~~ - -### Manipulation of indices - -It is also possible to manipulated the RVecs acting on their indices. For example, -the following syntax -~~~{.cpp} -RVec v0 {9., 7., 8.}; -auto v1 = Take(v0, {1, 2, 0}); -~~~ -will yield a new RVec the content of which is the first, second and zeroth element of -v0, i.e. `{7., 8., 9.}`. - -The `Argsort` helper extracts the indices which order the content of a `RVec`. For example, -this snippet accomplish in a more expressive way what we just achieved: -~~~{.cpp} -auto v1_indices = Argsort(v0); // The content of v1_indices is {1, 2, 0}. -v1 = Take(v0, v1_indices); -~~~ - -The `Take` utility allows to extract portions of the `RVec`. The content to be *taken* -can be specified with an `RVec` of indices or an integer. If the integer is negative, -elements will be picked starting from the end of the container: -~~~{.cpp} -RVec vf {1.f, 2.f, 3.f, 4.f}; -auto vf_1 = Take(vf, {1, 3}); // The content is {2.f, 4.f} -auto vf_2 = Take(vf, 2); // The content is {1.f, 2.f} -auto vf_3 = Take(vf, -3); // The content is {2.f, 3.f, 4.f} -~~~ - -\anchor usagetdataframe -## Usage in combination with RDataFrame -RDataFrame leverages internally RVecs. Suppose to have a dataset stored in a -TTree which holds these columns (here we choose C arrays to represent the -collections, they could be as well std::vector instances): -~~~{.bash} - nPart "nPart/I" An integer representing the number of particles - px "px[nPart]/D" The C array of the particles' x component of the momentum - py "py[nPart]/D" The C array of the particles' y component of the momentum - E "E[nPart]/D" The C array of the particles' Energy -~~~ -Suppose you'd like to plot in a histogram the transverse momenta of all particles -for which the energy is greater than 200 MeV. -The code required would just be: -~~~{.cpp} -RDataFrame d("mytree", "myfile.root"); -using doubles = RVec; -auto cutPt = [](doubles &pxs, doubles &pys, doubles &Es) { - auto all_pts = sqrt(pxs * pxs + pys * pys); - auto good_pts = all_pts[Es > 200.]; - return good_pts; - }; - -auto hpt = d.Define("pt", cutPt, {"px", "py", "E"}) - .Histo1D("pt"); -hpt->Draw(); -~~~ -And if you'd like to express your selection as a string: -~~~{.cpp} -RDataFrame d("mytree", "myfile.root"); -auto hpt = d.Define("pt", "sqrt(pxs * pxs + pys * pys)[E>200]") - .Histo1D("pt"); -hpt->Draw(); -~~~ -\anchor RVecdoxyref - -**/ - -// clang-format on - -//FIXME put things in the namespace they belong to - -/// Returns the next power of two (in 64-bits) that is strictly greater than A. -/// Returns zero on overflow. +/// Return the next power of two (in 64-bits) that is strictly greater than A. +/// Return zero on overflow. inline uint64_t NextPowerOf2(uint64_t A) { A |= (A >> 1); @@ -330,7 +167,7 @@ protected: public: size_t size() const { return fSize; } - size_t capacity() const { return Capacity; } + size_t capacity() const noexcept { return Capacity; } RVEC_NODISCARD bool empty() const { return !fSize; } @@ -348,6 +185,11 @@ public: assert(N <= capacity()); fSize = N; } + + // LLVM SmallVector does not have a shrink_to_fit method + // it's technically ok to do nothing, but assuming no one uses this method for RVec anyway, I'd rather deprecate it + R__DEPRECATED(6, 28, "This method will be removed.") + constexpr void shrink_to_fit() { } }; template @@ -415,26 +257,30 @@ public: using Base::size; // forward iterator creation methods. - iterator begin() { return (iterator)this->BeginX; } - const_iterator begin() const { return (const_iterator)this->BeginX; } - iterator end() { return begin() + size(); } - const_iterator end() const { return begin() + size(); } + iterator begin() noexcept { return (iterator)this->BeginX; } + const_iterator begin() const noexcept { return (const_iterator)this->BeginX; } + const_iterator cbegin() const noexcept { return (const_iterator)this->BeginX; } + iterator end() noexcept { return begin() + size(); } + const_iterator end() const noexcept { return begin() + size(); } + const_iterator cend() const noexcept { return begin() + size(); } // reverse iterator creation methods. - reverse_iterator rbegin() { return reverse_iterator(end()); } - const_reverse_iterator rbegin() const { return const_reverse_iterator(end()); } - reverse_iterator rend() { return reverse_iterator(begin()); } - const_reverse_iterator rend() const { return const_reverse_iterator(begin()); } + reverse_iterator rbegin() noexcept { return reverse_iterator(end()); } + const_reverse_iterator rbegin() const noexcept { return const_reverse_iterator(end()); } + const_reverse_iterator crbegin() const noexcept { return const_reverse_iterator(end()); } + reverse_iterator rend() noexcept { return reverse_iterator(begin()); } + const_reverse_iterator rend() const noexcept { return const_reverse_iterator(begin()); } + const_reverse_iterator crend() const noexcept { return const_reverse_iterator(begin()); } size_type size_in_bytes() const { return size() * sizeof(T); } - size_type max_size() const { return std::min(this->SizeTypeMax(), size_type(-1) / sizeof(T)); } + size_type max_size() const noexcept { return std::min(this->SizeTypeMax(), size_type(-1) / sizeof(T)); } size_t capacity_in_bytes() const { return capacity() * sizeof(T); } /// Return a pointer to the vector's buffer, even if empty(). - pointer data() { return pointer(begin()); } + pointer data() noexcept { return pointer(begin()); } /// Return a pointer to the vector's buffer, even if empty(). - const_pointer data() const { return const_pointer(begin()); } + const_pointer data() const noexcept { return const_pointer(begin()); } reference front() { @@ -985,7 +831,15 @@ public: void insert(iterator I, std::initializer_list IL) { insert(I, IL.begin(), IL.end()); } template - reference emplace_back(ArgTypes &&... Args) + R__DEPRECATED(6, 28, "Please use RVec::insert instead.") + reference emplace(iterator I, ArgTypes &&...Args) + { + // these are not the exact semantics of emplace, of course, hence the deprecation. + return insert(I, T(std::forward...)); + } + + template + reference emplace_back(ArgTypes &&...Args) { if (R__unlikely(this->size() >= this->capacity())) this->grow(); @@ -1193,6 +1047,170 @@ struct SmallVectorStorage { template struct alignas(T) SmallVectorStorage { }; +} // namespace VecOps +} // namespace Internal + +namespace VecOps { +// Note that we open here with @{ the Doxygen group vecops and it is +// closed again at the end of the C++ namespace VecOps +/** + * \defgroup vecops VecOps + * A "std::vector"-like collection of values implementing handy operation to analyse them + * @{ +*/ + +// clang-format off +/** +\class ROOT::VecOps::RVec +\brief A "std::vector"-like collection of values implementing handy operation to analyse them +\tparam T The type of the contained objects + +A RVec is a container designed to make analysis of values' collections fast and easy. +Its storage is contiguous in memory and its interface is designed such to resemble to the one +of the stl vector. In addition the interface features methods and external functions to ease +the manipulation and analysis of the data in the RVec. + +\htmlonly +DOI +\endhtmlonly + +## Table of Contents +- [Example](\ref example) +- [Owning and adopting memory](\ref owningandadoptingmemory) +- [Sorting and manipulation of indices](\ref sorting) +- [Usage in combination with RDataFrame](\ref usagetdataframe) +- [Reference for the RVec class](\ref RVecdoxyref) + +Also see the [reference for RVec helper functions](https://root.cern/doc/master/namespaceROOT_1_1VecOps.html). + +\anchor example +## Example +Suppose to have an event featuring a collection of muons with a certain pseudorapidity, +momentum and charge, e.g.: +~~~{.cpp} +std::vector mu_charge {1, 1, -1, -1, -1, 1, 1, -1}; +std::vector mu_pt {56, 45, 32, 24, 12, 8, 7, 6.2}; +std::vector mu_eta {3.1, -.2, -1.1, 1, 4.1, 1.6, 2.4, -.5}; +~~~ +Suppose you want to extract the transverse momenta of the muons satisfying certain +criteria, for example consider only negatively charged muons with a pseudorapidity +smaller or equal to 2 and with a transverse momentum greater than 10 GeV. +Such a selection would require, among the other things, the management of an explicit +loop, for example: +~~~{.cpp} +std::vector goodMuons_pt; +const auto size = mu_charge.size(); +for (size_t i=0; i < size; ++i) { + if (mu_pt[i] > 10 && abs(mu_eta[i]) <= 2. && mu_charge[i] == -1) { + goodMuons_pt.emplace_back(mu_pt[i]); + } +} +~~~ +These operations become straightforward with RVec - we just need to *write what +we mean*: +~~~{.cpp} +auto goodMuons_pt = mu_pt[ (mu_pt > 10.f && abs(mu_eta) <= 2.f && mu_charge == -1) ] +~~~ +Now the clean collection of transverse momenta can be used within the rest of the data analysis, for +example to fill a histogram. + +\anchor owningandadoptingmemory +## Owning and adopting memory +RVec has contiguous memory associated to it. It can own it or simply adopt it. In the latter case, +it can be constructed with the address of the memory associated to it and its length. For example: +~~~{.cpp} +std::vector myStlVec {1,2,3}; +RVec myRVec(myStlVec.data(), myStlVec.size()); +~~~ +In this case, the memory associated to myStlVec and myRVec is the same, myRVec simply "adopted it". +If any method which implies a re-allocation is called, e.g. *emplace_back* or *resize*, the adopted +memory is released and new one is allocated. The previous content is copied in the new memory and +preserved. + +\anchor sorting +## Sorting and manipulation of indices + +### Sorting +RVec complies to the STL interfaces when it comes to iterations. As a result, standard algorithms +can be used, for example sorting: +~~~{.cpp} +RVec v{6., 4., 5.}; +std::sort(v.begin(), v.end()); +~~~ + +For convinience, helpers are provided too: +~~~{.cpp} +auto sorted_v = Sort(v); +auto reversed_v = Reverse(v); +~~~ + +### Manipulation of indices + +It is also possible to manipulated the RVecs acting on their indices. For example, +the following syntax +~~~{.cpp} +RVec v0 {9., 7., 8.}; +auto v1 = Take(v0, {1, 2, 0}); +~~~ +will yield a new RVec the content of which is the first, second and zeroth element of +v0, i.e. `{7., 8., 9.}`. + +The `Argsort` helper extracts the indices which order the content of a `RVec`. For example, +this snippet accomplish in a more expressive way what we just achieved: +~~~{.cpp} +auto v1_indices = Argsort(v0); // The content of v1_indices is {1, 2, 0}. +v1 = Take(v0, v1_indices); +~~~ + +The `Take` utility allows to extract portions of the `RVec`. The content to be *taken* +can be specified with an `RVec` of indices or an integer. If the integer is negative, +elements will be picked starting from the end of the container: +~~~{.cpp} +RVec vf {1.f, 2.f, 3.f, 4.f}; +auto vf_1 = Take(vf, {1, 3}); // The content is {2.f, 4.f} +auto vf_2 = Take(vf, 2); // The content is {1.f, 2.f} +auto vf_3 = Take(vf, -3); // The content is {2.f, 3.f, 4.f} +~~~ + +\anchor usagetdataframe +## Usage in combination with RDataFrame +RDataFrame leverages internally RVecs. Suppose to have a dataset stored in a +TTree which holds these columns (here we choose C arrays to represent the +collections, they could be as well std::vector instances): +~~~{.bash} + nPart "nPart/I" An integer representing the number of particles + px "px[nPart]/D" The C array of the particles' x component of the momentum + py "py[nPart]/D" The C array of the particles' y component of the momentum + E "E[nPart]/D" The C array of the particles' Energy +~~~ +Suppose you'd like to plot in a histogram the transverse momenta of all particles +for which the energy is greater than 200 MeV. +The code required would just be: +~~~{.cpp} +RDataFrame d("mytree", "myfile.root"); +using doubles = RVec; +auto cutPt = [](doubles &pxs, doubles &pys, doubles &Es) { + auto all_pts = sqrt(pxs * pxs + pys * pys); + auto good_pts = all_pts[Es > 200.]; + return good_pts; + }; + +auto hpt = d.Define("pt", cutPt, {"px", "py", "E"}) + .Histo1D("pt"); +hpt->Draw(); +~~~ +And if you'd like to express your selection as a string: +~~~{.cpp} +RDataFrame d("mytree", "myfile.root"); +auto hpt = d.Define("pt", "sqrt(pxs * pxs + pys * pys)[E>200]") + .Histo1D("pt"); +hpt->Draw(); +~~~ +\anchor RVecdoxyref +**/ +// clang-format on + + /// This is a 'vector' (really, a variable-sized array), optimized /// for the case when the array is small. It contains some number of elements @@ -1203,10 +1221,10 @@ struct alignas(T) SmallVectorStorage { /// Note that this does not attempt to be exception safe. /// template -class RVec : public SmallVectorImpl, SmallVectorStorage { +class RVec : public Internal::VecOps::SmallVectorImpl, Internal::VecOps::SmallVectorStorage { public: static constexpr unsigned N = 8; - RVec() : SmallVectorImpl(N) {} + RVec() : Internal::VecOps::SmallVectorImpl(N) {} ~RVec() { @@ -1216,51 +1234,54 @@ public: } } - explicit RVec(size_t Size, const T &Value = T()) : SmallVectorImpl(N) { this->assign(Size, Value); } + explicit RVec(size_t Size, const T &Value = T()) : Internal::VecOps::SmallVectorImpl(N) + { + this->assign(Size, Value); + } template ::iterator_category, std::input_iterator_tag>::value>::type> - RVec(ItTy S, ItTy E) : SmallVectorImpl(N) + RVec(ItTy S, ItTy E) : Internal::VecOps::SmallVectorImpl(N) { this->append(S, E); } - RVec(std::initializer_list IL) : SmallVectorImpl(N) { this->assign(IL); } + RVec(std::initializer_list IL) : Internal::VecOps::SmallVectorImpl(N) { this->assign(IL); } - RVec(const RVec &RHS) : SmallVectorImpl(N) + RVec(const RVec &RHS) : Internal::VecOps::SmallVectorImpl(N) { if (!RHS.empty()) - SmallVectorImpl::operator=(RHS); + Internal::VecOps::SmallVectorImpl::operator=(RHS); } RVec &operator=(const RVec &RHS) { - SmallVectorImpl::operator=(RHS); + Internal::VecOps::SmallVectorImpl::operator=(RHS); return *this; } - RVec(RVec &&RHS) : SmallVectorImpl(N) + RVec(RVec &&RHS) : Internal::VecOps::SmallVectorImpl(N) { if (!RHS.empty()) - SmallVectorImpl::operator=(::std::move(RHS)); + Internal::VecOps::SmallVectorImpl::operator=(::std::move(RHS)); } - RVec(SmallVectorImpl &&RHS) : SmallVectorImpl(N) + RVec(Internal::VecOps::SmallVectorImpl &&RHS) : Internal::VecOps::SmallVectorImpl(N) { if (!RHS.empty()) - SmallVectorImpl::operator=(::std::move(RHS)); + Internal::VecOps::SmallVectorImpl::operator=(::std::move(RHS)); } RVec(const std::vector &RHS) : RVec(RHS.begin(), RHS.end()) {} RVec &operator=(RVec &&RHS) { - SmallVectorImpl::operator=(::std::move(RHS)); + Internal::VecOps::SmallVectorImpl::operator=(::std::move(RHS)); return *this; } - RVec(typename RVec::pointer p, typename RVec::size_type n) : SmallVectorImpl(N) + RVec(typename RVec::pointer p, typename RVec::size_type n) : Internal::VecOps::SmallVectorImpl(N) { this->BeginX = p; this->fSize = n; @@ -1268,9 +1289,9 @@ public: this->Owns = false; } - RVec &operator=(SmallVectorImpl &&RHS) + RVec &operator=(Internal::VecOps::SmallVectorImpl &&RHS) { - SmallVectorImpl::operator=(::std::move(RHS)); + Internal::VecOps::SmallVectorImpl::operator=(::std::move(RHS)); return *this; } @@ -1280,11 +1301,11 @@ public: return *this; } - using reference = typename SmallVectorTemplateCommon::reference; - using const_reference = typename SmallVectorTemplateCommon::const_reference; - using size_type = typename SmallVectorTemplateCommon::size_type; - using SmallVectorTemplateCommon::begin; - using SmallVectorTemplateCommon::size; + using reference = typename Internal::VecOps::SmallVectorTemplateCommon::reference; + using const_reference = typename Internal::VecOps::SmallVectorTemplateCommon::const_reference; + using size_type = typename Internal::VecOps::SmallVectorTemplateCommon::size_type; + using Internal::VecOps::SmallVectorTemplateCommon::begin; + using Internal::VecOps::SmallVectorTemplateCommon::size; reference operator[](size_type idx) { @@ -1322,35 +1343,34 @@ public: // from the original LLVM implementation: // FIXME find a less verbose way - typename SmallVectorTemplateCommon::reference at(typename SmallVectorTemplateCommon::size_type pos) + typename Internal::VecOps::SmallVectorTemplateCommon::reference at(typename Internal::VecOps::SmallVectorTemplateCommon::size_type pos) { if (pos >= this->fSize) throw std::out_of_range("RVec"); return this->operator[](pos); } - typename SmallVectorTemplateCommon::const_reference at(typename SmallVectorTemplateCommon::size_type pos) const + typename Internal::VecOps::SmallVectorTemplateCommon::const_reference at(typename Internal::VecOps::SmallVectorTemplateCommon::size_type pos) const { if (pos >= this->fSize) throw std::out_of_range("RVec"); return this->operator[](pos); } /// No exception thrown. The user specifies the desired value in case the RVec is shorter than `pos`. - typename SmallVectorTemplateCommon::value_type - at(typename SmallVectorTemplateCommon::size_type pos, typename SmallVectorTemplateCommon::value_type fallback) + typename Internal::VecOps::SmallVectorTemplateCommon::value_type + at(typename Internal::VecOps::SmallVectorTemplateCommon::size_type pos, typename Internal::VecOps::SmallVectorTemplateCommon::value_type fallback) { if (pos >= this->fSize) return fallback; return this->operator[](pos); } /// No exception thrown. The user specifies the desired value in case the RVec is shorter than `pos`. - typename SmallVectorTemplateCommon::value_type - at(typename SmallVectorTemplateCommon::size_type pos, typename SmallVectorTemplateCommon::value_type fallback) const + typename Internal::VecOps::SmallVectorTemplateCommon::value_type + at(typename Internal::VecOps::SmallVectorTemplateCommon::size_type pos, typename Internal::VecOps::SmallVectorTemplateCommon::value_type fallback) const { if (pos >= this->fSize) return fallback; return this->operator[](pos); } - }; template diff --git a/math/vecops/src/RVec.cxx b/math/vecops/src/RVec.cxx index d1b73341f3ec7..2dd5c8e22abb0 100644 --- a/math/vecops/src/RVec.cxx +++ b/math/vecops/src/RVec.cxx @@ -25,7 +25,7 @@ static_assert(sizeof(SmallVector) == sizeof(void *) * 2 + sizeof(void * ************/ template -void SmallVectorBase::report_size_overflow(size_t MinSize) +void ROOT::Internal::VecOps::SmallVectorBase::report_size_overflow(size_t MinSize) { std::string Reason = "RVec unable to grow. Requested capacity (" + std::to_string(MinSize) + ") is larger than maximum value for size type (" + std::to_string(SizeTypeMax()) + ")"; @@ -33,7 +33,7 @@ void SmallVectorBase::report_size_overflow(size_t MinSize) } template -void SmallVectorBase::report_at_maximum_capacity() +void ROOT::Internal::VecOps::SmallVectorBase::report_at_maximum_capacity() { std::string Reason = "RVec capacity unable to grow. Already at maximum size " + std::to_string(SizeTypeMax()); throw std::length_error(Reason); @@ -41,7 +41,7 @@ void SmallVectorBase::report_at_maximum_capacity() // Note: Moving this function into the header may cause performance regression. template -void SmallVectorBase::grow_pod(void *FirstEl, size_t MinSize, size_t TSize) +void ROOT::Internal::VecOps::SmallVectorBase::grow_pod(void *FirstEl, size_t MinSize, size_t TSize) { // Ensure we can fit the new capacity. // This is only going to be applicable when the capacity is 32 bit. @@ -75,20 +75,20 @@ void SmallVectorBase::grow_pod(void *FirstEl, size_t MinSize, size_t TSi this->Capacity = NewCapacity; } -template class ROOT::VecOps::SmallVectorBase; +template class ROOT::Internal::VecOps::SmallVectorBase; // Disable the uint64_t instantiation for 32-bit builds. // Both uint32_t and uint64_t instantations are needed for 64-bit builds. // This instantiation will never be used in 32-bit builds, and will cause // warnings when sizeof(Size_T) > sizeof(size_t). #if SIZE_MAX > UINT32_MAX -template class ROOT::VecOps::SmallVectorBase; +template class ROOT::Internal::VecOps::SmallVectorBase; // Assertions to ensure this #if stays in sync with SmallVectorSizeType. -static_assert(sizeof(SmallVectorSizeType) == sizeof(uint64_t), +static_assert(sizeof(ROOT::Internal::VecOps::SmallVectorSizeType) == sizeof(uint64_t), "Expected SmallVectorBase variant to be in use."); #else -static_assert(sizeof(SmallVectorSizeType) == sizeof(uint32_t), +static_assert(sizeof(ROOT::Internal::VecOps::SmallVectorSizeType) == sizeof(uint32_t), "Expected SmallVectorBase variant to be in use."); #endif From 549be6853529b26d4afd257bf56d4bb86c2a5683 Mon Sep 17 00:00:00 2001 From: Enrico Guiraud Date: Tue, 4 May 2021 16:15:12 +0200 Subject: [PATCH 07/51] [VecOps] Fix compilation errors on older compilers/standards - constexpr functions returning void are a C++14 feature - some compilers cannot access type aliases through multiple levels of inheritance --- math/vecops/inc/ROOT/RVec.hxx | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/math/vecops/inc/ROOT/RVec.hxx b/math/vecops/inc/ROOT/RVec.hxx index ada158cbc1ec7..d005daf28832f 100644 --- a/math/vecops/inc/ROOT/RVec.hxx +++ b/math/vecops/inc/ROOT/RVec.hxx @@ -189,7 +189,7 @@ public: // LLVM SmallVector does not have a shrink_to_fit method // it's technically ok to do nothing, but assuming no one uses this method for RVec anyway, I'd rather deprecate it R__DEPRECATED(6, 28, "This method will be removed.") - constexpr void shrink_to_fit() { } + void shrink_to_fit() { } }; template @@ -417,6 +417,8 @@ void SmallVectorTemplateBase::grow(size_t MinSize) /// skipping destruction. template class SmallVectorTemplateBase : public SmallVectorTemplateCommon { + using SuperClass = SmallVectorTemplateCommon; + protected: SmallVectorTemplateBase(size_t Size) : SmallVectorTemplateCommon(Size) {} @@ -465,6 +467,11 @@ protected: } public: + using iterator = typename SuperClass::iterator; + using const_iterator = typename SuperClass::const_iterator; + using reference = typename SuperClass::reference; + using size_type = typename SuperClass::size_type; + void push_back(const T &Elt) { if (R__unlikely(this->size() >= this->capacity())) From b967df31d42964144c2c46dc320cf5e8467ee46c Mon Sep 17 00:00:00 2001 From: Enrico Guiraud Date: Tue, 4 May 2021 16:45:06 +0200 Subject: [PATCH 08/51] [VecOps] Make windows happy MSVC could not digest the previous spelling. --- math/vecops/inc/ROOT/RVec.hxx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/math/vecops/inc/ROOT/RVec.hxx b/math/vecops/inc/ROOT/RVec.hxx index d005daf28832f..496cc0e9ffda4 100644 --- a/math/vecops/inc/ROOT/RVec.hxx +++ b/math/vecops/inc/ROOT/RVec.hxx @@ -1288,7 +1288,7 @@ public: return *this; } - RVec(typename RVec::pointer p, typename RVec::size_type n) : Internal::VecOps::SmallVectorImpl(N) + RVec(T* p, size_t n) : Internal::VecOps::SmallVectorImpl(N) { this->BeginX = p; this->fSize = n; @@ -1964,7 +1964,7 @@ template auto Any(const RVec &v) -> decltype(v[0] == true) { for (auto &&e : v) - if (e == true) + if (static_cast(e) == true) return true; return false; } @@ -1983,7 +1983,7 @@ template auto All(const RVec &v) -> decltype(v[0] == false) { for (auto &&e : v) - if (e == false) + if (static_cast(e) == false) return false; return true; } From defcd360c6e5122d428fa1df87dc3566986134f1 Mon Sep 17 00:00:00 2001 From: Enrico Guiraud Date: Thu, 27 May 2021 10:19:39 +0200 Subject: [PATCH 09/51] [VecOps][NFC] Update year in a copyright notice --- math/vecops/inc/LinkDef.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/math/vecops/inc/LinkDef.h b/math/vecops/inc/LinkDef.h index 5a03297c31c9c..3b3c2b5495b34 100644 --- a/math/vecops/inc/LinkDef.h +++ b/math/vecops/inc/LinkDef.h @@ -1,5 +1,5 @@ /************************************************************************* - * Copyright (C) 1995-2020, Rene Brun and Fons Rademakers. * + * Copyright (C) 1995-2021, Rene Brun and Fons Rademakers. * * All rights reserved. * * * * Author: Danilo Piparo - CERN * From aa3d7d1194179be05c801c33da2af9c217723d4f Mon Sep 17 00:00:00 2001 From: Enrico Guiraud Date: Thu, 27 May 2021 10:20:58 +0200 Subject: [PATCH 10/51] [VecOps][NFC] Clarify comment Co-authored-by: Axel Naumann --- math/vecops/inc/ROOT/RVec.hxx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/math/vecops/inc/ROOT/RVec.hxx b/math/vecops/inc/ROOT/RVec.hxx index 496cc0e9ffda4..cc9fead76adc2 100644 --- a/math/vecops/inc/ROOT/RVec.hxx +++ b/math/vecops/inc/ROOT/RVec.hxx @@ -9,9 +9,9 @@ *************************************************************************/ /* - * The implementation of RVec is based in LLVM's SmallVector. + * The implementation of RVec is based on LLVM's SmallVector. * - * The ability to adopt an existing memory buffer has been added, + * Compared to LLVM's SmallVectors, the ability to adopt an existing memory buffer has been added, * as well as other minor patches for backward compatibility with * the previous implementation of RVec (which did not have a small buffer * optimization and was based on std::vector with a custom allocator) and From f8e49ab842238b9868e51fe255125b939f6094be Mon Sep 17 00:00:00 2001 From: Enrico Guiraud Date: Thu, 27 May 2021 11:45:53 +0200 Subject: [PATCH 11/51] [VecOps][NFC] Rename RVEC_NODISCARD to R__RVEC_NODISCARD --- math/vecops/inc/ROOT/RVec.hxx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/math/vecops/inc/ROOT/RVec.hxx b/math/vecops/inc/ROOT/RVec.hxx index cc9fead76adc2..16ecf3eda215b 100644 --- a/math/vecops/inc/ROOT/RVec.hxx +++ b/math/vecops/inc/ROOT/RVec.hxx @@ -23,9 +23,9 @@ #define ROOT_RVEC #if __cplusplus > 201402L -#define RVEC_NODISCARD [[nodiscard]] +#define R__RVEC_NODISCARD [[nodiscard]] #else -#define RVEC_NODISCARD +#define R__RVEC_NODISCARD #endif #ifdef _WIN32 @@ -169,7 +169,7 @@ public: size_t size() const { return fSize; } size_t capacity() const noexcept { return Capacity; } - RVEC_NODISCARD bool empty() const { return !fSize; } + R__RVEC_NODISCARD bool empty() const { return !fSize; } /// Set the array size to \p N, which the current array must have enough /// capacity for. @@ -564,7 +564,7 @@ public: this->set_size(this->size() - NumItems); } - RVEC_NODISCARD T pop_back_val() + R__RVEC_NODISCARD T pop_back_val() { T Result = ::std::move(this->back()); this->pop_back(); From 4aa442043874c1fb2d4ca4fd2973a35b06d4eb2b Mon Sep 17 00:00:00 2001 From: Enrico Guiraud Date: Thu, 27 May 2021 11:46:22 +0200 Subject: [PATCH 12/51] [VecOps][NFC] Add comments --- math/vecops/inc/ROOT/RVec.hxx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/math/vecops/inc/ROOT/RVec.hxx b/math/vecops/inc/ROOT/RVec.hxx index 16ecf3eda215b..7e96c45069682 100644 --- a/math/vecops/inc/ROOT/RVec.hxx +++ b/math/vecops/inc/ROOT/RVec.hxx @@ -114,8 +114,8 @@ auto MapFromTuple(Tuple_t &&t, std::index_sequence) return MapImpl(std::get(t), std::get(t)...); } -} -} +} // namespace VecOps +} // namespace Detail namespace Internal { namespace VecOps { From 077371b4ba7a6f823d07d486e624080b622e987a Mon Sep 17 00:00:00 2001 From: Enrico Guiraud Date: Mon, 31 May 2021 10:45:40 +0200 Subject: [PATCH 13/51] [VecOps][NFC] Remove SmallVector-related comment from doxygen --- math/vecops/inc/ROOT/RVec.hxx | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/math/vecops/inc/ROOT/RVec.hxx b/math/vecops/inc/ROOT/RVec.hxx index 7e96c45069682..ac4296c4c9a14 100644 --- a/math/vecops/inc/ROOT/RVec.hxx +++ b/math/vecops/inc/ROOT/RVec.hxx @@ -1219,14 +1219,15 @@ hpt->Draw(); -/// This is a 'vector' (really, a variable-sized array), optimized -/// for the case when the array is small. It contains some number of elements -/// in-place, which allows it to avoid heap allocation when the actual number of -/// elements is below that threshold. This allows normal "small" cases to be -/// fast without losing generality for large inputs. -/// -/// Note that this does not attempt to be exception safe. -/// +// From the original SmallVector code: +// This is a 'vector' (really, a variable-sized array), optimized +// for the case when the array is small. It contains some number of elements +// in-place, which allows it to avoid heap allocation when the actual number of +// elements is below that threshold. This allows normal "small" cases to be +// fast without losing generality for large inputs. +// +// Note that this does not attempt to be exception safe. + template class RVec : public Internal::VecOps::SmallVectorImpl, Internal::VecOps::SmallVectorStorage { public: From 992c327c37510345926dd0000ecc5dc7a8d25d0f Mon Sep 17 00:00:00 2001 From: Enrico Guiraud Date: Mon, 31 May 2021 10:49:51 +0200 Subject: [PATCH 14/51] [VecOps] Add test for #6796 (interaction of memory adoption and clear) --- math/vecops/test/vecops_rvec.cxx | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/math/vecops/test/vecops_rvec.cxx b/math/vecops/test/vecops_rvec.cxx index 76caee7cfd9d4..7c5139bcfb4e4 100644 --- a/math/vecops/test/vecops_rvec.cxx +++ b/math/vecops/test/vecops_rvec.cxx @@ -1116,3 +1116,14 @@ TEST(VecOps, Construct) EXPECT_TRUE(fourVects[2] == ref2); } +// this is a regression test for https://github.com/root-project/root/issues/6796 +TEST(VecOps, MemoryAdoptionAndClear) +{ + ROOT::RVec v{1, 2, 3}; + ROOT::RVec v2(v.data(), v.size()); + v2[0] = 0; + v2.clear(); + EXPECT_TRUE(All(v == RVec{0, 2, 3})); + v2.push_back(42); + EXPECT_TRUE(All(v2 == RVec{42})); +} From 539fdd0b8bf4dddc74063fa3f653af72bc3df4b9 Mon Sep 17 00:00:00 2001 From: Enrico Guiraud Date: Mon, 31 May 2021 11:15:35 +0200 Subject: [PATCH 15/51] [VecOps][NFC] Use ROOT naming convention for SmallVector data members E.g. rename Capacity to fCapacity --- math/vecops/inc/ROOT/RVec.hxx | 81 ++++++++++++++++++----------------- math/vecops/src/RVec.cxx | 10 ++--- 2 files changed, 46 insertions(+), 45 deletions(-) diff --git a/math/vecops/inc/ROOT/RVec.hxx b/math/vecops/inc/ROOT/RVec.hxx index ac4296c4c9a14..a21cf0d51c71f 100644 --- a/math/vecops/inc/ROOT/RVec.hxx +++ b/math/vecops/inc/ROOT/RVec.hxx @@ -143,15 +143,16 @@ inline uint64_t NextPowerOf2(uint64_t A) template class SmallVectorBase { protected: - void *BeginX; - Size_T fSize = 0, Capacity; - bool Owns = true; + void *fBeginX; + Size_T fSize = 0; + Size_T fCapacity; + bool fOwns = true; /// The maximum value of the Size_T used. static constexpr size_t SizeTypeMax() { return std::numeric_limits::max(); } SmallVectorBase() = delete; - SmallVectorBase(void *FirstEl, size_t TotalCapacity) : BeginX(FirstEl), Capacity(TotalCapacity) {} + SmallVectorBase(void *FirstEl, size_t TotalCapacity) : fBeginX(FirstEl), fCapacity(TotalCapacity) {} /// This is an implementation of the grow() method which only works /// on POD-like data types and is out of line to reduce code duplication. @@ -167,7 +168,7 @@ protected: public: size_t size() const { return fSize; } - size_t capacity() const noexcept { return Capacity; } + size_t capacity() const noexcept { return fCapacity; } R__RVEC_NODISCARD bool empty() const { return !fSize; } @@ -226,15 +227,15 @@ protected: /// Return true if this is a smallvector which has not had dynamic /// memory allocated for it. - bool isSmall() const { return this->BeginX == getFirstEl(); } + bool isSmall() const { return this->fBeginX == getFirstEl(); } /// Put this vector in a state of being small. void resetToSmall() { - this->BeginX = getFirstEl(); + this->fBeginX = getFirstEl(); // from the original LLVM implementation: - // FIXME: Setting Capacity to 0 is suspect. - this->fSize = this->Capacity = 0; + // FIXME: Setting fCapacity to 0 is suspect. + this->fSize = this->fCapacity = 0; } public: @@ -257,9 +258,9 @@ public: using Base::size; // forward iterator creation methods. - iterator begin() noexcept { return (iterator)this->BeginX; } - const_iterator begin() const noexcept { return (const_iterator)this->BeginX; } - const_iterator cbegin() const noexcept { return (const_iterator)this->BeginX; } + iterator begin() noexcept { return (iterator)this->fBeginX; } + const_iterator begin() const noexcept { return (const_iterator)this->fBeginX; } + const_iterator cbegin() const noexcept { return (const_iterator)this->fBeginX; } iterator end() noexcept { return begin() + size(); } const_iterator end() const noexcept { return begin() + size(); } const_iterator cend() const noexcept { return begin() + size(); } @@ -397,7 +398,7 @@ void SmallVectorTemplateBase::grow(size_t MinSize) // Move the elements over. this->uninitialized_move(this->begin(), this->end(), NewElts); - if (this->Owns) { + if (this->fOwns) { // Destroy the original elements. destroy_range(this->begin(), this->end()); @@ -406,9 +407,9 @@ void SmallVectorTemplateBase::grow(size_t MinSize) free(this->begin()); } - this->BeginX = NewElts; - this->Capacity = NewCapacity; - this->Owns = true; + this->fBeginX = NewElts; + this->fCapacity = NewCapacity; + this->fOwns = true; } /// SmallVectorTemplateBase - This is where we put @@ -463,7 +464,7 @@ protected: void grow(size_t MinSize = 0) { this->grow_pod(MinSize, sizeof(T)); - this->Owns = true; + this->fOwns = true; } public: @@ -506,14 +507,14 @@ public: { // Subclass has already destructed this vector's elements. // If this wasn't grown from the inline copy, deallocate the old space. - if (!this->isSmall() && this->Owns) + if (!this->isSmall() && this->fOwns) free(this->begin()); } // also give up adopted memory if applicable void clear() { - if (this->Owns) { + if (this->fOwns) { this->destroy_range(this->begin(), this->end()); this->fSize = 0; } else { @@ -524,7 +525,7 @@ public: void resize(size_type N) { if (N < this->size()) { - if (this->Owns) + if (this->fOwns) this->destroy_range(this->begin() + N, this->end()); this->set_size(N); } else if (N > this->size()) { @@ -539,7 +540,7 @@ public: void resize(size_type N, const T &NV) { if (N < this->size()) { - if (this->Owns) + if (this->fOwns) this->destroy_range(this->begin() + N, this->end()); this->set_size(N); } else if (N > this->size()) { @@ -559,7 +560,7 @@ public: void pop_back_n(size_type NumItems) { assert(this->size() >= NumItems); - if (this->Owns) + if (this->fOwns) this->destroy_range(this->end() - NumItems, this->end()); this->set_size(this->size() - NumItems); } @@ -657,7 +658,7 @@ public: // Shift all elts down. iterator I = std::move(E, this->end(), S); // Drop the last elts. - if (this->Owns) + if (this->fOwns) this->destroy_range(I, this->end()); this->set_size(I - this->begin()); return (N); @@ -881,10 +882,10 @@ void SmallVectorImpl::swap(SmallVectorImpl &RHS) // We can only avoid copying elements if neither vector is small. if (!this->isSmall() && !RHS.isSmall()) { - std::swap(this->BeginX, RHS.BeginX); + std::swap(this->fBeginX, RHS.fBeginX); std::swap(this->fSize, RHS.fSize); - std::swap(this->Capacity, RHS.Capacity); - std::swap(this->Owns, RHS.Owns); + std::swap(this->fCapacity, RHS.fCapacity); + std::swap(this->fOwns, RHS.fOwns); return; } if (RHS.size() > this->capacity()) @@ -904,14 +905,14 @@ void SmallVectorImpl::swap(SmallVectorImpl &RHS) size_t EltDiff = this->size() - RHS.size(); this->uninitialized_copy(this->begin() + NumShared, this->end(), RHS.end()); RHS.set_size(RHS.size() + EltDiff); - if (this->Owns) + if (this->fOwns) this->destroy_range(this->begin() + NumShared, this->end()); this->set_size(NumShared); } else if (RHS.size() > this->size()) { size_t EltDiff = RHS.size() - this->size(); this->uninitialized_copy(RHS.begin() + NumShared, RHS.end(), this->end()); this->set_size(this->size() + EltDiff); - if (RHS.Owns) + if (RHS.fOwns) this->destroy_range(RHS.begin() + NumShared, RHS.end()); RHS.set_size(NumShared); } @@ -937,7 +938,7 @@ SmallVectorImpl &SmallVectorImpl::operator=(const SmallVectorImpl &RHS) NewEnd = this->begin(); // Destroy excess elements. - if (this->Owns) + if (this->fOwns) this->destroy_range(NewEnd, this->end()); // Trim. @@ -950,7 +951,7 @@ SmallVectorImpl &SmallVectorImpl::operator=(const SmallVectorImpl &RHS) // From the original LLVM implementation: // FIXME: don't do this if they're efficiently moveable. if (this->capacity() < RHSSize) { - if (this->Owns) { + if (this->fOwns) { // Destroy current elements. this->destroy_range(this->begin(), this->end()); } @@ -979,15 +980,15 @@ SmallVectorImpl &SmallVectorImpl::operator=(SmallVectorImpl &&RHS) // If the RHS isn't small, clear this vector and then steal its buffer. if (!RHS.isSmall()) { - if (this->Owns) { + if (this->fOwns) { this->destroy_range(this->begin(), this->end()); if (!this->isSmall()) free(this->begin()); } - this->BeginX = RHS.BeginX; + this->fBeginX = RHS.fBeginX; this->fSize = RHS.fSize; - this->Capacity = RHS.Capacity; - this->Owns = RHS.Owns; + this->fCapacity = RHS.fCapacity; + this->fOwns = RHS.fOwns; RHS.resetToSmall(); return *this; } @@ -1003,7 +1004,7 @@ SmallVectorImpl &SmallVectorImpl::operator=(SmallVectorImpl &&RHS) NewEnd = std::move(RHS.begin(), RHS.end(), NewEnd); // Destroy excess elements and trim the bounds. - if (this->Owns) + if (this->fOwns) this->destroy_range(NewEnd, this->end()); this->set_size(RHSSize); @@ -1019,7 +1020,7 @@ SmallVectorImpl &SmallVectorImpl::operator=(SmallVectorImpl &&RHS) // FIXME: this may not actually make any sense if we can efficiently move // elements. if (this->capacity() < RHSSize) { - if (this->Owns) { + if (this->fOwns) { // Destroy current elements. this->destroy_range(this->begin(), this->end()); } @@ -1236,7 +1237,7 @@ public: ~RVec() { - if (this->Owns) { + if (this->fOwns) { // Destroy the constructed elements in the vector. this->destroy_range(this->begin(), this->end()); } @@ -1291,10 +1292,10 @@ public: RVec(T* p, size_t n) : Internal::VecOps::SmallVectorImpl(N) { - this->BeginX = p; + this->fBeginX = p; this->fSize = n; - this->Capacity = n; - this->Owns = false; + this->fCapacity = n; + this->fOwns = false; } RVec &operator=(Internal::VecOps::SmallVectorImpl &&RHS) diff --git a/math/vecops/src/RVec.cxx b/math/vecops/src/RVec.cxx index 2dd5c8e22abb0..610e911c04a93 100644 --- a/math/vecops/src/RVec.cxx +++ b/math/vecops/src/RVec.cxx @@ -61,18 +61,18 @@ void ROOT::Internal::VecOps::SmallVectorBase::grow_pod(void *FirstEl, si NewCapacity = std::min(std::max(NewCapacity, MinSize), SizeTypeMax()); void *NewElts; - if (BeginX == FirstEl) { + if (fBeginX == FirstEl) { NewElts = malloc(NewCapacity * TSize); // Copy the elements over. No need to run dtors on PODs. - memcpy(NewElts, this->BeginX, size() * TSize); + memcpy(NewElts, this->fBeginX, size() * TSize); } else { // If this wasn't grown from the inline copy, grow the allocated space. - NewElts = realloc(this->BeginX, NewCapacity * TSize); + NewElts = realloc(this->fBeginX, NewCapacity * TSize); } - this->BeginX = NewElts; - this->Capacity = NewCapacity; + this->fBeginX = NewElts; + this->fCapacity = NewCapacity; } template class ROOT::Internal::VecOps::SmallVectorBase; From 422dd59d4f0f1a1d94376088e8c294a6f0e5d068 Mon Sep 17 00:00:00 2001 From: Enrico Guiraud Date: Tue, 15 Jun 2021 14:59:30 +0200 Subject: [PATCH 16/51] [VecOps] Rename Internal::SmallVectorImpl => Detail::RVecImpl We will expose RVec's base class that erases the small-size template parameter to expert users. --- math/vecops/inc/ROOT/RVec.hxx | 96 +++++++++++++++++++---------------- 1 file changed, 51 insertions(+), 45 deletions(-) diff --git a/math/vecops/inc/ROOT/RVec.hxx b/math/vecops/inc/ROOT/RVec.hxx index a21cf0d51c71f..dcd99c14e87ca 100644 --- a/math/vecops/inc/ROOT/RVec.hxx +++ b/math/vecops/inc/ROOT/RVec.hxx @@ -484,11 +484,31 @@ public: void pop_back() { this->set_size(this->size() - 1); } }; +/// Storage for the SmallVector elements. This is specialized for the N=0 case +/// to avoid allocating unnecessary storage. +template +struct SmallVectorStorage { + alignas(T) char InlineElts[N * sizeof(T)]; +}; + +/// We need the storage to be properly aligned even for small-size of 0 so that +/// the pointer math in \a SmallVectorTemplateCommon::getFirstEl() is +/// well-defined. +template +struct alignas(T) SmallVectorStorage { +}; + +} // namespace VecOps +} // namespace Internal + +namespace Detail { +namespace VecOps { + /// This class consists of common code factored out of the SmallVector class to /// reduce code duplication based on the SmallVector 'N' template parameter. template -class SmallVectorImpl : public SmallVectorTemplateBase { - using SuperClass = SmallVectorTemplateBase; +class RVecImpl : public Internal::VecOps::SmallVectorTemplateBase { + using SuperClass = Internal::VecOps::SmallVectorTemplateBase; public: using iterator = typename SuperClass::iterator; @@ -498,12 +518,12 @@ public: protected: // Default ctor - Initialize to empty. - explicit SmallVectorImpl(unsigned N) : SmallVectorTemplateBase(N) {} + explicit RVecImpl(unsigned N) : Internal::VecOps::SmallVectorTemplateBase(N) {} public: - SmallVectorImpl(const SmallVectorImpl &) = delete; + RVecImpl(const RVecImpl &) = delete; - ~SmallVectorImpl() + ~RVecImpl() { // Subclass has already destructed this vector's elements. // If this wasn't grown from the inline copy, deallocate the old space. @@ -572,7 +592,7 @@ public: return Result; } - void swap(SmallVectorImpl &RHS); + void swap(RVecImpl &RHS); /// Add the specified range to the end of the SmallVector. template back(); } - SmallVectorImpl &operator=(const SmallVectorImpl &RHS); + RVecImpl &operator=(const RVecImpl &RHS); - SmallVectorImpl &operator=(SmallVectorImpl &&RHS); + RVecImpl &operator=(RVecImpl &&RHS); - bool operator==(const SmallVectorImpl &RHS) const + bool operator==(const RVecImpl &RHS) const { if (this->size() != RHS.size()) return false; return std::equal(this->begin(), this->end(), RHS.begin()); } - bool operator!=(const SmallVectorImpl &RHS) const { return !(*this == RHS); } + bool operator!=(const RVecImpl &RHS) const { return !(*this == RHS); } - bool operator<(const SmallVectorImpl &RHS) const + bool operator<(const RVecImpl &RHS) const { return std::lexicographical_compare(this->begin(), this->end(), RHS.begin(), RHS.end()); } }; template -void SmallVectorImpl::swap(SmallVectorImpl &RHS) +void RVecImpl::swap(RVecImpl &RHS) { if (this == &RHS) return; @@ -919,7 +939,7 @@ void SmallVectorImpl::swap(SmallVectorImpl &RHS) } template -SmallVectorImpl &SmallVectorImpl::operator=(const SmallVectorImpl &RHS) +RVecImpl &RVecImpl::operator=(const RVecImpl &RHS) { // Avoid self-assignment. if (this == &RHS) @@ -972,7 +992,7 @@ SmallVectorImpl &SmallVectorImpl::operator=(const SmallVectorImpl &RHS) } template -SmallVectorImpl &SmallVectorImpl::operator=(SmallVectorImpl &&RHS) +RVecImpl &RVecImpl::operator=(RVecImpl &&RHS) { // Avoid self-assignment. if (this == &RHS) @@ -1041,22 +1061,8 @@ SmallVectorImpl &SmallVectorImpl::operator=(SmallVectorImpl &&RHS) RHS.clear(); return *this; } - -/// Storage for the SmallVector elements. This is specialized for the N=0 case -/// to avoid allocating unnecessary storage. -template -struct SmallVectorStorage { - alignas(T) char InlineElts[N * sizeof(T)]; -}; - -/// We need the storage to be properly aligned even for small-size of 0 so that -/// the pointer math in \a SmallVectorTemplateCommon::getFirstEl() is -/// well-defined. -template -struct alignas(T) SmallVectorStorage { -}; } // namespace VecOps -} // namespace Internal +} // namespace Detail namespace VecOps { // Note that we open here with @{ the Doxygen group vecops and it is @@ -1230,10 +1236,10 @@ hpt->Draw(); // Note that this does not attempt to be exception safe. template -class RVec : public Internal::VecOps::SmallVectorImpl, Internal::VecOps::SmallVectorStorage { +class RVec : public Detail::VecOps::RVecImpl, Internal::VecOps::SmallVectorStorage { public: static constexpr unsigned N = 8; - RVec() : Internal::VecOps::SmallVectorImpl(N) {} + RVec() : Detail::VecOps::RVecImpl(N) {} ~RVec() { @@ -1243,7 +1249,7 @@ public: } } - explicit RVec(size_t Size, const T &Value = T()) : Internal::VecOps::SmallVectorImpl(N) + explicit RVec(size_t Size, const T &Value = T()) : Detail::VecOps::RVecImpl(N) { this->assign(Size, Value); } @@ -1251,46 +1257,46 @@ public: template ::iterator_category, std::input_iterator_tag>::value>::type> - RVec(ItTy S, ItTy E) : Internal::VecOps::SmallVectorImpl(N) + RVec(ItTy S, ItTy E) : Detail::VecOps::RVecImpl(N) { this->append(S, E); } - RVec(std::initializer_list IL) : Internal::VecOps::SmallVectorImpl(N) { this->assign(IL); } + RVec(std::initializer_list IL) : Detail::VecOps::RVecImpl(N) { this->assign(IL); } - RVec(const RVec &RHS) : Internal::VecOps::SmallVectorImpl(N) + RVec(const RVec &RHS) : Detail::VecOps::RVecImpl(N) { if (!RHS.empty()) - Internal::VecOps::SmallVectorImpl::operator=(RHS); + Detail::VecOps::RVecImpl::operator=(RHS); } RVec &operator=(const RVec &RHS) { - Internal::VecOps::SmallVectorImpl::operator=(RHS); + Detail::VecOps::RVecImpl::operator=(RHS); return *this; } - RVec(RVec &&RHS) : Internal::VecOps::SmallVectorImpl(N) + RVec(RVec &&RHS) : Detail::VecOps::RVecImpl(N) { if (!RHS.empty()) - Internal::VecOps::SmallVectorImpl::operator=(::std::move(RHS)); + Detail::VecOps::RVecImpl::operator=(::std::move(RHS)); } - RVec(Internal::VecOps::SmallVectorImpl &&RHS) : Internal::VecOps::SmallVectorImpl(N) + RVec(Detail::VecOps::RVecImpl &&RHS) : Detail::VecOps::RVecImpl(N) { if (!RHS.empty()) - Internal::VecOps::SmallVectorImpl::operator=(::std::move(RHS)); + Detail::VecOps::RVecImpl::operator=(::std::move(RHS)); } RVec(const std::vector &RHS) : RVec(RHS.begin(), RHS.end()) {} RVec &operator=(RVec &&RHS) { - Internal::VecOps::SmallVectorImpl::operator=(::std::move(RHS)); + Detail::VecOps::RVecImpl::operator=(::std::move(RHS)); return *this; } - RVec(T* p, size_t n) : Internal::VecOps::SmallVectorImpl(N) + RVec(T* p, size_t n) : Detail::VecOps::RVecImpl(N) { this->fBeginX = p; this->fSize = n; @@ -1298,9 +1304,9 @@ public: this->fOwns = false; } - RVec &operator=(Internal::VecOps::SmallVectorImpl &&RHS) + RVec &operator=(Detail::VecOps::RVecImpl &&RHS) { - Internal::VecOps::SmallVectorImpl::operator=(::std::move(RHS)); + Detail::VecOps::RVecImpl::operator=(::std::move(RHS)); return *this; } From d6f0a66f618bfdbe4cb46d7e6089d88caed68dd5 Mon Sep 17 00:00:00 2001 From: Enrico Guiraud Date: Tue, 15 Jun 2021 17:25:18 +0200 Subject: [PATCH 17/51] [VecOps][NFC] Take advantage of local aliases to make code less verbose --- math/vecops/inc/ROOT/RVec.hxx | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/math/vecops/inc/ROOT/RVec.hxx b/math/vecops/inc/ROOT/RVec.hxx index dcd99c14e87ca..2d8bc5198ecdd 100644 --- a/math/vecops/inc/ROOT/RVec.hxx +++ b/math/vecops/inc/ROOT/RVec.hxx @@ -1319,6 +1319,7 @@ public: using reference = typename Internal::VecOps::SmallVectorTemplateCommon::reference; using const_reference = typename Internal::VecOps::SmallVectorTemplateCommon::const_reference; using size_type = typename Internal::VecOps::SmallVectorTemplateCommon::size_type; + using value_type = typename Internal::VecOps::SmallVectorTemplateCommon::value_type; using Internal::VecOps::SmallVectorTemplateCommon::begin; using Internal::VecOps::SmallVectorTemplateCommon::size; @@ -1356,31 +1357,30 @@ public: return RVec(this->begin(), this->end()); } - // from the original LLVM implementation: - // FIXME find a less verbose way - typename Internal::VecOps::SmallVectorTemplateCommon::reference at(typename Internal::VecOps::SmallVectorTemplateCommon::size_type pos) + reference at(size_type pos) { if (pos >= this->fSize) throw std::out_of_range("RVec"); return this->operator[](pos); } - typename Internal::VecOps::SmallVectorTemplateCommon::const_reference at(typename Internal::VecOps::SmallVectorTemplateCommon::size_type pos) const + + const_reference at(size_type pos) const { if (pos >= this->fSize) throw std::out_of_range("RVec"); return this->operator[](pos); } + /// No exception thrown. The user specifies the desired value in case the RVec is shorter than `pos`. - typename Internal::VecOps::SmallVectorTemplateCommon::value_type - at(typename Internal::VecOps::SmallVectorTemplateCommon::size_type pos, typename Internal::VecOps::SmallVectorTemplateCommon::value_type fallback) + value_type at(size_type pos, value_type fallback) { if (pos >= this->fSize) return fallback; return this->operator[](pos); } + /// No exception thrown. The user specifies the desired value in case the RVec is shorter than `pos`. - typename Internal::VecOps::SmallVectorTemplateCommon::value_type - at(typename Internal::VecOps::SmallVectorTemplateCommon::size_type pos, typename Internal::VecOps::SmallVectorTemplateCommon::value_type fallback) const + value_type at(size_type pos, value_type fallback) const { if (pos >= this->fSize) return fallback; From 88c6ad35aa166e2bfec90560a9fb5654567906e5 Mon Sep 17 00:00:00 2001 From: Enrico Guiraud Date: Tue, 15 Jun 2021 17:01:32 +0200 Subject: [PATCH 18/51] [VecOps] Always use int32_t as size type for RVec LLVM SmallVector uses uint64_t for value types with small size and uint32_t otherwise. For RVec, we need signed-ness of the size type to give special meaning to fCapacity == -1 (it will signal the RVec is in "memory adoption" mode) and we don't expect to ever need more than the maximum size allowed by int32_t (i.e. we don't need to switch to int64_t for small value types). --- math/vecops/inc/ROOT/RVec.hxx | 25 +++++++++++++------------ math/vecops/src/RVec.cxx | 26 +++----------------------- 2 files changed, 16 insertions(+), 35 deletions(-) diff --git a/math/vecops/inc/ROOT/RVec.hxx b/math/vecops/inc/ROOT/RVec.hxx index 2d8bc5198ecdd..0a85ba6f2d984 100644 --- a/math/vecops/inc/ROOT/RVec.hxx +++ b/math/vecops/inc/ROOT/RVec.hxx @@ -140,11 +140,13 @@ inline uint64_t NextPowerOf2(uint64_t A) /// Using 64 bit size is desirable for cases like SmallVector, where a /// 32 bit size would limit the vector to ~4GB. SmallVectors are used for /// buffering bitcode output - which can exceed 4GB. -template class SmallVectorBase { +public: + using Size_T = int32_t; + protected: void *fBeginX; - Size_T fSize = 0; + Size_T fSize = 0; // never negative even if type is signed Size_T fCapacity; bool fOwns = true; @@ -193,13 +195,10 @@ public: void shrink_to_fit() { } }; -template -using SmallVectorSizeType = typename std::conditional= 8, uint64_t, uint32_t>::type; - /// Figure out the offset of the first element. template struct SmallVectorAlignmentAndSize { - alignas(SmallVectorBase>) char Base[sizeof(SmallVectorBase>)]; + alignas(SmallVectorBase) char Base[sizeof(SmallVectorBase)]; alignas(T) char FirstEl[sizeof(T)]; }; @@ -207,8 +206,8 @@ struct SmallVectorAlignmentAndSize { /// the type T is a POD. The extra dummy template argument is used by ArrayRef /// to avoid unnecessarily requiring T to be complete. template -class SmallVectorTemplateCommon : public SmallVectorBase> { - using Base = SmallVectorBase>; +class SmallVectorTemplateCommon : public SmallVectorBase { + using Base = SmallVectorBase; /// Find the address of the first element. For this pointer math to be valid /// with small-size of 0 for T with lots of alignment, it's important that @@ -239,6 +238,8 @@ protected: } public: + // note that fSize is a _signed_ integer, but we expose it as an unsigned integer for consistency with STL containers + // as well as backward-compatibility using size_type = size_t; using difference_type = ptrdiff_t; using value_type = T; @@ -1359,14 +1360,14 @@ public: reference at(size_type pos) { - if (pos >= this->fSize) + if (pos >= size_type(this->fSize)) throw std::out_of_range("RVec"); return this->operator[](pos); } const_reference at(size_type pos) const { - if (pos >= this->fSize) + if (pos >= size_type(this->fSize)) throw std::out_of_range("RVec"); return this->operator[](pos); } @@ -1374,7 +1375,7 @@ public: /// No exception thrown. The user specifies the desired value in case the RVec is shorter than `pos`. value_type at(size_type pos, value_type fallback) { - if (pos >= this->fSize) + if (pos >= size_type(this->fSize)) return fallback; return this->operator[](pos); } @@ -1382,7 +1383,7 @@ public: /// No exception thrown. The user specifies the desired value in case the RVec is shorter than `pos`. value_type at(size_type pos, value_type fallback) const { - if (pos >= this->fSize) + if (pos >= size_type(this->fSize)) return fallback; return this->operator[](pos); } diff --git a/math/vecops/src/RVec.cxx b/math/vecops/src/RVec.cxx index 610e911c04a93..bb2829baea554 100644 --- a/math/vecops/src/RVec.cxx +++ b/math/vecops/src/RVec.cxx @@ -24,24 +24,21 @@ static_assert(sizeof(SmallVector) == sizeof(void *) * 2 + sizeof(void * "1 byte elements have word-sized type for size and capacity"); ************/ -template -void ROOT::Internal::VecOps::SmallVectorBase::report_size_overflow(size_t MinSize) +void ROOT::Internal::VecOps::SmallVectorBase::report_size_overflow(size_t MinSize) { std::string Reason = "RVec unable to grow. Requested capacity (" + std::to_string(MinSize) + ") is larger than maximum value for size type (" + std::to_string(SizeTypeMax()) + ")"; throw std::length_error(Reason); } -template -void ROOT::Internal::VecOps::SmallVectorBase::report_at_maximum_capacity() +void ROOT::Internal::VecOps::SmallVectorBase::report_at_maximum_capacity() { std::string Reason = "RVec capacity unable to grow. Already at maximum size " + std::to_string(SizeTypeMax()); throw std::length_error(Reason); } // Note: Moving this function into the header may cause performance regression. -template -void ROOT::Internal::VecOps::SmallVectorBase::grow_pod(void *FirstEl, size_t MinSize, size_t TSize) +void ROOT::Internal::VecOps::SmallVectorBase::grow_pod(void *FirstEl, size_t MinSize, size_t TSize) { // Ensure we can fit the new capacity. // This is only going to be applicable when the capacity is 32 bit. @@ -75,23 +72,6 @@ void ROOT::Internal::VecOps::SmallVectorBase::grow_pod(void *FirstEl, si this->fCapacity = NewCapacity; } -template class ROOT::Internal::VecOps::SmallVectorBase; - -// Disable the uint64_t instantiation for 32-bit builds. -// Both uint32_t and uint64_t instantations are needed for 64-bit builds. -// This instantiation will never be used in 32-bit builds, and will cause -// warnings when sizeof(Size_T) > sizeof(size_t). -#if SIZE_MAX > UINT32_MAX -template class ROOT::Internal::VecOps::SmallVectorBase; - -// Assertions to ensure this #if stays in sync with SmallVectorSizeType. -static_assert(sizeof(ROOT::Internal::VecOps::SmallVectorSizeType) == sizeof(uint64_t), - "Expected SmallVectorBase variant to be in use."); -#else -static_assert(sizeof(ROOT::Internal::VecOps::SmallVectorSizeType) == sizeof(uint32_t), - "Expected SmallVectorBase variant to be in use."); -#endif - #if (_VECOPS_USE_EXTERN_TEMPLATES) namespace ROOT { From e51f9e147906e5a9c430f3c2da1d7dabdab23599 Mon Sep 17 00:00:00 2001 From: Enrico Guiraud Date: Tue, 15 Jun 2021 18:29:50 +0200 Subject: [PATCH 19/51] [VecOps] Use fCapacity == -1 to indicate memory adoption mode --- math/vecops/inc/ROOT/RVec.hxx | 48 +++++++++++++++++------------------ 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/math/vecops/inc/ROOT/RVec.hxx b/math/vecops/inc/ROOT/RVec.hxx index 0a85ba6f2d984..3351d3a3c8486 100644 --- a/math/vecops/inc/ROOT/RVec.hxx +++ b/math/vecops/inc/ROOT/RVec.hxx @@ -146,9 +146,11 @@ public: protected: void *fBeginX; - Size_T fSize = 0; // never negative even if type is signed + /// Always >= 0. + // Type is signed only for consistency with fCapacity. + Size_T fSize = 0; + /// Always >= -1. fCapacity == -1 indicates the RVec is in "memory adoption" mode. Size_T fCapacity; - bool fOwns = true; /// The maximum value of the Size_T used. static constexpr size_t SizeTypeMax() { return std::numeric_limits::max(); } @@ -168,9 +170,12 @@ protected: /// std::length_error or calls report_fatal_error. static void report_at_maximum_capacity(); + /// If true, the RVec is in "memory adoption" mode, i.e. it is acting as a view on a memory buffer it does not own. + bool Owns() const { return fCapacity != -1; } + public: size_t size() const { return fSize; } - size_t capacity() const noexcept { return fCapacity; } + size_t capacity() const noexcept { return Owns() ? fCapacity : fSize; } R__RVEC_NODISCARD bool empty() const { return !fSize; } @@ -399,7 +404,7 @@ void SmallVectorTemplateBase::grow(size_t MinSize) // Move the elements over. this->uninitialized_move(this->begin(), this->end(), NewElts); - if (this->fOwns) { + if (this->Owns()) { // Destroy the original elements. destroy_range(this->begin(), this->end()); @@ -410,7 +415,6 @@ void SmallVectorTemplateBase::grow(size_t MinSize) this->fBeginX = NewElts; this->fCapacity = NewCapacity; - this->fOwns = true; } /// SmallVectorTemplateBase - This is where we put @@ -465,7 +469,6 @@ protected: void grow(size_t MinSize = 0) { this->grow_pod(MinSize, sizeof(T)); - this->fOwns = true; } public: @@ -528,14 +531,14 @@ public: { // Subclass has already destructed this vector's elements. // If this wasn't grown from the inline copy, deallocate the old space. - if (!this->isSmall() && this->fOwns) + if (!this->isSmall() && this->Owns()) free(this->begin()); } // also give up adopted memory if applicable void clear() { - if (this->fOwns) { + if (this->Owns()) { this->destroy_range(this->begin(), this->end()); this->fSize = 0; } else { @@ -546,7 +549,7 @@ public: void resize(size_type N) { if (N < this->size()) { - if (this->fOwns) + if (this->Owns()) this->destroy_range(this->begin() + N, this->end()); this->set_size(N); } else if (N > this->size()) { @@ -561,7 +564,7 @@ public: void resize(size_type N, const T &NV) { if (N < this->size()) { - if (this->fOwns) + if (this->Owns()) this->destroy_range(this->begin() + N, this->end()); this->set_size(N); } else if (N > this->size()) { @@ -581,7 +584,7 @@ public: void pop_back_n(size_type NumItems) { assert(this->size() >= NumItems); - if (this->fOwns) + if (this->Owns()) this->destroy_range(this->end() - NumItems, this->end()); this->set_size(this->size() - NumItems); } @@ -679,7 +682,7 @@ public: // Shift all elts down. iterator I = std::move(E, this->end(), S); // Drop the last elts. - if (this->fOwns) + if (this->Owns()) this->destroy_range(I, this->end()); this->set_size(I - this->begin()); return (N); @@ -906,7 +909,6 @@ void RVecImpl::swap(RVecImpl &RHS) std::swap(this->fBeginX, RHS.fBeginX); std::swap(this->fSize, RHS.fSize); std::swap(this->fCapacity, RHS.fCapacity); - std::swap(this->fOwns, RHS.fOwns); return; } if (RHS.size() > this->capacity()) @@ -926,14 +928,14 @@ void RVecImpl::swap(RVecImpl &RHS) size_t EltDiff = this->size() - RHS.size(); this->uninitialized_copy(this->begin() + NumShared, this->end(), RHS.end()); RHS.set_size(RHS.size() + EltDiff); - if (this->fOwns) + if (this->Owns()) this->destroy_range(this->begin() + NumShared, this->end()); this->set_size(NumShared); } else if (RHS.size() > this->size()) { size_t EltDiff = RHS.size() - this->size(); this->uninitialized_copy(RHS.begin() + NumShared, RHS.end(), this->end()); this->set_size(this->size() + EltDiff); - if (RHS.fOwns) + if (RHS.Owns()) this->destroy_range(RHS.begin() + NumShared, RHS.end()); RHS.set_size(NumShared); } @@ -959,7 +961,7 @@ RVecImpl &RVecImpl::operator=(const RVecImpl &RHS) NewEnd = this->begin(); // Destroy excess elements. - if (this->fOwns) + if (this->Owns()) this->destroy_range(NewEnd, this->end()); // Trim. @@ -972,7 +974,7 @@ RVecImpl &RVecImpl::operator=(const RVecImpl &RHS) // From the original LLVM implementation: // FIXME: don't do this if they're efficiently moveable. if (this->capacity() < RHSSize) { - if (this->fOwns) { + if (this->Owns()) { // Destroy current elements. this->destroy_range(this->begin(), this->end()); } @@ -1001,7 +1003,7 @@ RVecImpl &RVecImpl::operator=(RVecImpl &&RHS) // If the RHS isn't small, clear this vector and then steal its buffer. if (!RHS.isSmall()) { - if (this->fOwns) { + if (this->Owns()) { this->destroy_range(this->begin(), this->end()); if (!this->isSmall()) free(this->begin()); @@ -1009,7 +1011,6 @@ RVecImpl &RVecImpl::operator=(RVecImpl &&RHS) this->fBeginX = RHS.fBeginX; this->fSize = RHS.fSize; this->fCapacity = RHS.fCapacity; - this->fOwns = RHS.fOwns; RHS.resetToSmall(); return *this; } @@ -1025,7 +1026,7 @@ RVecImpl &RVecImpl::operator=(RVecImpl &&RHS) NewEnd = std::move(RHS.begin(), RHS.end(), NewEnd); // Destroy excess elements and trim the bounds. - if (this->fOwns) + if (this->Owns()) this->destroy_range(NewEnd, this->end()); this->set_size(RHSSize); @@ -1041,7 +1042,7 @@ RVecImpl &RVecImpl::operator=(RVecImpl &&RHS) // FIXME: this may not actually make any sense if we can efficiently move // elements. if (this->capacity() < RHSSize) { - if (this->fOwns) { + if (this->Owns()) { // Destroy current elements. this->destroy_range(this->begin(), this->end()); } @@ -1244,7 +1245,7 @@ public: ~RVec() { - if (this->fOwns) { + if (this->Owns()) { // Destroy the constructed elements in the vector. this->destroy_range(this->begin(), this->end()); } @@ -1301,8 +1302,7 @@ public: { this->fBeginX = p; this->fSize = n; - this->fCapacity = n; - this->fOwns = false; + this->fCapacity = -1; } RVec &operator=(Detail::VecOps::RVecImpl &&RHS) From b86f75d48756c84864e1378d0e8310931da5dd90 Mon Sep 17 00:00:00 2001 From: Enrico Guiraud Date: Tue, 15 Jun 2021 18:45:02 +0200 Subject: [PATCH 20/51] [VecOps] Prefer throws to asserts in non-hot methods In operator[], prefer `R__ASSERT` to `assert` for consistency. --- math/vecops/inc/ROOT/RVec.hxx | 62 ++++++++++++++++++++++------------- 1 file changed, 40 insertions(+), 22 deletions(-) diff --git a/math/vecops/inc/ROOT/RVec.hxx b/math/vecops/inc/ROOT/RVec.hxx index 3351d3a3c8486..dff7e1b600785 100644 --- a/math/vecops/inc/ROOT/RVec.hxx +++ b/math/vecops/inc/ROOT/RVec.hxx @@ -48,7 +48,6 @@ #include #include -#include #include #include // for inner_product #include @@ -190,7 +189,9 @@ public: /// which will only be overwritten. void set_size(size_t N) { - assert(N <= capacity()); + if (N > capacity()) { + throw std::runtime_error("Setting size to a value greater than capacity."); + } fSize = N; } @@ -291,23 +292,33 @@ public: reference front() { - assert(!empty()); + if (empty()) { + throw std::runtime_error("`front` called on an empty RVec"); + } return begin()[0]; } + const_reference front() const { - assert(!empty()); + if (empty()) { + throw std::runtime_error("`front` called on an empty RVec"); + } return begin()[0]; } reference back() { - assert(!empty()); + if (empty()) { + throw std::runtime_error("`back` called on an empty RVec"); + } return end()[-1]; } + const_reference back() const { - assert(!empty()); + if (empty()) { + throw std::runtime_error("`back` called on an empty RVec"); + } return end()[-1]; } }; @@ -583,7 +594,9 @@ public: void pop_back_n(size_type NumItems) { - assert(this->size() >= NumItems); + if (this->size() < NumItems) { + throw std::runtime_error("Popping back more elements than those available."); + } if (this->Owns()) this->destroy_range(this->end() - NumItems, this->end()); this->set_size(this->size() - NumItems); @@ -657,8 +670,9 @@ public: // Just cast away constness because this is a non-const member function. iterator I = const_cast(CI); - assert(I >= this->begin() && "Iterator to erase is out of bounds."); - assert(I < this->end() && "Erasing at past-the-end iterator."); + if (I < this->begin() || I >= this->end()) { + throw std::runtime_error("The iterator passed to `erase` is out of bounds."); + } iterator N = I; // Shift all elts down one. @@ -674,9 +688,9 @@ public: iterator S = const_cast(CS); iterator E = const_cast(CE); - assert(S >= this->begin() && "Range to erase is out of bounds."); - assert(S <= E && "Trying to erase invalid range."); - assert(E <= this->end() && "Trying to erase past the end."); + if (S < this->begin() || E > this->end() || S > E) { + throw std::runtime_error("Invalid start/end pair passed to `erase` (out of bounds or start > end)."); + } iterator N = S; // Shift all elts down. @@ -695,8 +709,9 @@ public: return this->end() - 1; } - assert(I >= this->begin() && "Insertion iterator is out of bounds."); - assert(I <= this->end() && "Inserting past the end of the vector."); + if (I < this->begin() || I > this->end()) { + throw std::runtime_error("The iterator passed to `insert` is out of bounds."); + } if (this->size() >= this->capacity()) { size_t EltNo = I - this->begin(); @@ -726,8 +741,9 @@ public: return this->end() - 1; } - assert(I >= this->begin() && "Insertion iterator is out of bounds."); - assert(I <= this->end() && "Inserting past the end of the vector."); + if (I < this->begin() || I > this->end()) { + throw std::runtime_error("The iterator passed to `insert` is out of bounds."); + } if (this->size() >= this->capacity()) { size_t EltNo = I - this->begin(); @@ -759,8 +775,9 @@ public: return this->begin() + InsertElt; } - assert(I >= this->begin() && "Insertion iterator is out of bounds."); - assert(I <= this->end() && "Inserting past the end of the vector."); + if (I < this->begin() || I > this->end()) { + throw std::runtime_error("The iterator passed to `insert` is out of bounds."); + } // Ensure there is enough space. reserve(this->size() + NumToInsert); @@ -813,8 +830,9 @@ public: return this->begin() + InsertElt; } - assert(I >= this->begin() && "Insertion iterator is out of bounds."); - assert(I <= this->end() && "Inserting past the end of the vector."); + if (I < this->begin() || I > this->end()) { + throw std::runtime_error("The iterator passed to `insert` is out of bounds."); + } size_t NumToInsert = std::distance(From, To); @@ -1326,12 +1344,12 @@ public: reference operator[](size_type idx) { - assert(idx < size()); + R__ASSERT(idx < size()); return begin()[idx]; } const_reference operator[](size_type idx) const { - assert(idx < size()); + R__ASSERT(idx < size()); return begin()[idx]; } From 29fdc0edd25cb59a223a39bad74b31eb8a4ddd86 Mon Sep 17 00:00:00 2001 From: Enrico Guiraud Date: Tue, 15 Jun 2021 19:11:35 +0200 Subject: [PATCH 21/51] [VecOps][NFC] Add math/vecops/ARCHITECTURE.md This document is meant for ROOT developers, to quickly get their bearings around the VecOps library. See https://matklad.github.io/2021/02/06/ARCHITECTURE.md.html for more motivation. --- math/vecops/ARCHITECTURE.md | 103 ++++++++++++++++++++++++++++++++++++ 1 file changed, 103 insertions(+) create mode 100644 math/vecops/ARCHITECTURE.md diff --git a/math/vecops/ARCHITECTURE.md b/math/vecops/ARCHITECTURE.md new file mode 100644 index 0000000000000..3592032fdf9c2 --- /dev/null +++ b/math/vecops/ARCHITECTURE.md @@ -0,0 +1,103 @@ +# Architecture of the VecOps library + +> This document is meant for ROOT developers, to quickly get their bearings around the VecOps library. + +The main type in the library is `RVec`. Besides `RVec`, the library only contains helper types and functions. + +`RVec` is a vector type that tries to be as `std::vector`-like as possible while adding a +few important features, namely: +- the ability to act as a view over an existing memory buffer (see "Memory adoption" below) +- a small-buffer optimization +- vectorized operator overloads +- a vectorized `operator[](mask)` to allow quick element selection together with vectorized operators + (e.g. `etas[etas > k]` returns a new `RVec` with all elements greater than `k`) +- helper functions such as `InvariantMass`, `DeltaR`, `Argsort` are also provided + +The current implementation of `RVec` is based on LLVM's SmallVector, extracted +from the head of LLVM's repo around December 2020. +We are not tracking the upstream implementation. + +Compared to LLVM's SmallVectors: + +- memory adoption capabilities have been added +- patches have been applied to make RVec work with (ROOT's version of) cppyy (notably `using` declarations had to be + lowered in the inheritance hierarchy for cppyy to pick them up) +- `operator[](mask)` has been added, as well as several other "numpy-like" helper + functions (these latter ones are free functions) +- logical operators `==`, `<`, `>` etc. return vectors rather than booleans +- the type of fSize and fCapacity is signed rather than unsigned, and fixed to 32 bits +- a number of minor patches have been applied for backward compatibility with the previous + implementation of RVec (which did not have a small buffer optimization and was implemented + in terms of `std::vector` with a custom allocator) and to make the code more consistent + with ROOT's coding conventions + +## RVec design + +`SmallVectorBase` + - `fBeginX` + - `fSize` + - `fCapacity` + + Basically the same as the corresponding LLVM class, with the template parameter removed: LLVM's SmallVectorBase + is templated over the type of fSize and fCapacity. It contains the parts of `RVec` that do not depend on the value + type. + No other classes in the hierarchy can contain data members! We expect the memory after `SmallVectorBase` to be + occupied by the small inline buffer. + +`SmallVectorTemplateCommon` + - `getFirstEl()`: returns the address of the beginning of the small buffer + - `begin()`, `end()`, `front()`, `back()`, etc. + + Basically the same as the corresponding LLVM class. + It contains the parts that are independent of whether T is a POD or not. + +`SmallVectorTemplateBase` and the specialization `SmallVectorTemplateBase` + - `grow()`, `uninitialized_copy`, `uninitialized_move`, `push_back()`, `pop_back()` + + This class contains the parts of `RVec` that can be optimized for trivially copiable types. + In particular, destruction can be skipped and memcpy can be used in place of copy/move construction. + These optimizations are inherited from LLVM's SmallVector. + +`RVecImpl` + The analogous of LLVM's `SmallVectorImpl`, it factors out of `RVec` the parts that are independent of + the small buffer size, to limit the amount of code generated and provide a way to slice the small buffer + size when passing around `RVec` objects. + +`RVec>` + It aggregates `RVecImpl` and `SmallVectorStorage` (see below) through public inheritance. + `N` is the small buffer size and defaults to a sensible value that depends on `sizeof(T)`. + We expect most users to use the default and only rarely tweak the small buffer size. + +### Helper types + +- `SmallVectorAlignmentAndSize`: used to figure out the offset of the first small-buffer element in + `SmallVectorTemplateCommon::getFirstEl` +- `SmallVectorStorage`: properly aligned "small buffer" storage. It's a separate type so that it can be specialized to + be properly aligned also for the case of small buffer size = 0 + +## Memory adoption + +We need RVec to be able to act as a view over an existing buffer rather than use its own +to save copies and allocations when reading ROOT data into `RVec`s, e.g. in `RDataFrame`. + +The feature is exposed via a dedicated constructor: `RVec(pointer, size)`. +`RVec` then switches to its own storage as soon as a resize is requested. +`fCapacity == -1` indicates that we are in "memory adoption mode". + +## Exception safety guarantees + +As per [its docs](https://llvm.org/doxygen/classllvm_1_1SmallVector.html), LLVM's +`SmallVector` implementation "does not attempt to be exception-safe". +In its current implementation, `RVec` does not attempt to fix that. +This should not be a problem for `RVec`'s usecases (an exception thrown during +construction of an `RVec` typically means there is a bug to fix in the analysis code), +and we expect to be able to revisit the implementation and fix broken behavior if it +ever turns out to be problematic. + +Relevant discussion on GitHub: + +- https://github.com/root-project/root/pull/7502#issuecomment-818864506 +- https://github.com/root-project/root/pull/7502#issuecomment-818905333 +- https://github.com/root-project/root/pull/7502#issuecomment-821054757 + + From c320ee2a83fdcfd3c8dd3ce7cae8602ff86a2273 Mon Sep 17 00:00:00 2001 From: Enrico Guiraud Date: Mon, 28 Jun 2021 12:59:57 +0200 Subject: [PATCH 22/51] [VecOps] Introduce RVecN, which exposes the small buffer size RVec inherits from RVecN and fixes the small buffer size to a reasonable number. For now, all operators and functions are still defined in terms of RVec. --- math/vecops/inc/ROOT/RVec.hxx | 329 +++++++++++++++++++++------------- 1 file changed, 206 insertions(+), 123 deletions(-) diff --git a/math/vecops/inc/ROOT/RVec.hxx b/math/vecops/inc/ROOT/RVec.hxx index dff7e1b600785..3f44cfac50185 100644 --- a/math/vecops/inc/ROOT/RVec.hxx +++ b/math/vecops/inc/ROOT/RVec.hxx @@ -70,6 +70,17 @@ template class RVec; } +namespace Internal { +namespace VecOps { + +constexpr unsigned FirstOf(unsigned N0, ...) +{ + return N0; +} + +} // namespace VecOps +} // namespace Internal + namespace Detail { namespace VecOps { @@ -513,6 +524,11 @@ template struct alignas(T) SmallVectorStorage { }; +template +struct RVecInlineStorageSize { + static constexpr unsigned int value = 8; +}; + } // namespace VecOps } // namespace Internal @@ -1093,6 +1109,166 @@ namespace VecOps { * @{ */ +// From the original SmallVector code: +// This is a 'vector' (really, a variable-sized array), optimized +// for the case when the array is small. It contains some number of elements +// in-place, which allows it to avoid heap allocation when the actual number of +// elements is below that threshold. This allows normal "small" cases to be +// fast without losing generality for large inputs. +// +// Note that this does not attempt to be exception safe. + +template +class RVecN : public Detail::VecOps::RVecImpl, Internal::VecOps::SmallVectorStorage { +public: + RVecN() : Detail::VecOps::RVecImpl(N) {} + + ~RVecN() + { + if (this->Owns()) { + // Destroy the constructed elements in the vector. + this->destroy_range(this->begin(), this->end()); + } + } + + explicit RVecN(size_t Size, const T &Value = T()) : Detail::VecOps::RVecImpl(N) + { + this->assign(Size, Value); + } + + template ::iterator_category, std::input_iterator_tag>::value>::type> + RVecN(ItTy S, ItTy E) : Detail::VecOps::RVecImpl(N) + { + this->append(S, E); + } + + RVecN(std::initializer_list IL) : Detail::VecOps::RVecImpl(N) { this->assign(IL); } + + RVecN(const RVecN &RHS) : Detail::VecOps::RVecImpl(N) + { + if (!RHS.empty()) + Detail::VecOps::RVecImpl::operator=(RHS); + } + + RVecN &operator=(const RVecN &RHS) + { + Detail::VecOps::RVecImpl::operator=(RHS); + return *this; + } + + RVecN(RVecN &&RHS) : Detail::VecOps::RVecImpl(N) + { + if (!RHS.empty()) + Detail::VecOps::RVecImpl::operator=(::std::move(RHS)); + } + + RVecN(Detail::VecOps::RVecImpl &&RHS) : Detail::VecOps::RVecImpl(N) + { + if (!RHS.empty()) + Detail::VecOps::RVecImpl::operator=(::std::move(RHS)); + } + + RVecN(const std::vector &RHS) : RVecN(RHS.begin(), RHS.end()) {} + + RVecN &operator=(RVecN &&RHS) + { + Detail::VecOps::RVecImpl::operator=(::std::move(RHS)); + return *this; + } + + RVecN(T* p, size_t n) : Detail::VecOps::RVecImpl(N) + { + this->fBeginX = p; + this->fSize = n; + this->fCapacity = -1; + } + + RVecN &operator=(Detail::VecOps::RVecImpl &&RHS) + { + Detail::VecOps::RVecImpl::operator=(::std::move(RHS)); + return *this; + } + + RVecN &operator=(std::initializer_list IL) + { + this->assign(IL); + return *this; + } + + using reference = typename Internal::VecOps::SmallVectorTemplateCommon::reference; + using const_reference = typename Internal::VecOps::SmallVectorTemplateCommon::const_reference; + using size_type = typename Internal::VecOps::SmallVectorTemplateCommon::size_type; + using value_type = typename Internal::VecOps::SmallVectorTemplateCommon::value_type; + using Internal::VecOps::SmallVectorTemplateCommon::begin; + using Internal::VecOps::SmallVectorTemplateCommon::size; + + reference operator[](size_type idx) + { + R__ASSERT(idx < size()); + return begin()[idx]; + } + const_reference operator[](size_type idx) const + { + R__ASSERT(idx < size()); + return begin()[idx]; + } + + template ::value>> + RVecN operator[](const RVecN &conds) const + { + const auto n = conds.size(); + + if (n != this->size()) + throw std::runtime_error("Cannot index RVecN with condition vector of different size"); + + RVecN ret; + ret.reserve(n); + for (auto i = 0u; i < n; ++i) + if (conds[i]) + ret.emplace_back(this->operator[](i)); + return ret; + } + + // conversion + template ::value>> + operator RVecN() const + { + return RVecN(this->begin(), this->end()); + } + + reference at(size_type pos) + { + if (pos >= size_type(this->fSize)) + throw std::out_of_range("RVecN"); + return this->operator[](pos); + } + + const_reference at(size_type pos) const + { + if (pos >= size_type(this->fSize)) + throw std::out_of_range("RVecN"); + return this->operator[](pos); + } + + /// No exception thrown. The user specifies the desired value in case the RVecN is shorter than `pos`. + value_type at(size_type pos, value_type fallback) + { + if (pos >= size_type(this->fSize)) + return fallback; + return this->operator[](pos); + } + + /// No exception thrown. The user specifies the desired value in case the RVecN is shorter than `pos`. + value_type at(size_type pos, value_type fallback) const + { + if (pos >= size_type(this->fSize)) + return fallback; + return this->operator[](pos); + } +}; + // clang-format off /** \class ROOT::VecOps::RVec @@ -1244,130 +1420,57 @@ hpt->Draw(); **/ // clang-format on - - -// From the original SmallVector code: -// This is a 'vector' (really, a variable-sized array), optimized -// for the case when the array is small. It contains some number of elements -// in-place, which allows it to avoid heap allocation when the actual number of -// elements is below that threshold. This allows normal "small" cases to be -// fast without losing generality for large inputs. -// -// Note that this does not attempt to be exception safe. - template -class RVec : public Detail::VecOps::RVecImpl, Internal::VecOps::SmallVectorStorage { +class RVec : public RVecN::value> { + using SuperClass = RVecN::value>; public: - static constexpr unsigned N = 8; - RVec() : Detail::VecOps::RVecImpl(N) {} + using reference = typename SuperClass::reference; + using const_reference = typename SuperClass::const_reference; + using size_type = typename SuperClass::size_type; + using value_type = typename SuperClass::value_type; + using SuperClass::begin; + using SuperClass::size; - ~RVec() - { - if (this->Owns()) { - // Destroy the constructed elements in the vector. - this->destroy_range(this->begin(), this->end()); - } - } + RVec() {} - explicit RVec(size_t Size, const T &Value = T()) : Detail::VecOps::RVecImpl(N) - { - this->assign(Size, Value); - } + explicit RVec(size_t Size, const T &Value = T()) : SuperClass(Size, Value) {} template ::iterator_category, std::input_iterator_tag>::value>::type> - RVec(ItTy S, ItTy E) : Detail::VecOps::RVecImpl(N) + RVec(ItTy S, ItTy E) : SuperClass(S, E) { - this->append(S, E); } - RVec(std::initializer_list IL) : Detail::VecOps::RVecImpl(N) { this->assign(IL); } + RVec(std::initializer_list IL) : SuperClass(IL) {} - RVec(const RVec &RHS) : Detail::VecOps::RVecImpl(N) - { - if (!RHS.empty()) - Detail::VecOps::RVecImpl::operator=(RHS); - } + RVec(const RVec &RHS) : SuperClass(RHS) {} RVec &operator=(const RVec &RHS) { - Detail::VecOps::RVecImpl::operator=(RHS); + SuperClass::operator=(RHS); return *this; } - RVec(RVec &&RHS) : Detail::VecOps::RVecImpl(N) - { - if (!RHS.empty()) - Detail::VecOps::RVecImpl::operator=(::std::move(RHS)); - } - - RVec(Detail::VecOps::RVecImpl &&RHS) : Detail::VecOps::RVecImpl(N) - { - if (!RHS.empty()) - Detail::VecOps::RVecImpl::operator=(::std::move(RHS)); - } - - RVec(const std::vector &RHS) : RVec(RHS.begin(), RHS.end()) {} + RVec(RVec &&RHS) : SuperClass(std::move(RHS)) {} RVec &operator=(RVec &&RHS) { - Detail::VecOps::RVecImpl::operator=(::std::move(RHS)); - return *this; - } - - RVec(T* p, size_t n) : Detail::VecOps::RVecImpl(N) - { - this->fBeginX = p; - this->fSize = n; - this->fCapacity = -1; - } - - RVec &operator=(Detail::VecOps::RVecImpl &&RHS) - { - Detail::VecOps::RVecImpl::operator=(::std::move(RHS)); + SuperClass::operator=(std::move(RHS)); return *this; } - RVec &operator=(std::initializer_list IL) - { - this->assign(IL); - return *this; - } + RVec(Detail::VecOps::RVecImpl &&RHS) : SuperClass(std::move(RHS)) {} - using reference = typename Internal::VecOps::SmallVectorTemplateCommon::reference; - using const_reference = typename Internal::VecOps::SmallVectorTemplateCommon::const_reference; - using size_type = typename Internal::VecOps::SmallVectorTemplateCommon::size_type; - using value_type = typename Internal::VecOps::SmallVectorTemplateCommon::value_type; - using Internal::VecOps::SmallVectorTemplateCommon::begin; - using Internal::VecOps::SmallVectorTemplateCommon::size; + template + RVec(RVecN &&RHS) : SuperClass(std::move(RHS)) {} - reference operator[](size_type idx) - { - R__ASSERT(idx < size()); - return begin()[idx]; - } - const_reference operator[](size_type idx) const - { - R__ASSERT(idx < size()); - return begin()[idx]; - } + template + RVec(const RVecN &RHS) : SuperClass(RHS) {} - template ::value>> - RVec operator[](const RVec &conds) const - { - const auto n = conds.size(); + RVec(const std::vector &RHS) : SuperClass(RHS) {} - if (n != this->size()) - throw std::runtime_error("Cannot index RVec with condition vector of different size"); - - RVec ret; - ret.reserve(n); - for (auto i = 0u; i < n; ++i) - if (conds[i]) - ret.emplace_back(this->operator[](i)); - return ret; - } + RVec(T* p, size_t n) : SuperClass(p, n) {} // conversion template ::value>> @@ -1376,39 +1479,19 @@ public: return RVec(this->begin(), this->end()); } - reference at(size_type pos) - { - if (pos >= size_type(this->fSize)) - throw std::out_of_range("RVec"); - return this->operator[](pos); - } + using SuperClass::operator[]; - const_reference at(size_type pos) const - { - if (pos >= size_type(this->fSize)) - throw std::out_of_range("RVec"); - return this->operator[](pos); - } - - /// No exception thrown. The user specifies the desired value in case the RVec is shorter than `pos`. - value_type at(size_type pos, value_type fallback) + template ::value>> + RVec operator[](const RVec &conds) const { - if (pos >= size_type(this->fSize)) - return fallback; - return this->operator[](pos); + return RVec(SuperClass::operator[](conds)); } - /// No exception thrown. The user specifies the desired value in case the RVec is shorter than `pos`. - value_type at(size_type pos, value_type fallback) const - { - if (pos >= size_type(this->fSize)) - return fallback; - return this->operator[](pos); - } + using SuperClass::at; }; -template -inline size_t capacity_in_bytes(const RVec &X) +template +inline size_t capacity_in_bytes(const RVecN &X) { return X.capacity_in_bytes(); } From 03d030ee2c4f0802279c35318fb584a2912a1b99 Mon Sep 17 00:00:00 2001 From: Enrico Guiraud Date: Mon, 28 Jun 2021 17:51:50 +0200 Subject: [PATCH 23/51] [VecOps][NFC] Update ARCHITECTURE.md --- math/vecops/ARCHITECTURE.md | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/math/vecops/ARCHITECTURE.md b/math/vecops/ARCHITECTURE.md index 3592032fdf9c2..bf9720989f5ac 100644 --- a/math/vecops/ARCHITECTURE.md +++ b/math/vecops/ARCHITECTURE.md @@ -63,10 +63,13 @@ Compared to LLVM's SmallVectors: the small buffer size, to limit the amount of code generated and provide a way to slice the small buffer size when passing around `RVec` objects. -`RVec>` +`RVecN` It aggregates `RVecImpl` and `SmallVectorStorage` (see below) through public inheritance. - `N` is the small buffer size and defaults to a sensible value that depends on `sizeof(T)`. - We expect most users to use the default and only rarely tweak the small buffer size. + `N` is the small buffer size. + +`RVec` + Inherits from `RVecN` and fixes the small buffer size `N` to a reasonable default. + We expect most users to use this type and only very rarely switch to `RVecN` to tweak the small buffer size. ### Helper types From 963aed03af392f6550cc7c2bd8c195526c4e8b24 Mon Sep 17 00:00:00 2001 From: Enrico Guiraud Date: Mon, 28 Jun 2021 17:52:27 +0200 Subject: [PATCH 24/51] [VecOps] Make the small buffer size of RVec a function of T Our policy is to allocate at least 8 elements (or more if they all fit into one cacheline) unless the size of the buffer with 8 elements would be over a certain maximum size. --- math/vecops/ARCHITECTURE.md | 1 + math/vecops/inc/ROOT/RVec.hxx | 17 ++++++++++++++++- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/math/vecops/ARCHITECTURE.md b/math/vecops/ARCHITECTURE.md index bf9720989f5ac..88fa3dfbcc469 100644 --- a/math/vecops/ARCHITECTURE.md +++ b/math/vecops/ARCHITECTURE.md @@ -77,6 +77,7 @@ Compared to LLVM's SmallVectors: `SmallVectorTemplateCommon::getFirstEl` - `SmallVectorStorage`: properly aligned "small buffer" storage. It's a separate type so that it can be specialized to be properly aligned also for the case of small buffer size = 0 +- `RVecInlineStorageSize`: calculates the size of the small buffer in `RVec`. ## Memory adoption diff --git a/math/vecops/inc/ROOT/RVec.hxx b/math/vecops/inc/ROOT/RVec.hxx index 3f44cfac50185..ce4b792b70a0e 100644 --- a/math/vecops/inc/ROOT/RVec.hxx +++ b/math/vecops/inc/ROOT/RVec.hxx @@ -524,9 +524,24 @@ template struct alignas(T) SmallVectorStorage { }; +/// The size of the inline storage of an RVec. +/// Our policy is to allocate at least 8 elements (or more if they all fit into one cacheline) +/// unless the size of the buffer with 8 elements would be over a certain maximum size. template struct RVecInlineStorageSize { - static constexpr unsigned int value = 8; +private: +#ifdef R__HAS_HARDWARE_INTERFERENCE_SIZE + constexpr std::size_t cacheLineSize = std::hardware_destructive_interference_size; +#else + // safe bet: assume the typical 64 bytes + static constexpr std::size_t cacheLineSize = 64; +#endif + static constexpr unsigned elementsPerCacheLine = (cacheLineSize - sizeof(SmallVectorBase)) / sizeof(T); + static constexpr unsigned maxInlineByteSize = 1024; + +public: + static constexpr unsigned value = + elementsPerCacheLine >= 8 ? elementsPerCacheLine : (sizeof(T) * 8 > maxInlineByteSize ? 0 : 8); }; } // namespace VecOps From a31fe1b306573d7e305d8d0e53c67e8e9097597d Mon Sep 17 00:00:00 2001 From: Enrico Guiraud Date: Thu, 1 Jul 2021 17:44:01 +0200 Subject: [PATCH 25/51] [VecOps] Avoid costly asserts in optimized builds --- math/vecops/inc/ROOT/RVec.hxx | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/math/vecops/inc/ROOT/RVec.hxx b/math/vecops/inc/ROOT/RVec.hxx index ce4b792b70a0e..daa436dd03d0c 100644 --- a/math/vecops/inc/ROOT/RVec.hxx +++ b/math/vecops/inc/ROOT/RVec.hxx @@ -1221,12 +1221,16 @@ public: reference operator[](size_type idx) { +#ifndef NDEBUG R__ASSERT(idx < size()); +#endif return begin()[idx]; } const_reference operator[](size_type idx) const { +#ifndef NDEBUG R__ASSERT(idx < size()); +#endif return begin()[idx]; } From 17b4399b3490eb575ede7265403bf5b6af698305 Mon Sep 17 00:00:00 2001 From: Enrico Guiraud Date: Sat, 3 Jul 2021 13:17:19 +0200 Subject: [PATCH 26/51] [VecOps] Add SmallVectorBase to LinkDef --- math/vecops/inc/LinkDef.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/math/vecops/inc/LinkDef.h b/math/vecops/inc/LinkDef.h index 3b3c2b5495b34..ebdbc6cf7f1a4 100644 --- a/math/vecops/inc/LinkDef.h +++ b/math/vecops/inc/LinkDef.h @@ -11,6 +11,8 @@ #pragma link C++ namespace ROOT::VecOps; +#pragma link C++ class ROOT::Internal::VecOps::SmallVectorBase-; + #pragma link C++ class ROOT::VecOps::RVec-; #pragma link C++ class ROOT::VecOps::RVec-; From 7c0c45741419e270a53b7176d9ed99c1c78f80b0 Mon Sep 17 00:00:00 2001 From: Enrico Guiraud Date: Mon, 5 Jul 2021 11:41:27 +0200 Subject: [PATCH 27/51] [VecOps][NFC] Remove unused include --- math/vecops/inc/ROOT/RVec.hxx | 1 - 1 file changed, 1 deletion(-) diff --git a/math/vecops/inc/ROOT/RVec.hxx b/math/vecops/inc/ROOT/RVec.hxx index daa436dd03d0c..406665a1e4f18 100644 --- a/math/vecops/inc/ROOT/RVec.hxx +++ b/math/vecops/inc/ROOT/RVec.hxx @@ -41,7 +41,6 @@ #define _VECOPS_USE_EXTERN_TEMPLATES true #endif -#include #include #include // R__ASSERT #include From 4ae2da8060ac582f9674ae83af3735427eba68a4 Mon Sep 17 00:00:00 2001 From: Enrico Guiraud Date: Mon, 5 Jul 2021 11:42:48 +0200 Subject: [PATCH 28/51] [VecOps] Add test for interaction between SBO and memory adoption --- math/vecops/test/vecops_rvec.cxx | 37 ++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/math/vecops/test/vecops_rvec.cxx b/math/vecops/test/vecops_rvec.cxx index 7c5139bcfb4e4..c2c721f2b8be8 100644 --- a/math/vecops/test/vecops_rvec.cxx +++ b/math/vecops/test/vecops_rvec.cxx @@ -1116,14 +1116,51 @@ TEST(VecOps, Construct) EXPECT_TRUE(fourVects[2] == ref2); } +bool IsSmall(const RVec &v) +{ + // the first array element is right after the 3 data members of SmallVectorBase + return reinterpret_cast(v.begin()) - reinterpret_cast(&v) == 16u; +} + // this is a regression test for https://github.com/root-project/root/issues/6796 TEST(VecOps, MemoryAdoptionAndClear) { ROOT::RVec v{1, 2, 3}; + EXPECT_TRUE(IsSmall(v)); ROOT::RVec v2(v.data(), v.size()); v2[0] = 0; v2.clear(); EXPECT_TRUE(All(v == RVec{0, 2, 3})); v2.push_back(42); + EXPECT_FALSE(IsSmall(v2)); // currently RVec does not go back to a small state after `clear()` EXPECT_TRUE(All(v2 == RVec{42})); } + +// interaction between small buffer optimization and memory adoption +TEST(VecOps, MemoryAdoptionAndSBO) +{ + int *values = new int[3]{1, 2, 3}; + ROOT::RVec v(values, 3); + auto check = [](const RVec &mv) { + EXPECT_EQ(mv.size(), 3u); + EXPECT_EQ(mv[0], 1); + EXPECT_EQ(mv[1], 2); + EXPECT_EQ(mv[2], 3); + }; + check(v); + EXPECT_FALSE(IsSmall(v)); + ROOT::RVec v2 = std::move(v); + EXPECT_TRUE(v.empty()); + check(v2); + // this changes the RVec from memory adoption mode to "long" mode, even if the size is small + // currently we don't allow going from memory adoption to small buffer mode directly, it could be future optimization + v2.push_back(4); + EXPECT_FALSE(IsSmall(v2)); + v2.clear(); + v2.push_back(1); + v2.push_back(2); + v2.push_back(3); + check(v2); + delete[] values; + check(v2); +} From 034d7c546aab4019575fdce11464fab3e334e98d Mon Sep 17 00:00:00 2001 From: Enrico Guiraud Date: Mon, 5 Jul 2021 13:00:39 +0200 Subject: [PATCH 29/51] [VecOps] Fix reallocation with memory adoption and POD types Before this patch, `grow_pod` called `realloc` to expand the memory buffer in case the RVec was in "memory view" mode. Instead we want a full malloc in case we are adopting memory. --- math/vecops/src/RVec.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/math/vecops/src/RVec.cxx b/math/vecops/src/RVec.cxx index bb2829baea554..09f56c995b534 100644 --- a/math/vecops/src/RVec.cxx +++ b/math/vecops/src/RVec.cxx @@ -58,7 +58,7 @@ void ROOT::Internal::VecOps::SmallVectorBase::grow_pod(void *FirstEl, size_t Min NewCapacity = std::min(std::max(NewCapacity, MinSize), SizeTypeMax()); void *NewElts; - if (fBeginX == FirstEl) { + if (fBeginX == FirstEl || !this->Owns()) { NewElts = malloc(NewCapacity * TSize); // Copy the elements over. No need to run dtors on PODs. From 72a108a2748e0b0bfc3dd634d87f9ab0fe71951f Mon Sep 17 00:00:00 2001 From: Enrico Guiraud Date: Fri, 9 Jul 2021 13:09:31 +0200 Subject: [PATCH 30/51] [VecOps] Avoid comparison between uint32 and uint64 This greatly reduces the number of branch misses measured in our RVecMaskingSimple micro-benchmark, presumably because it removes the need for a bunch of integer-overflow-checking logic. Co-authored-by: Jonas Hahnfeld --- math/vecops/inc/ROOT/RVec.hxx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/math/vecops/inc/ROOT/RVec.hxx b/math/vecops/inc/ROOT/RVec.hxx index 406665a1e4f18..9c0c1f66cf8b4 100644 --- a/math/vecops/inc/ROOT/RVec.hxx +++ b/math/vecops/inc/ROOT/RVec.hxx @@ -1236,14 +1236,14 @@ public: template ::value>> RVecN operator[](const RVecN &conds) const { - const auto n = conds.size(); + const size_type n = conds.size(); if (n != this->size()) throw std::runtime_error("Cannot index RVecN with condition vector of different size"); RVecN ret; ret.reserve(n); - for (auto i = 0u; i < n; ++i) + for (size_type i = 0u; i < n; ++i) if (conds[i]) ret.emplace_back(this->operator[](i)); return ret; From a6b1d4f619d2158c03df287c857d61fa0fee52d4 Mon Sep 17 00:00:00 2001 From: Enrico Guiraud Date: Fri, 9 Jul 2021 13:13:16 +0200 Subject: [PATCH 31/51] [VecOps] Make sure small buffer is always zero-initialized This removes an uninitialized memory warning in dataframe_splitcoll_arrayview with gcc 11 and seems to also improve branch predition for some of our micro-benchmarks. --- math/vecops/inc/ROOT/RVec.hxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/math/vecops/inc/ROOT/RVec.hxx b/math/vecops/inc/ROOT/RVec.hxx index 9c0c1f66cf8b4..9c9ddd5468813 100644 --- a/math/vecops/inc/ROOT/RVec.hxx +++ b/math/vecops/inc/ROOT/RVec.hxx @@ -513,7 +513,7 @@ public: /// to avoid allocating unnecessary storage. template struct SmallVectorStorage { - alignas(T) char InlineElts[N * sizeof(T)]; + alignas(T) char InlineElts[N * sizeof(T)]{}; }; /// We need the storage to be properly aligned even for small-size of 0 so that From 25a05708ceefefac972322be037431e65f627a9f Mon Sep 17 00:00:00 2001 From: Enrico Guiraud Date: Tue, 13 Jul 2021 16:15:19 +0200 Subject: [PATCH 32/51] [VecOps] Spell out the constructor with default element initialization This is a performance optimization. GCC 11 generates worse code if we use assign(Size, T()) instead of spelling out what we want. --- math/vecops/inc/ROOT/RVec.hxx | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/math/vecops/inc/ROOT/RVec.hxx b/math/vecops/inc/ROOT/RVec.hxx index 9c9ddd5468813..23a25704de08c 100644 --- a/math/vecops/inc/ROOT/RVec.hxx +++ b/math/vecops/inc/ROOT/RVec.hxx @@ -1145,9 +1145,14 @@ public: } } - explicit RVecN(size_t Size, const T &Value = T()) : Detail::VecOps::RVecImpl(N) + explicit RVecN(size_t Size, const T &Value) : Detail::VecOps::RVecImpl(N) { this->assign(Size, Value); } + + explicit RVecN(size_t Size) : Detail::VecOps::RVecImpl(N) { - this->assign(Size, Value); + if (Size > N) + this->grow(Size); + this->fSize = Size; + std::uninitialized_fill(this->begin(), this->end(), T{}); } template Date: Tue, 13 Jul 2021 19:53:31 +0200 Subject: [PATCH 33/51] [VecOps][NFC] Point to ARCHITECTURE.md instead of repeating info --- math/vecops/inc/ROOT/RVec.hxx | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/math/vecops/inc/ROOT/RVec.hxx b/math/vecops/inc/ROOT/RVec.hxx index 23a25704de08c..fd9be8d28dbc0 100644 --- a/math/vecops/inc/ROOT/RVec.hxx +++ b/math/vecops/inc/ROOT/RVec.hxx @@ -1,4 +1,5 @@ // Author: Enrico Guiraud, Enric Tejedor, Danilo Piparo CERN 04/2021 +// See /math/vecops/ARCHITECTURE.md for more information. /************************************************************************* * Copyright (C) 1995-2021, Rene Brun and Fons Rademakers. * @@ -8,17 +9,6 @@ * For the list of contributors see $ROOTSYS/README/CREDITS. * *************************************************************************/ -/* - * The implementation of RVec is based on LLVM's SmallVector. - * - * Compared to LLVM's SmallVectors, the ability to adopt an existing memory buffer has been added, - * as well as other minor patches for backward compatibility with - * the previous implementation of RVec (which did not have a small buffer - * optimization and was based on std::vector with a custom allocator) and - * for compatibility with cppyy. - * RVec also adds a vectorized operator[] and many "numpy-like" helper functions. - */ - #ifndef ROOT_RVEC #define ROOT_RVEC From 0f792cea09c4135c6ecfa99ea1291ba80632c3c6 Mon Sep 17 00:00:00 2001 From: Enrico Guiraud Date: Tue, 13 Jul 2021 19:59:05 +0200 Subject: [PATCH 34/51] [VecOps][NFC] Add disclaimer about lack of exception-safety --- math/vecops/inc/ROOT/RVec.hxx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/math/vecops/inc/ROOT/RVec.hxx b/math/vecops/inc/ROOT/RVec.hxx index fd9be8d28dbc0..e698b7c2c9d5b 100644 --- a/math/vecops/inc/ROOT/RVec.hxx +++ b/math/vecops/inc/ROOT/RVec.hxx @@ -1293,6 +1293,9 @@ Its storage is contiguous in memory and its interface is designed such to resemb of the stl vector. In addition the interface features methods and external functions to ease the manipulation and analysis of the data in the RVec. +\note RVec does not attempt to be exception safe. Exceptions thrown by element constructors during insertions, swaps or +other operations will be propagated potentially leaving the RVec object in an invalid state. + \htmlonly DOI \endhtmlonly From 125c1ff2f725b76d4491ea0bf49420baa500eefa Mon Sep 17 00:00:00 2001 From: Enrico Guiraud Date: Tue, 13 Jul 2021 22:38:40 +0200 Subject: [PATCH 35/51] [VecOps] Avoid using emplace_back in vectorized operator[] The previous implementation of the vectorized operator[] was using emplace_back to add elements to the returned vector. emplace_back is slightly more complex in RVec 2.0 due to the SBO and compilers sometimes do not inline it. Instead, spell out the appropriate logic: for example we know the return vector has sufficient capacity and we can skip the related checks performed at every emplace_back call. --- math/vecops/inc/ROOT/RVec.hxx | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/math/vecops/inc/ROOT/RVec.hxx b/math/vecops/inc/ROOT/RVec.hxx index e698b7c2c9d5b..20fa42bdeb0d7 100644 --- a/math/vecops/inc/ROOT/RVec.hxx +++ b/math/vecops/inc/ROOT/RVec.hxx @@ -1238,9 +1238,15 @@ public: RVecN ret; ret.reserve(n); - for (size_type i = 0u; i < n; ++i) - if (conds[i]) - ret.emplace_back(this->operator[](i)); + size_type j = 0u; + for (size_type i = 0u; i < n; ++i) { + if (conds[i]) { + // the begin() is to go around the R__ASSERT in operator[] + ret.begin()[j] = this->operator[](i); + ++j; + } + } + ret.set_size(j); return ret; } From ee56782a462eb4d5b8fca55afcc64d6822ef2ba1 Mon Sep 17 00:00:00 2001 From: Enrico Guiraud Date: Wed, 14 Jul 2021 12:01:41 +0200 Subject: [PATCH 36/51] [VecOps] Remove unused logical operators from RVecImpl We define vectorized ones for RVecs that are used instead. --- math/vecops/inc/ROOT/RVec.hxx | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/math/vecops/inc/ROOT/RVec.hxx b/math/vecops/inc/ROOT/RVec.hxx index 20fa42bdeb0d7..365ec1292151e 100644 --- a/math/vecops/inc/ROOT/RVec.hxx +++ b/math/vecops/inc/ROOT/RVec.hxx @@ -921,19 +921,6 @@ public: RVecImpl &operator=(const RVecImpl &RHS); RVecImpl &operator=(RVecImpl &&RHS); - - bool operator==(const RVecImpl &RHS) const - { - if (this->size() != RHS.size()) - return false; - return std::equal(this->begin(), this->end(), RHS.begin()); - } - bool operator!=(const RVecImpl &RHS) const { return !(*this == RHS); } - - bool operator<(const RVecImpl &RHS) const - { - return std::lexicographical_compare(this->begin(), this->end(), RHS.begin(), RHS.end()); - } }; template From 71a31430a299d08b3e77b7e854aabf5ecb8e429b Mon Sep 17 00:00:00 2001 From: Enrico Guiraud Date: Wed, 14 Jul 2021 12:28:06 +0200 Subject: [PATCH 37/51] [VecOps] Fix `IsSmall` impl. on 32bit machines sizeof(void*) is 4, not 8 on 32bit machines. --- math/vecops/test/vecops_rvec.cxx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/math/vecops/test/vecops_rvec.cxx b/math/vecops/test/vecops_rvec.cxx index c2c721f2b8be8..eef4f45d9251a 100644 --- a/math/vecops/test/vecops_rvec.cxx +++ b/math/vecops/test/vecops_rvec.cxx @@ -1119,7 +1119,8 @@ TEST(VecOps, Construct) bool IsSmall(const RVec &v) { // the first array element is right after the 3 data members of SmallVectorBase - return reinterpret_cast(v.begin()) - reinterpret_cast(&v) == 16u; + return reinterpret_cast(v.begin()) - reinterpret_cast(&v) == + sizeof(void *) + 2 * sizeof(int); } // this is a regression test for https://github.com/root-project/root/issues/6796 From 5c6b93ffc94bf40e5d6066eb3fbfdecbdf04ae3c Mon Sep 17 00:00:00 2001 From: Enrico Guiraud Date: Tue, 13 Jul 2021 20:13:53 +0200 Subject: [PATCH 38/51] [VecOps] Test RVec does not badly crash in case of exceptions --- math/vecops/test/vecops_rvec.cxx | 61 ++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/math/vecops/test/vecops_rvec.cxx b/math/vecops/test/vecops_rvec.cxx index eef4f45d9251a..4d647f71cbd5c 100644 --- a/math/vecops/test/vecops_rvec.cxx +++ b/math/vecops/test/vecops_rvec.cxx @@ -1165,3 +1165,64 @@ TEST(VecOps, MemoryAdoptionAndSBO) delete[] values; check(v2); } + +struct ThrowingCtor { + ThrowingCtor() { throw std::runtime_error("This exception should have been caught."); } +}; + +struct ThrowingMove { + ThrowingMove() = default; + ThrowingMove(const ThrowingMove &) = default; + ThrowingMove &operator=(const ThrowingMove &) = default; + ThrowingMove(ThrowingMove &&) { throw std::runtime_error("This exception should have been caught."); } + ThrowingMove &operator=(ThrowingMove &&) { throw std::runtime_error("This exception should have been caught."); } +}; + +struct ThrowingCopy { + ThrowingCopy() = default; + ThrowingCopy(const ThrowingCopy &) { throw std::runtime_error("This exception should have been caught."); } + ThrowingCopy &operator=(const ThrowingCopy &) + { + throw std::runtime_error("This exception should have been caught."); + } + ThrowingCopy(ThrowingCopy &&) = default; + ThrowingCopy &operator=(ThrowingCopy &&) = default; +}; + +// RVec does not guarantee exception safety, but we still want to test +// that we don't segfault or otherwise crash if element construction or move throws. +TEST(VecOps, NoExceptionSafety) +{ + EXPECT_NO_THROW(ROOT::RVec()); + + EXPECT_THROW(ROOT::RVec(1), std::runtime_error); + EXPECT_THROW(ROOT::RVec(42), std::runtime_error); + ROOT::RVec v1; + EXPECT_THROW(v1.push_back(ThrowingCtor{}), std::runtime_error); + ROOT::RVec v2; + EXPECT_THROW(v2.emplace_back(ThrowingCtor{}), std::runtime_error); + + ROOT::RVec v3(2); + ROOT::RVec v4(42); + EXPECT_THROW(std::swap(v3, v4), std::runtime_error); + ThrowingMove tm; + EXPECT_THROW(v3.emplace_back(std::move(tm)), std::runtime_error); + + ROOT::RVec v7; + EXPECT_THROW(std::fill_n(std::back_inserter(v7), 16, ThrowingCopy{}), std::runtime_error); + + // now with memory adoption + ThrowingCtor *p1 = new ThrowingCtor[0]; + ROOT::RVec v8(p1, 0); + EXPECT_THROW(v8.push_back(ThrowingCtor{}), std::runtime_error); + delete[] p1; + + ThrowingMove *p2 = new ThrowingMove[2]; + ROOT::RVec v9(p2, 2); + EXPECT_THROW(std::swap(v9, v3), std::runtime_error); + delete[] p2; + + ThrowingCopy *p3 = new ThrowingCopy[2]; + ROOT::RVec v10(p3, 2); + EXPECT_THROW(v10.push_back(*p3), std::runtime_error); +} From 43e81371d596f7622e33fea30efdb05eba96ac89 Mon Sep 17 00:00:00 2001 From: Enrico Guiraud Date: Thu, 22 Jul 2021 11:18:51 +0200 Subject: [PATCH 39/51] [VecOps] Remove RVec's dependency on string_view As a simple standalone component, let's try to reduce its dependencies to the minimum. --- math/vecops/inc/ROOT/RVec.hxx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/math/vecops/inc/ROOT/RVec.hxx b/math/vecops/inc/ROOT/RVec.hxx index 365ec1292151e..29f7e2f83d3ee 100644 --- a/math/vecops/inc/ROOT/RVec.hxx +++ b/math/vecops/inc/ROOT/RVec.hxx @@ -31,7 +31,6 @@ #define _VECOPS_USE_EXTERN_TEMPLATES true #endif -#include #include // R__ASSERT #include @@ -77,7 +76,7 @@ template using RVec = ROOT::VecOps::RVec; template -std::size_t GetVectorsSize(std::string_view id, const RVec &... vs) +std::size_t GetVectorsSize(const std::string &id, const RVec &... vs) { constexpr const auto nArgs = sizeof...(T); const std::size_t sizes[] = {vs.size()...}; From 8e324dc9d6c038dfcecdb9697c3d46495fea6a3c Mon Sep 17 00:00:00 2001 From: Enrico Guiraud Date: Thu, 22 Jul 2021 11:19:57 +0200 Subject: [PATCH 40/51] [VecOps][NFC] Remove unused include --- math/vecops/inc/ROOT/RVec.hxx | 1 - 1 file changed, 1 deletion(-) diff --git a/math/vecops/inc/ROOT/RVec.hxx b/math/vecops/inc/ROOT/RVec.hxx index 29f7e2f83d3ee..1e873221d0ba9 100644 --- a/math/vecops/inc/ROOT/RVec.hxx +++ b/math/vecops/inc/ROOT/RVec.hxx @@ -32,7 +32,6 @@ #endif #include // R__ASSERT -#include #include #include From 12a4189504e290fcefe4779d7ba7ed4e563d0cef Mon Sep 17 00:00:00 2001 From: Enrico Guiraud Date: Tue, 7 Sep 2021 16:19:11 +0200 Subject: [PATCH 41/51] [VecOps] Remove unnecessary casts --- math/vecops/inc/ROOT/RVec.hxx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/math/vecops/inc/ROOT/RVec.hxx b/math/vecops/inc/ROOT/RVec.hxx index 1e873221d0ba9..8b4ed0a927a22 100644 --- a/math/vecops/inc/ROOT/RVec.hxx +++ b/math/vecops/inc/ROOT/RVec.hxx @@ -2083,7 +2083,7 @@ template auto Any(const RVec &v) -> decltype(v[0] == true) { for (auto &&e : v) - if (static_cast(e) == true) + if (e == true) return true; return false; } @@ -2102,7 +2102,7 @@ template auto All(const RVec &v) -> decltype(v[0] == false) { for (auto &&e : v) - if (static_cast(e) == false) + if (e == false) return false; return true; } From 9764f466026a971eb8401ed2d7a61356c03ede9c Mon Sep 17 00:00:00 2001 From: Enrico Guiraud Date: Wed, 29 Sep 2021 10:13:07 +0200 Subject: [PATCH 42/51] [NFC][VecOps] Order includes alphabetically --- math/vecops/inc/ROOT/RVec.hxx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/math/vecops/inc/ROOT/RVec.hxx b/math/vecops/inc/ROOT/RVec.hxx index 8b4ed0a927a22..11494d35512cf 100644 --- a/math/vecops/inc/ROOT/RVec.hxx +++ b/math/vecops/inc/ROOT/RVec.hxx @@ -36,14 +36,14 @@ #include #include #include -#include // for inner_product #include +#include // for inner_product #include #include +#include #include -#include #include -#include +#include #ifdef R__HAS_VDT #include From 350bfc35580e22f4e204c0f0aa754eb9044aa879 Mon Sep 17 00:00:00 2001 From: Enrico Guiraud Date: Wed, 29 Sep 2021 10:17:58 +0200 Subject: [PATCH 43/51] [VecOps] Remove now unused helper function --- math/vecops/inc/ROOT/RVec.hxx | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/math/vecops/inc/ROOT/RVec.hxx b/math/vecops/inc/ROOT/RVec.hxx index 11494d35512cf..1c5f3c9323f52 100644 --- a/math/vecops/inc/ROOT/RVec.hxx +++ b/math/vecops/inc/ROOT/RVec.hxx @@ -57,17 +57,6 @@ template class RVec; } -namespace Internal { -namespace VecOps { - -constexpr unsigned FirstOf(unsigned N0, ...) -{ - return N0; -} - -} // namespace VecOps -} // namespace Internal - namespace Detail { namespace VecOps { From 81c8e8afe740fa4e911c0534b6c15ea875d81f66 Mon Sep 17 00:00:00 2001 From: Enrico Guiraud Date: Wed, 29 Sep 2021 10:32:08 +0200 Subject: [PATCH 44/51] [VecOps] Move internal helper functions to internal namespace --- math/vecops/inc/ROOT/RVec.hxx | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/math/vecops/inc/ROOT/RVec.hxx b/math/vecops/inc/ROOT/RVec.hxx index 1c5f3c9323f52..6e602cc3eff36 100644 --- a/math/vecops/inc/ROOT/RVec.hxx +++ b/math/vecops/inc/ROOT/RVec.hxx @@ -57,7 +57,7 @@ template class RVec; } -namespace Detail { +namespace Internal { namespace VecOps { template @@ -100,11 +100,6 @@ auto MapFromTuple(Tuple_t &&t, std::index_sequence) return MapImpl(std::get(t), std::get(t)...); } -} // namespace VecOps -} // namespace Detail - -namespace Internal { -namespace VecOps { /// Return the next power of two (in 64-bits) that is strictly greater than A. /// Return zero on overflow. inline uint64_t NextPowerOf2(uint64_t A) @@ -2018,8 +2013,6 @@ double StdDev(const RVec &v) /// ~~~ template auto Map(Args &&... args) - -> decltype(ROOT::Detail::VecOps::MapFromTuple(std::forward_as_tuple(args...), - std::make_index_sequence())) { /* Here the strategy in order to generalise the previous implementation of Map, i.e. @@ -2028,11 +2021,9 @@ auto Map(Args &&... args) This is achieved in two steps: 1. Forward as tuple the pack to MapFromTuple 2. Invoke the MapImpl helper which has the signature `template<...T, F> RVec MapImpl(F &&f, RVec...)` - NOTA BENE: the signature is very heavy but it is one of the lightest ways to manage in C++11 - to build the return type based on the template args. */ - return ROOT::Detail::VecOps::MapFromTuple(std::forward_as_tuple(args...), - std::make_index_sequence()); + return ROOT::Internal::VecOps::MapFromTuple(std::forward_as_tuple(args...), + std::make_index_sequence()); } /// Create a new collection with the elements passing the filter expressed by the predicate @@ -2774,7 +2765,7 @@ T InvariantMass(const RVec& pt, const RVec& eta, const RVec& phi, const template RVec Construct(const RVec &... args) { - const auto size = ::ROOT::Detail::VecOps::GetVectorsSize("Construct", args...); + const auto size = ::ROOT::Internal::VecOps::GetVectorsSize("Construct", args...); RVec ret; ret.reserve(size); for (auto i = 0UL; i < size; ++i) { From 17fa9f3b04866a814619c125a2cc531495b9b1b0 Mon Sep 17 00:00:00 2001 From: Enrico Guiraud Date: Wed, 29 Sep 2021 10:39:14 +0200 Subject: [PATCH 45/51] [NFC][VecOps] Update some comments --- math/vecops/inc/ROOT/RVec.hxx | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/math/vecops/inc/ROOT/RVec.hxx b/math/vecops/inc/ROOT/RVec.hxx index 6e602cc3eff36..5f7cc2cbcff98 100644 --- a/math/vecops/inc/ROOT/RVec.hxx +++ b/math/vecops/inc/ROOT/RVec.hxx @@ -114,15 +114,10 @@ inline uint64_t NextPowerOf2(uint64_t A) } /// This is all the stuff common to all SmallVectors. -/// -/// The template parameter specifies the type which should be used to hold the -/// Size and Capacity of the SmallVector, so it can be adjusted. -/// Using 32 bit size is desirable to shrink the size of the SmallVector. -/// Using 64 bit size is desirable for cases like SmallVector, where a -/// 32 bit size would limit the vector to ~4GB. SmallVectors are used for -/// buffering bitcode output - which can exceed 4GB. class SmallVectorBase { public: + // This limits the maximum size of an RVec to ~4GB but we don't expect this to ever be a problem, + // and we prefer the smaller Size_T to reduce the size of each RVec object. using Size_T = int32_t; protected: @@ -177,8 +172,8 @@ public: fSize = N; } - // LLVM SmallVector does not have a shrink_to_fit method - // it's technically ok to do nothing, but assuming no one uses this method for RVec anyway, I'd rather deprecate it + // LLVM SmallVector does not have a shrink_to_fit method, but RVec used to. + // It's technically ok to do nothing, but assuming no one uses this method for RVec anyway, I'd rather deprecate it R__DEPRECATED(6, 28, "This method will be removed.") void shrink_to_fit() { } }; From aa687a37278a470fbbb351d465d28fef94ca08a5 Mon Sep 17 00:00:00 2001 From: Enrico Guiraud Date: Wed, 29 Sep 2021 10:46:20 +0200 Subject: [PATCH 46/51] [VecOps] Remove unnecessary default template args --- math/vecops/inc/ROOT/RVec.hxx | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/math/vecops/inc/ROOT/RVec.hxx b/math/vecops/inc/ROOT/RVec.hxx index 5f7cc2cbcff98..61544fa047baf 100644 --- a/math/vecops/inc/ROOT/RVec.hxx +++ b/math/vecops/inc/ROOT/RVec.hxx @@ -178,17 +178,15 @@ public: void shrink_to_fit() { } }; -/// Figure out the offset of the first element. -template +/// Used to figure out the offset of the first element of an RVec +template struct SmallVectorAlignmentAndSize { alignas(SmallVectorBase) char Base[sizeof(SmallVectorBase)]; alignas(T) char FirstEl[sizeof(T)]; }; -/// This is the part of SmallVectorTemplateBase which does not depend on whether -/// the type T is a POD. The extra dummy template argument is used by ArrayRef -/// to avoid unnecessarily requiring T to be complete. -template +/// This is the part of SmallVectorTemplateBase which does not depend on whether the type T is a POD. +template class SmallVectorTemplateCommon : public SmallVectorBase { using Base = SmallVectorBase; From acb2d83265e7c2f0aad38e42620b01b33e0a756a Mon Sep 17 00:00:00 2001 From: Enrico Guiraud Date: Wed, 29 Sep 2021 10:59:06 +0200 Subject: [PATCH 47/51] [VecOps] Adapt and reinstate alignment, padding and size static checks --- math/vecops/src/RVec.cxx | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/math/vecops/src/RVec.cxx b/math/vecops/src/RVec.cxx index 09f56c995b534..4eb022b19b76a 100644 --- a/math/vecops/src/RVec.cxx +++ b/math/vecops/src/RVec.cxx @@ -1,7 +1,6 @@ #include "ROOT/RVec.hxx" using namespace ROOT::VecOps; -/***** FIXME the extra boolean breaks this assert and the two below -- maybe use unused bits in pointer? // Check that no bytes are wasted and everything is well-aligned. namespace { struct Struct16B { @@ -10,19 +9,18 @@ struct Struct16B { struct Struct32B { alignas(32) void *X; }; -} // namespace -static_assert(sizeof(SmallVector) == sizeof(unsigned) * 2 + sizeof(void *), - "wasted space in RVec size 0"); -static_assert(alignof(SmallVector) >= alignof(Struct16B), "wrong alignment for 16-byte aligned T"); -static_assert(alignof(SmallVector) >= alignof(Struct32B), "wrong alignment for 32-byte aligned T"); -static_assert(sizeof(SmallVector) >= alignof(Struct16B), "missing padding for 16-byte aligned T"); -static_assert(sizeof(SmallVector) >= alignof(Struct32B), "missing padding for 32-byte aligned T"); -static_assert(sizeof(SmallVector) == sizeof(unsigned) * 2 + sizeof(void *) * 2, +} +static_assert(alignof(RVec) >= alignof(Struct16B), "wrong alignment for 16-byte aligned T"); +static_assert(alignof(RVec) >= alignof(Struct32B), "wrong alignment for 32-byte aligned T"); +static_assert(sizeof(RVec) >= alignof(Struct16B), "missing padding for 16-byte aligned T"); +static_assert(sizeof(RVec) >= alignof(Struct32B), "missing padding for 32-byte aligned T"); +static_assert(sizeof(RVecN) == sizeof(int32_t) * 2 + sizeof(void *), "wasted space in RVec"); +static_assert(sizeof(RVecN) == sizeof(int32_t) * 2 + sizeof(void *) * 2, "wasted space in SmallVector size 1"); - -static_assert(sizeof(SmallVector) == sizeof(void *) * 2 + sizeof(void *), - "1 byte elements have word-sized type for size and capacity"); -************/ +static_assert(sizeof(RVec) == + sizeof(int32_t) * 2 + + (1 + ROOT::Internal::VecOps::RVecInlineStorageSize::value) * sizeof(void *), + "wasted space in RVec"); void ROOT::Internal::VecOps::SmallVectorBase::report_size_overflow(size_t MinSize) { From 18f831378cc1524f12be4bad79ab7d5640f89b2d Mon Sep 17 00:00:00 2001 From: Enrico Guiraud Date: Wed, 29 Sep 2021 11:02:39 +0200 Subject: [PATCH 48/51] [VecOps] Assert that malloc/realloc succeeded --- math/vecops/inc/ROOT/RVec.hxx | 1 + math/vecops/src/RVec.cxx | 2 ++ 2 files changed, 3 insertions(+) diff --git a/math/vecops/inc/ROOT/RVec.hxx b/math/vecops/inc/ROOT/RVec.hxx index 61544fa047baf..2f15a1b1a19f7 100644 --- a/math/vecops/inc/ROOT/RVec.hxx +++ b/math/vecops/inc/ROOT/RVec.hxx @@ -386,6 +386,7 @@ void SmallVectorTemplateBase::grow(size_t MinSize) size_t NewCapacity = size_t(NextPowerOf2(this->capacity() + 2)); NewCapacity = std::min(std::max(NewCapacity, MinSize), this->SizeTypeMax()); T *NewElts = static_cast(malloc(NewCapacity * sizeof(T))); + R__ASSERT(NewElts != nullptr); // Move the elements over. this->uninitialized_move(this->begin(), this->end(), NewElts); diff --git a/math/vecops/src/RVec.cxx b/math/vecops/src/RVec.cxx index 4eb022b19b76a..e7f0c4c828ab7 100644 --- a/math/vecops/src/RVec.cxx +++ b/math/vecops/src/RVec.cxx @@ -58,12 +58,14 @@ void ROOT::Internal::VecOps::SmallVectorBase::grow_pod(void *FirstEl, size_t Min void *NewElts; if (fBeginX == FirstEl || !this->Owns()) { NewElts = malloc(NewCapacity * TSize); + R__ASSERT(NewElts != nullptr); // Copy the elements over. No need to run dtors on PODs. memcpy(NewElts, this->fBeginX, size() * TSize); } else { // If this wasn't grown from the inline copy, grow the allocated space. NewElts = realloc(this->fBeginX, NewCapacity * TSize); + R__ASSERT(NewElts != nullptr); } this->fBeginX = NewElts; From 1b912f43a43ccf58ea26d1476144cf3afeefcdad Mon Sep 17 00:00:00 2001 From: Enrico Guiraud Date: Wed, 29 Sep 2021 11:56:41 +0200 Subject: [PATCH 49/51] [VecOps] Credit LLVM also in header/source files. --- math/vecops/inc/ROOT/RVec.hxx | 1 + math/vecops/src/RVec.cxx | 12 ++++++++++++ 2 files changed, 13 insertions(+) diff --git a/math/vecops/inc/ROOT/RVec.hxx b/math/vecops/inc/ROOT/RVec.hxx index 2f15a1b1a19f7..ea3b03f6e383e 100644 --- a/math/vecops/inc/ROOT/RVec.hxx +++ b/math/vecops/inc/ROOT/RVec.hxx @@ -1,4 +1,5 @@ // Author: Enrico Guiraud, Enric Tejedor, Danilo Piparo CERN 04/2021 +// Implementation adapted from from llvm::SmallVector. // See /math/vecops/ARCHITECTURE.md for more information. /************************************************************************* diff --git a/math/vecops/src/RVec.cxx b/math/vecops/src/RVec.cxx index e7f0c4c828ab7..6f54578478ecb 100644 --- a/math/vecops/src/RVec.cxx +++ b/math/vecops/src/RVec.cxx @@ -1,3 +1,15 @@ +// Author: Enrico Guiraud, Enric Tejedor, Danilo Piparo CERN 04/2021 +// Implementation adapted from from llvm::SmallVector. +// See /math/vecops/ARCHITECTURE.md for more information. + +/************************************************************************* + * Copyright (C) 1995-2021, Rene Brun and Fons Rademakers. * + * All rights reserved. * + * * + * For the licensing terms see $ROOTSYS/LICENSE. * + * For the list of contributors see $ROOTSYS/README/CREDITS. * + *************************************************************************/ + #include "ROOT/RVec.hxx" using namespace ROOT::VecOps; From 5fab3e6f96313e97bb97efc940ccd652715b7405 Mon Sep 17 00:00:00 2001 From: Enrico Guiraud Date: Wed, 29 Sep 2021 12:16:21 +0200 Subject: [PATCH 50/51] [VecOps] Rename capacity_in_bytes -> CapacityInBytes --- math/vecops/inc/ROOT/RVec.hxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/math/vecops/inc/ROOT/RVec.hxx b/math/vecops/inc/ROOT/RVec.hxx index ea3b03f6e383e..3af0304c9624e 100644 --- a/math/vecops/inc/ROOT/RVec.hxx +++ b/math/vecops/inc/ROOT/RVec.hxx @@ -1479,7 +1479,7 @@ public: }; template -inline size_t capacity_in_bytes(const RVecN &X) +inline size_t CapacityInBytes(const RVecN &X) { return X.capacity_in_bytes(); } From 48ef5254a61547526b526cdce150527ff623b12e Mon Sep 17 00:00:00 2001 From: Enrico Guiraud Date: Wed, 29 Sep 2021 12:26:17 +0200 Subject: [PATCH 51/51] [VecOps][NFC] Add note about RVec method naming convention --- math/vecops/inc/ROOT/RVec.hxx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/math/vecops/inc/ROOT/RVec.hxx b/math/vecops/inc/ROOT/RVec.hxx index 3af0304c9624e..3078639acdce2 100644 --- a/math/vecops/inc/ROOT/RVec.hxx +++ b/math/vecops/inc/ROOT/RVec.hxx @@ -1266,6 +1266,9 @@ the manipulation and analysis of the data in the RVec. \note RVec does not attempt to be exception safe. Exceptions thrown by element constructors during insertions, swaps or other operations will be propagated potentially leaving the RVec object in an invalid state. +\note RVec methods (e.g. `at` or `size`) follow the STL naming convention instead of the ROOT naming convention in order +to make RVec a drop-in replacement for `std::vector`. + \htmlonly DOI \endhtmlonly