From 419347ba4aff90ecb9209c3fd18367a2ae7d83dc Mon Sep 17 00:00:00 2001 From: Matthew Parkinson Date: Fri, 7 Jan 2022 17:09:13 +0000 Subject: [PATCH] Optimise guarded memcpy (#449) * Improve testing of memcpy including adding perf test. * Change remaining_bytes to be branch free. Use reciprocal division followed by multiply to remove a branch. --- src/mem/sizeclasstable.h | 80 +++++++-------- src/override/memcpy.cc | 35 ++++--- src/test/measuretime.h | 29 +++++- src/test/perf/memcpy/memcpy.cc | 172 +++++++++++++++++++++++++++++++++ 4 files changed, 263 insertions(+), 53 deletions(-) create mode 100644 src/test/perf/memcpy/memcpy.cc diff --git a/src/mem/sizeclasstable.h b/src/mem/sizeclasstable.h index e243fc1..7355987 100644 --- a/src/mem/sizeclasstable.h +++ b/src/mem/sizeclasstable.h @@ -148,7 +148,7 @@ namespace snmalloc // the slab. size_t slab_mask; // Table of constants for reciprocal division for each sizeclass. - size_t mod_mult; + size_t div_mult; // Table of constants for reciprocal modulus for each sizeclass. size_t mod_zero_mult; }; @@ -168,6 +168,8 @@ namespace snmalloc ModArray fast_; ModArray slow_; + size_t DIV_MULT_SHIFT{0}; + [[nodiscard]] constexpr sizeclass_data_fast& fast(sizeclass_t index) { return fast_[index.raw()]; @@ -199,8 +201,10 @@ namespace snmalloc return slow_[index.raw()]; } - constexpr SizeClassTable() : fast_(), slow_() + constexpr SizeClassTable() : fast_(), slow_(), DIV_MULT_SHIFT() { + size_t max_capacity = 0; + for (sizeclass_compress_t sizeclass = 0; sizeclass < NUM_SMALL_SIZECLASSES; sizeclass++) @@ -225,47 +229,49 @@ namespace snmalloc #else static_cast(bits::min((meta_slow.capacity / 4), 32)); #endif + + if (meta_slow.capacity > max_capacity) + { + max_capacity = meta_slow.capacity; + } } + // Get maximum precision to calculate largest division range. + DIV_MULT_SHIFT = bits::BITS - bits::next_pow2_bits_const(max_capacity); + for (sizeclass_compress_t sizeclass = 0; sizeclass < NUM_SMALL_SIZECLASSES; sizeclass++) { - // Calculate reciprocal modulus constant like reciprocal division, but - // constant is choosen to overflow and only leave the modulus as the - // result. + // Calculate reciprocal division constant. auto& meta = fast_small(sizeclass); - meta.mod_mult = bits::one_at_bit(bits::BITS - 1) / meta.size; - meta.mod_mult *= 2; - - if (bits::is_pow2(meta.size)) - { - // Set to zero, so masking path is taken if power of 2. - meta.mod_mult = 0; - } + meta.div_mult = + ((bits::one_at_bit(DIV_MULT_SHIFT) - 1) / meta.size) + 1; size_t zero = 0; meta.mod_zero_mult = (~zero / meta.size) + 1; } - // Set up table for large classes. - // Note skipping sizeclass == 0 as this is size == 0, so the tables can be - // all zero. - for (size_t sizeclass = 1; sizeclass < bits::BITS; sizeclass++) + for (size_t sizeclass = 0; sizeclass < bits::BITS; sizeclass++) { auto lsc = sizeclass_t::from_large_class(sizeclass); auto& meta = fast(lsc); - meta.size = bits::one_at_bit(lsc.as_large()); + meta.size = sizeclass == 0 ? 0 : bits::one_at_bit(lsc.as_large()); meta.slab_mask = meta.size - 1; // The slab_mask will do all the necessary work, so // perform identity multiplication for the test. meta.mod_zero_mult = 1; + // The slab_mask will do all the necessary work for division + // so collapse the calculated offset. + meta.div_mult = 0; } } }; static inline constexpr SizeClassTable sizeclass_metadata = SizeClassTable(); + static constexpr size_t DIV_MULT_SHIFT = sizeclass_metadata.DIV_MULT_SHIFT; + constexpr static inline size_t sizeclass_to_size(smallsizeclass_t sizeclass) { return sizeclass_metadata.fast_small(sizeclass).size; @@ -328,40 +334,36 @@ namespace snmalloc .capacity; } - inline static size_t mod_by_sizeclass(sizeclass_t sc, size_t offset) + inline static address_t start_of_object(sizeclass_t sc, address_t addr) { - // Only works up to certain offsets, exhaustively tested by rounding.cc auto meta = sizeclass_metadata.fast(sc); + address_t slab_start = addr & ~meta.slab_mask; + size_t offset = addr & meta.slab_mask; + size_t size = meta.size; - // Powers of two should use straigt mask. - SNMALLOC_ASSERT(meta.mod_mult != 0); - - if constexpr (sizeof(offset) >= 8) + if constexpr (sizeof(addr) >= 8) { // Only works for 64 bit multiplication, as the following will overflow in // 32bit. - // Could be made nicer with 128bit multiply (umulh): + // Based on // https://lemire.me/blog/2019/02/20/more-fun-with-fast-remainders-when-the-divisor-is-a-constant/ - auto bits_l = bits::BITS / 2; - auto bits_h = bits::BITS - bits_l; - return ( - ((((offset + 1) * meta.mod_mult) >> (bits_l)) * meta.size) >> bits_h); + // We are using an adaptation of the "indirect" method. By using the + // indirect method we can handle the large power of two classes just with + // the slab_mask by making the `div_mult` zero. The link uses 128 bit + // multiplication, we have shrunk the range of the calculation to remove + // this dependency. + size_t offset_start = ((offset * meta.div_mult) >> DIV_MULT_SHIFT) * size; + return slab_start + offset_start; } else - // Use 32-bit division as considerably faster than 64-bit, and - // everything fits into 32bits here. - return static_cast(offset % meta.size); + { + return slab_start + (offset / size) * size; + } } inline static size_t index_in_object(sizeclass_t sc, address_t addr) { - if (sizeclass_metadata.fast(sc).mod_mult == 0) - { - return addr & (sizeclass_metadata.fast(sc).size - 1); - } - - address_t offset = addr & (sizeclass_full_to_slab_size(sc) - 1); - return mod_by_sizeclass(sc, offset); + return addr - start_of_object(sc, addr); } inline static size_t remaining_bytes(sizeclass_t sc, address_t addr) diff --git a/src/override/memcpy.cc b/src/override/memcpy.cc index 6c858f4..884e1c2 100644 --- a/src/override/memcpy.cc +++ b/src/override/memcpy.cc @@ -141,7 +141,7 @@ namespace { if constexpr (FailFast) { - UNUSED(ptr, len, msg); + UNUSED(p, len, msg); SNMALLOC_FAST_FAIL(); } else @@ -193,15 +193,12 @@ namespace return (pointer_align_down(const_cast(src)) == src) && (pointer_align_down(dst) == dst); } -} -extern "C" -{ /** * Snmalloc checked memcpy. */ - SNMALLOC_EXPORT void* - SNMALLOC_NAME_MANGLE(memcpy)(void* dst, const void* src, size_t len) + template + void* memcpy(void* dst, const void* src, size_t len) { // 0 is a very common size for memcpy and we don't need to do external // pointer checks if we hit it. It's also the fastest case, to encourage @@ -210,11 +207,15 @@ extern "C" { return dst; } - // Check the bounds of the arguments. - check_bounds( - dst, len, "memcpy with destination out of bounds of heap allocation"); - check_bounds( - src, len, "memcpy with source out of bounds of heap allocation"); + + if constexpr (checked) + { + // Check the bounds of the arguments. + check_bounds( + dst, len, "memcpy with destination out of bounds of heap allocation"); + check_bounds( + src, len, "memcpy with source out of bounds of heap allocation"); + } // If this is a small size, do byte-by-byte copies. if (len < LargestRegisterSize) { @@ -225,4 +226,16 @@ extern "C" copy_end(dst, src, len); return dst; } +} // namespace + +extern "C" +{ + /** + * Snmalloc checked memcpy. + */ + SNMALLOC_EXPORT void* + SNMALLOC_NAME_MANGLE(memcpy)(void* dst, const void* src, size_t len) + { + return memcpy(dst, src, len); + } } diff --git a/src/test/measuretime.h b/src/test/measuretime.h index a67a65e..903961c 100644 --- a/src/test/measuretime.h +++ b/src/test/measuretime.h @@ -5,17 +5,40 @@ #include #include -class MeasureTime : public std::stringstream +class MeasureTime { + std::stringstream ss; std::chrono::time_point start = std::chrono::high_resolution_clock::now(); + bool quiet = false; + public: ~MeasureTime() { auto finish = std::chrono::high_resolution_clock::now(); auto diff = finish - start; - std::cout << str() << ": " << std::setw(12) << diff.count() << " ns" - << std::endl; + if (!quiet) + { + std::cout << ss.str() << ": " << std::setw(12) << diff.count() << " ns" + << std::endl; + } + } + + MeasureTime(bool quiet = false) : quiet(quiet) {} + + template + MeasureTime& operator<<(const T& s) + { + ss << s; + start = std::chrono::high_resolution_clock::now(); + return *this; + } + + std::chrono::nanoseconds get_time() + { + auto finish = std::chrono::high_resolution_clock::now(); + auto diff = finish - start; + return diff; } }; \ No newline at end of file diff --git a/src/test/perf/memcpy/memcpy.cc b/src/test/perf/memcpy/memcpy.cc new file mode 100644 index 0000000..9bfb985 --- /dev/null +++ b/src/test/perf/memcpy/memcpy.cc @@ -0,0 +1,172 @@ +#include +#include + +#define SNMALLOC_NAME_MANGLE(a) our_##a +#include "override/memcpy.cc" + +#include + +struct Shape +{ + void* object; + void* dst; +}; + +size_t my_random() +{ + return (size_t)rand(); +} + +std::vector allocs; + +void shape(size_t size) +{ + for (size_t i = 0; i < 1000; i++) + { + auto rsize = size * 2; + auto offset = 0; + // Uncomment the next two lines to introduce some randomness to the start of + // the memcpys. constexpr size_t alignment = 16; offset = (my_random() % + // size / alignment) * alignment; + Shape s; + s.object = ThreadAlloc::get().alloc(rsize); + s.dst = reinterpret_cast(s.object) + offset; + // Bring into cache the destination of the copy. + memset(s.dst, 0xFF, size); + allocs.push_back(s); + } +} + +void unshape() +{ + for (auto& s : allocs) + { + ThreadAlloc::get().dealloc(s.object); + } + allocs.clear(); +} + +template +void test_memcpy(size_t size, void* src, Memcpy mc) +{ + for (auto& s : allocs) + { + auto* dst = reinterpret_cast(s.dst); + mc(dst, src, size); + } +} + +template +void test( + size_t size, + Memcpy mc, + std::vector>& stats) +{ + auto src = ThreadAlloc::get().alloc(size); + shape(size); + for (size_t i = 0; i < 10; i++) + { + MeasureTime m(true); + test_memcpy(size, src, mc); + auto time = m.get_time(); + stats.push_back({size, time}); + } + ThreadAlloc::get().dealloc(src); + unshape(); +} + +NOINLINE +void memcpy_checked(void* dst, const void* src, size_t size) +{ + memcpy(dst, src, size); +} + +NOINLINE +void memcpy_unchecked(void* dst, const void* src, size_t size) +{ + memcpy(dst, src, size); +} + +NOINLINE +void memcpy_platform_checked(void* dst, const void* src, size_t size) +{ + check_bounds(dst, size, ""); + memcpy(dst, src, size); +} + +int main(int argc, char** argv) +{ + opt::Opt opt(argc, argv); +#ifndef SNMALLOC_PASS_THROUGH + bool full_test = opt.has("--full_test"); + + // size_t size = 0; + auto mc1 = [](void* dst, const void* src, size_t len) { + memcpy_platform_checked(dst, src, len); + }; + auto mc2 = [](void* dst, const void* src, size_t len) { + memcpy_unchecked(dst, src, len); + }; + auto mc3 = [](void* dst, const void* src, size_t len) { + memcpy(dst, src, len); + }; + + std::vector sizes; + for (size_t size = 1; size < 64; size++) + { + sizes.push_back(size); + } + for (size_t size = 64; size < 256; size += 16) + { + sizes.push_back(size); + sizes.push_back(size + 5); + } + for (size_t size = 256; size < 1024; size += 64) + { + sizes.push_back(size); + sizes.push_back(size + 5); + } + for (size_t size = 1024; size < 8192; size += 256) + { + sizes.push_back(size); + sizes.push_back(size + 5); + } + for (size_t size = 8192; size < bits::one_at_bit(18); size <<= 1) + { + sizes.push_back(size); + sizes.push_back(size + 5); + } + + std::vector> stats_checked; + std::vector> stats_unchecked; + std::vector> stats_platform; + + printf("size, checked, unchecked, platform\n"); + + size_t repeats = full_test ? 80 : 1; + + for (auto repeat = repeats; 0 < repeat; repeat--) + { + for (auto copy_size : sizes) + { + test(copy_size, mc1, stats_checked); + test(copy_size, mc2, stats_unchecked); + test(copy_size, mc3, stats_platform); + } + for (size_t i = 0; i < stats_checked.size(); i++) + { + auto& s1 = stats_checked[i]; + auto& s2 = stats_unchecked[i]; + auto& s3 = stats_platform[i]; + std::cout << s1.first << ", " << s1.second.count() << ", " + << s2.second.count() << ", " << s3.second.count() << std::endl; + } + stats_checked.clear(); + stats_unchecked.clear(); + stats_platform.clear(); + } +#else + snmalloc::UNUSED(opt); +#endif + return 0; +}