From b7c6ef0225275013bddbfafadde4879229a5ef58 Mon Sep 17 00:00:00 2001 From: Alexander Polcyn Date: Mon, 6 May 2019 15:23:41 -0700 Subject: [PATCH 1/4] Revert "Use platform align_malloc function when setting custom allocators and no override provided" This reverts commit 22c6e166c439481d5d5fa0990b8c59db92ec5152. --- src/core/lib/gpr/alloc.cc | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/src/core/lib/gpr/alloc.cc b/src/core/lib/gpr/alloc.cc index f79e645971a..b12b7d8534d 100644 --- a/src/core/lib/gpr/alloc.cc +++ b/src/core/lib/gpr/alloc.cc @@ -43,9 +43,21 @@ static constexpr bool is_power_of_two(size_t value) { } #endif +static void* aligned_alloc_with_gpr_malloc(size_t size, size_t alignment) { + GPR_DEBUG_ASSERT(is_power_of_two(alignment)); + size_t extra = alignment - 1 + sizeof(void*); + void* p = gpr_malloc(size + extra); + void** ret = (void**)(((uintptr_t)p + extra) & ~(alignment - 1)); + ret[-1] = p; + return (void*)ret; +} + +static void aligned_free_with_gpr_malloc(void* ptr) { + gpr_free((static_cast(ptr))[-1]); +} + static void* platform_malloc_aligned(size_t size, size_t alignment) { #if defined(GPR_HAS_ALIGNED_ALLOC) - GPR_DEBUG_ASSERT(is_power_of_two(alignment)); size = GPR_ROUND_UP_TO_ALIGNMENT_SIZE(size, alignment); void* ret = aligned_alloc(alignment, size); GPR_ASSERT(ret != nullptr); @@ -62,12 +74,7 @@ static void* platform_malloc_aligned(size_t size, size_t alignment) { GPR_ASSERT(posix_memalign(&ret, alignment, size) == 0); return ret; #else - GPR_DEBUG_ASSERT(is_power_of_two(alignment)); - size_t extra = alignment - 1 + sizeof(void*); - void* p = gpr_malloc(size + extra); - void** ret = (void**)(((uintptr_t)p + extra) & ~(alignment - 1)); - ret[-1] = p; - return (void*)ret; + return aligned_alloc_with_gpr_malloc(size, alignment); #endif } @@ -77,7 +84,7 @@ static void platform_free_aligned(void* ptr) { #elif defined(GPR_HAS_ALIGNED_MALLOC) _aligned_free(ptr); #else - gpr_free((static_cast(ptr))[-1]); + aligned_free_with_gpr_malloc(ptr); #endif } @@ -99,8 +106,8 @@ void gpr_set_allocation_functions(gpr_allocation_functions functions) { GPR_ASSERT((functions.aligned_alloc_fn == nullptr) == (functions.aligned_free_fn == nullptr)); if (functions.aligned_alloc_fn == nullptr) { - functions.aligned_alloc_fn = platform_malloc_aligned; - functions.aligned_free_fn = platform_free_aligned; + functions.aligned_alloc_fn = aligned_alloc_with_gpr_malloc; + functions.aligned_free_fn = aligned_free_with_gpr_malloc; } g_alloc_functions = functions; } From 0562b51f8e4f7bfd76b12555cc62de6a53df6205 Mon Sep 17 00:00:00 2001 From: Alexander Polcyn Date: Mon, 6 May 2019 15:23:54 -0700 Subject: [PATCH 2/4] Revert "Fixed non-debug build warning" This reverts commit e1a96b83471e229909890ebd9f9b7cf3d8d1edec. --- src/core/lib/gpr/alloc.cc | 2 -- src/core/lib/gprpp/arena.cc | 4 ++-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/core/lib/gpr/alloc.cc b/src/core/lib/gpr/alloc.cc index b12b7d8534d..0d9f6f7120e 100644 --- a/src/core/lib/gpr/alloc.cc +++ b/src/core/lib/gpr/alloc.cc @@ -34,14 +34,12 @@ static void* zalloc_with_gpr_malloc(size_t sz) { return p; } -#ifndef NDEBUG static constexpr bool is_power_of_two(size_t value) { // 2^N = 100000...000 // 2^N - 1 = 011111...111 // (2^N) && ((2^N)-1)) = 0 return (value & (value - 1)) == 0; } -#endif static void* aligned_alloc_with_gpr_malloc(size_t size, size_t alignment) { GPR_DEBUG_ASSERT(is_power_of_two(alignment)); diff --git a/src/core/lib/gprpp/arena.cc b/src/core/lib/gprpp/arena.cc index 2623ae870a1..e1c7b292f8c 100644 --- a/src/core/lib/gprpp/arena.cc +++ b/src/core/lib/gprpp/arena.cc @@ -35,8 +35,8 @@ namespace { void* ArenaStorage(size_t initial_size) { static constexpr size_t base_size = - GPR_ROUND_UP_TO_MAX_ALIGNMENT_SIZE(sizeof(grpc_core::Arena)); - initial_size = GPR_ROUND_UP_TO_MAX_ALIGNMENT_SIZE(initial_size); + GPR_ROUND_UP_TO_ALIGNMENT_SIZE(sizeof(grpc_core::Arena)); + initial_size = GPR_ROUND_UP_TO_ALIGNMENT_SIZE(initial_size); size_t alloc_size = base_size + initial_size; static constexpr size_t alignment = (GPR_CACHELINE_SIZE > GPR_MAX_ALIGNMENT && From cb966a4e5e09af1f5adabbee3e4c03f66bc5d9ae Mon Sep 17 00:00:00 2001 From: Alexander Polcyn Date: Mon, 6 May 2019 15:24:09 -0700 Subject: [PATCH 3/4] Revert "Renamed macros for memory alignment" This reverts commit a3fe7c0c908451deb04c3bb99a6b364b313467f6. --- .../ext/filters/client_channel/subchannel.cc | 22 +++++----- src/core/lib/channel/channel_stack.cc | 43 +++++++++---------- src/core/lib/gpr/alloc.cc | 2 +- src/core/lib/gpr/alloc.h | 14 +++--- src/core/lib/gprpp/arena.cc | 4 +- src/core/lib/gprpp/arena.h | 4 +- src/core/lib/surface/call.cc | 6 +-- src/core/lib/transport/transport.cc | 2 +- test/core/util/memory_counters.cc | 20 ++++----- 9 files changed, 56 insertions(+), 61 deletions(-) diff --git a/src/core/ext/filters/client_channel/subchannel.cc b/src/core/ext/filters/client_channel/subchannel.cc index 873db8ef57c..a284e692b09 100644 --- a/src/core/ext/filters/client_channel/subchannel.cc +++ b/src/core/ext/filters/client_channel/subchannel.cc @@ -66,13 +66,12 @@ #define GRPC_SUBCHANNEL_RECONNECT_JITTER 0.2 // Conversion between subchannel call and call stack. -#define SUBCHANNEL_CALL_TO_CALL_STACK(call) \ - (grpc_call_stack*)((char*)(call) + GPR_ROUND_UP_TO_MAX_ALIGNMENT_SIZE( \ - sizeof(SubchannelCall))) -#define CALL_STACK_TO_SUBCHANNEL_CALL(callstack) \ - (SubchannelCall*)(((char*)(call_stack)) - \ - GPR_ROUND_UP_TO_MAX_ALIGNMENT_SIZE( \ - sizeof(SubchannelCall))) +#define SUBCHANNEL_CALL_TO_CALL_STACK(call) \ + (grpc_call_stack*)((char*)(call) + \ + GPR_ROUND_UP_TO_ALIGNMENT_SIZE(sizeof(SubchannelCall))) +#define CALL_STACK_TO_SUBCHANNEL_CALL(callstack) \ + (SubchannelCall*)(((char*)(call_stack)) - \ + GPR_ROUND_UP_TO_ALIGNMENT_SIZE(sizeof(SubchannelCall))) namespace grpc_core { @@ -152,10 +151,10 @@ RefCountedPtr ConnectedSubchannel::CreateCall( size_t ConnectedSubchannel::GetInitialCallSizeEstimate( size_t parent_data_size) const { size_t allocation_size = - GPR_ROUND_UP_TO_MAX_ALIGNMENT_SIZE(sizeof(SubchannelCall)); + GPR_ROUND_UP_TO_ALIGNMENT_SIZE(sizeof(SubchannelCall)); if (parent_data_size > 0) { allocation_size += - GPR_ROUND_UP_TO_MAX_ALIGNMENT_SIZE(channel_stack_->call_stack_size) + + GPR_ROUND_UP_TO_ALIGNMENT_SIZE(channel_stack_->call_stack_size) + parent_data_size; } else { allocation_size += channel_stack_->call_stack_size; @@ -179,9 +178,8 @@ void SubchannelCall::StartTransportStreamOpBatch( void* SubchannelCall::GetParentData() { grpc_channel_stack* chanstk = connected_subchannel_->channel_stack(); - return (char*)this + - GPR_ROUND_UP_TO_MAX_ALIGNMENT_SIZE(sizeof(SubchannelCall)) + - GPR_ROUND_UP_TO_MAX_ALIGNMENT_SIZE(chanstk->call_stack_size); + return (char*)this + GPR_ROUND_UP_TO_ALIGNMENT_SIZE(sizeof(SubchannelCall)) + + GPR_ROUND_UP_TO_ALIGNMENT_SIZE(chanstk->call_stack_size); } grpc_call_stack* SubchannelCall::GetCallStack() { diff --git a/src/core/lib/channel/channel_stack.cc b/src/core/lib/channel/channel_stack.cc index 7dfabbb0a1c..df956c7176c 100644 --- a/src/core/lib/channel/channel_stack.cc +++ b/src/core/lib/channel/channel_stack.cc @@ -47,9 +47,9 @@ grpc_core::TraceFlag grpc_trace_channel(false, "channel"); size_t grpc_channel_stack_size(const grpc_channel_filter** filters, size_t filter_count) { /* always need the header, and size for the channel elements */ - size_t size = GPR_ROUND_UP_TO_MAX_ALIGNMENT_SIZE(sizeof(grpc_channel_stack)) + - GPR_ROUND_UP_TO_MAX_ALIGNMENT_SIZE( - filter_count * sizeof(grpc_channel_element)); + size_t size = GPR_ROUND_UP_TO_ALIGNMENT_SIZE(sizeof(grpc_channel_stack)) + + GPR_ROUND_UP_TO_ALIGNMENT_SIZE(filter_count * + sizeof(grpc_channel_element)); size_t i; GPR_ASSERT((GPR_MAX_ALIGNMENT & (GPR_MAX_ALIGNMENT - 1)) == 0 && @@ -57,18 +57,18 @@ size_t grpc_channel_stack_size(const grpc_channel_filter** filters, /* add the size for each filter */ for (i = 0; i < filter_count; i++) { - size += GPR_ROUND_UP_TO_MAX_ALIGNMENT_SIZE(filters[i]->sizeof_channel_data); + size += GPR_ROUND_UP_TO_ALIGNMENT_SIZE(filters[i]->sizeof_channel_data); } return size; } -#define CHANNEL_ELEMS_FROM_STACK(stk) \ - ((grpc_channel_element*)((char*)(stk) + GPR_ROUND_UP_TO_MAX_ALIGNMENT_SIZE( \ +#define CHANNEL_ELEMS_FROM_STACK(stk) \ + ((grpc_channel_element*)((char*)(stk) + GPR_ROUND_UP_TO_ALIGNMENT_SIZE( \ sizeof(grpc_channel_stack)))) -#define CALL_ELEMS_FROM_STACK(stk) \ - ((grpc_call_element*)((char*)(stk) + GPR_ROUND_UP_TO_MAX_ALIGNMENT_SIZE( \ +#define CALL_ELEMS_FROM_STACK(stk) \ + ((grpc_call_element*)((char*)(stk) + GPR_ROUND_UP_TO_ALIGNMENT_SIZE( \ sizeof(grpc_call_stack)))) grpc_channel_element* grpc_channel_stack_element( @@ -92,9 +92,8 @@ grpc_error* grpc_channel_stack_init( const grpc_channel_args* channel_args, grpc_transport* optional_transport, const char* name, grpc_channel_stack* stack) { size_t call_size = - GPR_ROUND_UP_TO_MAX_ALIGNMENT_SIZE(sizeof(grpc_call_stack)) + - GPR_ROUND_UP_TO_MAX_ALIGNMENT_SIZE(filter_count * - sizeof(grpc_call_element)); + GPR_ROUND_UP_TO_ALIGNMENT_SIZE(sizeof(grpc_call_stack)) + + GPR_ROUND_UP_TO_ALIGNMENT_SIZE(filter_count * sizeof(grpc_call_element)); grpc_channel_element* elems; grpc_channel_element_args args; char* user_data; @@ -105,8 +104,8 @@ grpc_error* grpc_channel_stack_init( name); elems = CHANNEL_ELEMS_FROM_STACK(stack); user_data = (reinterpret_cast(elems)) + - GPR_ROUND_UP_TO_MAX_ALIGNMENT_SIZE(filter_count * - sizeof(grpc_channel_element)); + GPR_ROUND_UP_TO_ALIGNMENT_SIZE(filter_count * + sizeof(grpc_channel_element)); /* init per-filter data */ grpc_error* first_error = GRPC_ERROR_NONE; @@ -127,9 +126,8 @@ grpc_error* grpc_channel_stack_init( } } user_data += - GPR_ROUND_UP_TO_MAX_ALIGNMENT_SIZE(filters[i]->sizeof_channel_data); - call_size += - GPR_ROUND_UP_TO_MAX_ALIGNMENT_SIZE(filters[i]->sizeof_call_data); + GPR_ROUND_UP_TO_ALIGNMENT_SIZE(filters[i]->sizeof_channel_data); + call_size += GPR_ROUND_UP_TO_ALIGNMENT_SIZE(filters[i]->sizeof_call_data); } GPR_ASSERT(user_data > (char*)stack); @@ -164,9 +162,8 @@ grpc_error* grpc_call_stack_init(grpc_channel_stack* channel_stack, GRPC_STREAM_REF_INIT(&elem_args->call_stack->refcount, initial_refs, destroy, destroy_arg, "CALL_STACK"); call_elems = CALL_ELEMS_FROM_STACK(elem_args->call_stack); - user_data = - (reinterpret_cast(call_elems)) + - GPR_ROUND_UP_TO_MAX_ALIGNMENT_SIZE(count * sizeof(grpc_call_element)); + user_data = (reinterpret_cast(call_elems)) + + GPR_ROUND_UP_TO_ALIGNMENT_SIZE(count * sizeof(grpc_call_element)); /* init per-filter data */ grpc_error* first_error = GRPC_ERROR_NONE; @@ -174,8 +171,8 @@ grpc_error* grpc_call_stack_init(grpc_channel_stack* channel_stack, call_elems[i].filter = channel_elems[i].filter; call_elems[i].channel_data = channel_elems[i].channel_data; call_elems[i].call_data = user_data; - user_data += GPR_ROUND_UP_TO_MAX_ALIGNMENT_SIZE( - call_elems[i].filter->sizeof_call_data); + user_data += + GPR_ROUND_UP_TO_ALIGNMENT_SIZE(call_elems[i].filter->sizeof_call_data); } for (size_t i = 0; i < count; i++) { grpc_error* error = @@ -245,11 +242,11 @@ grpc_channel_stack* grpc_channel_stack_from_top_element( grpc_channel_element* elem) { return reinterpret_cast( reinterpret_cast(elem) - - GPR_ROUND_UP_TO_MAX_ALIGNMENT_SIZE(sizeof(grpc_channel_stack))); + GPR_ROUND_UP_TO_ALIGNMENT_SIZE(sizeof(grpc_channel_stack))); } grpc_call_stack* grpc_call_stack_from_top_element(grpc_call_element* elem) { return reinterpret_cast( reinterpret_cast(elem) - - GPR_ROUND_UP_TO_MAX_ALIGNMENT_SIZE(sizeof(grpc_call_stack))); + GPR_ROUND_UP_TO_ALIGNMENT_SIZE(sizeof(grpc_call_stack))); } diff --git a/src/core/lib/gpr/alloc.cc b/src/core/lib/gpr/alloc.cc index 0d9f6f7120e..b601ad79f79 100644 --- a/src/core/lib/gpr/alloc.cc +++ b/src/core/lib/gpr/alloc.cc @@ -56,7 +56,7 @@ static void aligned_free_with_gpr_malloc(void* ptr) { static void* platform_malloc_aligned(size_t size, size_t alignment) { #if defined(GPR_HAS_ALIGNED_ALLOC) - size = GPR_ROUND_UP_TO_ALIGNMENT_SIZE(size, alignment); + size = GPR_ROUND_UP_TO_SPECIFIED_SIZE(size, alignment); void* ret = aligned_alloc(alignment, size); GPR_ASSERT(ret != nullptr); return ret; diff --git a/src/core/lib/gpr/alloc.h b/src/core/lib/gpr/alloc.h index c77fbeaca49..2493c87514d 100644 --- a/src/core/lib/gpr/alloc.h +++ b/src/core/lib/gpr/alloc.h @@ -22,13 +22,15 @@ #include /// Given a size, round up to the next multiple of sizeof(void*). -#define GPR_ROUND_UP_TO_ALIGNMENT_SIZE(x, align) \ - (((x) + (align)-1u) & ~((align)-1u)) - -#define GPR_ROUND_UP_TO_MAX_ALIGNMENT_SIZE(x) \ - GPR_ROUND_UP_TO_ALIGNMENT_SIZE((x), GPR_MAX_ALIGNMENT) +#define GPR_ROUND_UP_TO_ALIGNMENT_SIZE(x) \ + (((x) + GPR_MAX_ALIGNMENT - 1u) & ~(GPR_MAX_ALIGNMENT - 1u)) #define GPR_ROUND_UP_TO_CACHELINE_SIZE(x) \ - GPR_ROUND_UP_TO_ALIGNMENT_SIZE((x), GPR_CACHELINE_SIZE) + (((x) + GPR_CACHELINE_SIZE - 1u) & ~(GPR_CACHELINE_SIZE - 1u)) + +#define GPR_ROUND_UP_TO_SPECIFIED_SIZE(x, align) \ + (((x) + align - 1u) & ~(align - 1u)) + +void* gpr_malloc_cacheline(size_t size); #endif /* GRPC_CORE_LIB_GPR_ALLOC_H */ diff --git a/src/core/lib/gprpp/arena.cc b/src/core/lib/gprpp/arena.cc index e1c7b292f8c..5c344db4e35 100644 --- a/src/core/lib/gprpp/arena.cc +++ b/src/core/lib/gprpp/arena.cc @@ -67,7 +67,7 @@ Arena* Arena::Create(size_t initial_size) { Pair Arena::CreateWithAlloc(size_t initial_size, size_t alloc_size) { static constexpr size_t base_size = - GPR_ROUND_UP_TO_MAX_ALIGNMENT_SIZE(sizeof(Arena)); + GPR_ROUND_UP_TO_ALIGNMENT_SIZE(sizeof(Arena)); auto* new_arena = new (ArenaStorage(initial_size)) Arena(initial_size, alloc_size); void* first_alloc = reinterpret_cast(new_arena) + base_size; @@ -88,7 +88,7 @@ void* Arena::AllocZone(size_t size) { // sizing hysteresis (that is, most calls should have a large enough initial // zone and will not need to grow the arena). static constexpr size_t zone_base_size = - GPR_ROUND_UP_TO_MAX_ALIGNMENT_SIZE(sizeof(Zone)); + GPR_ROUND_UP_TO_ALIGNMENT_SIZE(sizeof(Zone)); size_t alloc_size = zone_base_size + size; Zone* z = new (gpr_malloc_aligned(alloc_size, GPR_MAX_ALIGNMENT)) Zone(); { diff --git a/src/core/lib/gprpp/arena.h b/src/core/lib/gprpp/arena.h index 6c646c5871d..915cd5c528e 100644 --- a/src/core/lib/gprpp/arena.h +++ b/src/core/lib/gprpp/arena.h @@ -58,8 +58,8 @@ class Arena { // Allocate \a size bytes from the arena. void* Alloc(size_t size) { static constexpr size_t base_size = - GPR_ROUND_UP_TO_MAX_ALIGNMENT_SIZE(sizeof(Arena)); - size = GPR_ROUND_UP_TO_MAX_ALIGNMENT_SIZE(size); + GPR_ROUND_UP_TO_ALIGNMENT_SIZE(sizeof(Arena)); + size = GPR_ROUND_UP_TO_ALIGNMENT_SIZE(size); size_t begin = total_used_.FetchAdd(size, MemoryOrder::RELAXED); if (GPR_LIKELY(begin + size <= initial_zone_size_)) { return reinterpret_cast(this) + base_size + begin; diff --git a/src/core/lib/surface/call.cc b/src/core/lib/surface/call.cc index 0a26872a9ea..bd140021c96 100644 --- a/src/core/lib/surface/call.cc +++ b/src/core/lib/surface/call.cc @@ -260,10 +260,10 @@ grpc_core::TraceFlag grpc_compression_trace(false, "compression"); #define CALL_STACK_FROM_CALL(call) \ (grpc_call_stack*)((char*)(call) + \ - GPR_ROUND_UP_TO_MAX_ALIGNMENT_SIZE(sizeof(grpc_call))) + GPR_ROUND_UP_TO_ALIGNMENT_SIZE(sizeof(grpc_call))) #define CALL_FROM_CALL_STACK(call_stack) \ (grpc_call*)(((char*)(call_stack)) - \ - GPR_ROUND_UP_TO_MAX_ALIGNMENT_SIZE(sizeof(grpc_call))) + GPR_ROUND_UP_TO_ALIGNMENT_SIZE(sizeof(grpc_call))) #define CALL_ELEM_FROM_CALL(call, idx) \ grpc_call_stack_element(CALL_STACK_FROM_CALL(call), idx) @@ -329,7 +329,7 @@ grpc_error* grpc_call_create(const grpc_call_create_args* args, size_t initial_size = grpc_channel_get_call_size_estimate(args->channel); GRPC_STATS_INC_CALL_INITIAL_SIZE(initial_size); size_t call_and_stack_size = - GPR_ROUND_UP_TO_MAX_ALIGNMENT_SIZE(sizeof(grpc_call)) + + GPR_ROUND_UP_TO_ALIGNMENT_SIZE(sizeof(grpc_call)) + channel_stack->call_stack_size; size_t call_alloc_size = call_and_stack_size + (args->parent ? sizeof(child_call) : 0); diff --git a/src/core/lib/transport/transport.cc b/src/core/lib/transport/transport.cc index 40870657b84..29c1e561891 100644 --- a/src/core/lib/transport/transport.cc +++ b/src/core/lib/transport/transport.cc @@ -115,7 +115,7 @@ void grpc_transport_move_stats(grpc_transport_stream_stats* from, } size_t grpc_transport_stream_size(grpc_transport* transport) { - return GPR_ROUND_UP_TO_MAX_ALIGNMENT_SIZE(transport->vtable->sizeof_stream); + return GPR_ROUND_UP_TO_ALIGNMENT_SIZE(transport->vtable->sizeof_stream); } void grpc_transport_destroy(grpc_transport* transport) { diff --git a/test/core/util/memory_counters.cc b/test/core/util/memory_counters.cc index 60d22b18309..11107f670fb 100644 --- a/test/core/util/memory_counters.cc +++ b/test/core/util/memory_counters.cc @@ -54,10 +54,9 @@ static void* guard_malloc(size_t size) { NO_BARRIER_FETCH_ADD(&g_memory_counters.total_allocs_absolute, (gpr_atm)1); NO_BARRIER_FETCH_ADD(&g_memory_counters.total_allocs_relative, (gpr_atm)1); void* ptr = g_old_allocs.malloc_fn( - GPR_ROUND_UP_TO_MAX_ALIGNMENT_SIZE(sizeof(size)) + size); + GPR_ROUND_UP_TO_ALIGNMENT_SIZE(sizeof(size)) + size); *static_cast(ptr) = size; - return static_cast(ptr) + - GPR_ROUND_UP_TO_MAX_ALIGNMENT_SIZE(sizeof(size)); + return static_cast(ptr) + GPR_ROUND_UP_TO_ALIGNMENT_SIZE(sizeof(size)); } static void* guard_realloc(void* vptr, size_t size) { @@ -68,24 +67,23 @@ static void* guard_realloc(void* vptr, size_t size) { guard_free(vptr); return nullptr; } - void* ptr = static_cast(vptr) - - GPR_ROUND_UP_TO_MAX_ALIGNMENT_SIZE(sizeof(size)); + void* ptr = + static_cast(vptr) - GPR_ROUND_UP_TO_ALIGNMENT_SIZE(sizeof(size)); NO_BARRIER_FETCH_ADD(&g_memory_counters.total_size_absolute, (gpr_atm)size); NO_BARRIER_FETCH_ADD(&g_memory_counters.total_size_relative, -*static_cast(ptr)); NO_BARRIER_FETCH_ADD(&g_memory_counters.total_size_relative, (gpr_atm)size); NO_BARRIER_FETCH_ADD(&g_memory_counters.total_allocs_absolute, (gpr_atm)1); ptr = g_old_allocs.realloc_fn( - ptr, GPR_ROUND_UP_TO_MAX_ALIGNMENT_SIZE(sizeof(size)) + size); + ptr, GPR_ROUND_UP_TO_ALIGNMENT_SIZE(sizeof(size)) + size); *static_cast(ptr) = size; - return static_cast(ptr) + - GPR_ROUND_UP_TO_MAX_ALIGNMENT_SIZE(sizeof(size)); + return static_cast(ptr) + GPR_ROUND_UP_TO_ALIGNMENT_SIZE(sizeof(size)); } static void guard_free(void* vptr) { if (vptr == nullptr) return; - void* ptr = static_cast(vptr) - - GPR_ROUND_UP_TO_MAX_ALIGNMENT_SIZE(sizeof(size_t)); + void* ptr = + static_cast(vptr) - GPR_ROUND_UP_TO_ALIGNMENT_SIZE(sizeof(size_t)); NO_BARRIER_FETCH_ADD(&g_memory_counters.total_size_relative, -*static_cast(ptr)); NO_BARRIER_FETCH_ADD(&g_memory_counters.total_allocs_relative, -(gpr_atm)1); @@ -95,7 +93,7 @@ static void guard_free(void* vptr) { // NB: We do not specify guard_malloc_aligned/guard_free_aligned methods. Since // they are null, calls to gpr_malloc_aligned/gpr_free_aligned are executed as a // wrapper over gpr_malloc/gpr_free, which do use guard_malloc/guard_free, and -// thus their allocations are tracked as well. +// thus there allocations are tracked as well. struct gpr_allocation_functions g_guard_allocs = { guard_malloc, nullptr, guard_realloc, guard_free, nullptr, nullptr}; From d5fb6da369719c2892eadfd92831bac7bc78118f Mon Sep 17 00:00:00 2001 From: Alexander Polcyn Date: Mon, 6 May 2019 15:24:21 -0700 Subject: [PATCH 4/4] Revert "Use aligned_alloc directly for grpc_core::Arena" This reverts commit 333ba8feaea465e35678c356542915140605b444. --- include/grpc/impl/codegen/port_platform.h | 3 - include/grpc/support/alloc.h | 2 - src/core/lib/gpr/alloc.cc | 78 +++-------------------- src/core/lib/gpr/alloc.h | 8 --- src/core/lib/gprpp/arena.h | 2 +- test/core/gpr/alloc_test.cc | 18 +----- test/core/util/memory_counters.cc | 8 +-- 7 files changed, 14 insertions(+), 105 deletions(-) diff --git a/include/grpc/impl/codegen/port_platform.h b/include/grpc/impl/codegen/port_platform.h index ff759d2baed..d7294d59d41 100644 --- a/include/grpc/impl/codegen/port_platform.h +++ b/include/grpc/impl/codegen/port_platform.h @@ -77,7 +77,6 @@ #define GPR_WINDOWS 1 #define GPR_WINDOWS_SUBPROCESS 1 #define GPR_WINDOWS_ENV -#define GPR_HAS_ALIGNED_MALLOC 1 #ifdef __MSYS__ #define GPR_GETPID_IN_UNISTD_H 1 #define GPR_MSYS_TMPFILE @@ -174,7 +173,6 @@ #define GPR_POSIX_SYNC 1 #define GPR_POSIX_TIME 1 #define GPR_HAS_PTHREAD_H 1 -#define GPR_HAS_ALIGNED_ALLOC 1 #define GPR_GETPID_IN_UNISTD_H 1 #ifdef _LP64 #define GPR_ARCH_64 1 @@ -240,7 +238,6 @@ #define GPR_POSIX_SUBPROCESS 1 #define GPR_POSIX_SYNC 1 #define GPR_POSIX_TIME 1 -#define GPR_HAS_POSIX_MEMALIGN 1 #define GPR_HAS_PTHREAD_H 1 #define GPR_GETPID_IN_UNISTD_H 1 #ifndef GRPC_CFSTREAM diff --git a/include/grpc/support/alloc.h b/include/grpc/support/alloc.h index 4d8e007a577..8bd940bec47 100644 --- a/include/grpc/support/alloc.h +++ b/include/grpc/support/alloc.h @@ -32,8 +32,6 @@ typedef struct gpr_allocation_functions { void* (*zalloc_fn)(size_t size); /** if NULL, uses malloc_fn then memset */ void* (*realloc_fn)(void* ptr, size_t size); void (*free_fn)(void* ptr); - void* (*aligned_alloc_fn)(size_t size, size_t alignment); - void (*aligned_free_fn)(void* ptr); } gpr_allocation_functions; /** malloc. diff --git a/src/core/lib/gpr/alloc.cc b/src/core/lib/gpr/alloc.cc index b601ad79f79..611e4cceee4 100644 --- a/src/core/lib/gpr/alloc.cc +++ b/src/core/lib/gpr/alloc.cc @@ -23,7 +23,6 @@ #include #include #include -#include "src/core/lib/gpr/alloc.h" #include "src/core/lib/profiling/timers.h" static void* zalloc_with_calloc(size_t sz) { return calloc(sz, 1); } @@ -34,61 +33,8 @@ static void* zalloc_with_gpr_malloc(size_t sz) { return p; } -static constexpr bool is_power_of_two(size_t value) { - // 2^N = 100000...000 - // 2^N - 1 = 011111...111 - // (2^N) && ((2^N)-1)) = 0 - return (value & (value - 1)) == 0; -} - -static void* aligned_alloc_with_gpr_malloc(size_t size, size_t alignment) { - GPR_DEBUG_ASSERT(is_power_of_two(alignment)); - size_t extra = alignment - 1 + sizeof(void*); - void* p = gpr_malloc(size + extra); - void** ret = (void**)(((uintptr_t)p + extra) & ~(alignment - 1)); - ret[-1] = p; - return (void*)ret; -} - -static void aligned_free_with_gpr_malloc(void* ptr) { - gpr_free((static_cast(ptr))[-1]); -} - -static void* platform_malloc_aligned(size_t size, size_t alignment) { -#if defined(GPR_HAS_ALIGNED_ALLOC) - size = GPR_ROUND_UP_TO_SPECIFIED_SIZE(size, alignment); - void* ret = aligned_alloc(alignment, size); - GPR_ASSERT(ret != nullptr); - return ret; -#elif defined(GPR_HAS_ALIGNED_MALLOC) - GPR_DEBUG_ASSERT(is_power_of_two(alignment)); - void* ret = _aligned_malloc(size, alignment); - GPR_ASSERT(ret != nullptr); - return ret; -#elif defined(GPR_HAS_POSIX_MEMALIGN) - GPR_DEBUG_ASSERT(is_power_of_two(alignment)); - GPR_DEBUG_ASSERT(alignment % sizeof(void*) == 0); - void* ret = nullptr; - GPR_ASSERT(posix_memalign(&ret, alignment, size) == 0); - return ret; -#else - return aligned_alloc_with_gpr_malloc(size, alignment); -#endif -} - -static void platform_free_aligned(void* ptr) { -#if defined(GPR_HAS_ALIGNED_ALLOC) || defined(GPR_HAS_POSIX_MEMALIGN) - free(ptr); -#elif defined(GPR_HAS_ALIGNED_MALLOC) - _aligned_free(ptr); -#else - aligned_free_with_gpr_malloc(ptr); -#endif -} - -static gpr_allocation_functions g_alloc_functions = { - malloc, zalloc_with_calloc, realloc, - free, platform_malloc_aligned, platform_free_aligned}; +static gpr_allocation_functions g_alloc_functions = {malloc, zalloc_with_calloc, + realloc, free}; gpr_allocation_functions gpr_get_allocation_functions() { return g_alloc_functions; @@ -101,12 +47,6 @@ void gpr_set_allocation_functions(gpr_allocation_functions functions) { if (functions.zalloc_fn == nullptr) { functions.zalloc_fn = zalloc_with_gpr_malloc; } - GPR_ASSERT((functions.aligned_alloc_fn == nullptr) == - (functions.aligned_free_fn == nullptr)); - if (functions.aligned_alloc_fn == nullptr) { - functions.aligned_alloc_fn = aligned_alloc_with_gpr_malloc; - functions.aligned_free_fn = aligned_free_with_gpr_malloc; - } g_alloc_functions = functions; } @@ -148,12 +88,12 @@ void* gpr_realloc(void* p, size_t size) { } void* gpr_malloc_aligned(size_t size, size_t alignment) { - GPR_TIMER_SCOPE("gpr_malloc_aligned", 0); - if (size == 0) return nullptr; - return g_alloc_functions.aligned_alloc_fn(size, alignment); + GPR_ASSERT(((alignment - 1) & alignment) == 0); // Must be power of 2. + size_t extra = alignment - 1 + sizeof(void*); + void* p = gpr_malloc(size + extra); + void** ret = (void**)(((uintptr_t)p + extra) & ~(alignment - 1)); + ret[-1] = p; + return (void*)ret; } -void gpr_free_aligned(void* ptr) { - GPR_TIMER_SCOPE("gpr_free_aligned", 0); - g_alloc_functions.aligned_free_fn(ptr); -} +void gpr_free_aligned(void* ptr) { gpr_free((static_cast(ptr))[-1]); } diff --git a/src/core/lib/gpr/alloc.h b/src/core/lib/gpr/alloc.h index 2493c87514d..762b51bf663 100644 --- a/src/core/lib/gpr/alloc.h +++ b/src/core/lib/gpr/alloc.h @@ -25,12 +25,4 @@ #define GPR_ROUND_UP_TO_ALIGNMENT_SIZE(x) \ (((x) + GPR_MAX_ALIGNMENT - 1u) & ~(GPR_MAX_ALIGNMENT - 1u)) -#define GPR_ROUND_UP_TO_CACHELINE_SIZE(x) \ - (((x) + GPR_CACHELINE_SIZE - 1u) & ~(GPR_CACHELINE_SIZE - 1u)) - -#define GPR_ROUND_UP_TO_SPECIFIED_SIZE(x, align) \ - (((x) + align - 1u) & ~(align - 1u)) - -void* gpr_malloc_cacheline(size_t size); - #endif /* GRPC_CORE_LIB_GPR_ALLOC_H */ diff --git a/src/core/lib/gprpp/arena.h b/src/core/lib/gprpp/arena.h index 915cd5c528e..b1b0c4a85cb 100644 --- a/src/core/lib/gprpp/arena.h +++ b/src/core/lib/gprpp/arena.h @@ -61,7 +61,7 @@ class Arena { GPR_ROUND_UP_TO_ALIGNMENT_SIZE(sizeof(Arena)); size = GPR_ROUND_UP_TO_ALIGNMENT_SIZE(size); size_t begin = total_used_.FetchAdd(size, MemoryOrder::RELAXED); - if (GPR_LIKELY(begin + size <= initial_zone_size_)) { + if (begin + size <= initial_zone_size_) { return reinterpret_cast(this) + base_size + begin; } else { return AllocZone(size); diff --git a/test/core/gpr/alloc_test.cc b/test/core/gpr/alloc_test.cc index fb6bc9a0bdd..0b110e2b77d 100644 --- a/test/core/gpr/alloc_test.cc +++ b/test/core/gpr/alloc_test.cc @@ -31,21 +31,12 @@ static void fake_free(void* addr) { *(static_cast(addr)) = static_cast(0xdeadd00d); } -static void* fake_aligned_malloc(size_t size, size_t alignment) { - return (void*)(size + alignment); -} - -static void fake_aligned_free(void* addr) { - *(static_cast(addr)) = static_cast(0xcafef00d); -} - static void test_custom_allocs() { const gpr_allocation_functions default_fns = gpr_get_allocation_functions(); intptr_t addr_to_free = 0; char* i; - gpr_allocation_functions fns = {fake_malloc, nullptr, - fake_realloc, fake_free, - fake_aligned_malloc, fake_aligned_free}; + gpr_allocation_functions fns = {fake_malloc, nullptr, fake_realloc, + fake_free}; gpr_set_allocation_functions(fns); GPR_ASSERT((void*)(size_t)0xdeadbeef == gpr_malloc(0xdeadbeef)); @@ -54,11 +45,6 @@ static void test_custom_allocs() { gpr_free(&addr_to_free); GPR_ASSERT(addr_to_free == (intptr_t)0xdeadd00d); - GPR_ASSERT((void*)(size_t)(0xdeadbeef + 64) == - gpr_malloc_aligned(0xdeadbeef, 64)); - gpr_free_aligned(&addr_to_free); - GPR_ASSERT(addr_to_free == (intptr_t)0xcafef00d); - /* Restore and check we don't get funky values and that we don't leak */ gpr_set_allocation_functions(default_fns); GPR_ASSERT((void*)sizeof(*i) != diff --git a/test/core/util/memory_counters.cc b/test/core/util/memory_counters.cc index 11107f670fb..787fb76e48b 100644 --- a/test/core/util/memory_counters.cc +++ b/test/core/util/memory_counters.cc @@ -90,12 +90,8 @@ static void guard_free(void* vptr) { g_old_allocs.free_fn(ptr); } -// NB: We do not specify guard_malloc_aligned/guard_free_aligned methods. Since -// they are null, calls to gpr_malloc_aligned/gpr_free_aligned are executed as a -// wrapper over gpr_malloc/gpr_free, which do use guard_malloc/guard_free, and -// thus there allocations are tracked as well. -struct gpr_allocation_functions g_guard_allocs = { - guard_malloc, nullptr, guard_realloc, guard_free, nullptr, nullptr}; +struct gpr_allocation_functions g_guard_allocs = {guard_malloc, nullptr, + guard_realloc, guard_free}; void grpc_memory_counters_init() { memset(&g_memory_counters, 0, sizeof(g_memory_counters));