[arena] Add ManagedNew(), gtest-ify test (#30159)

* [arena] Add ManagedNew(), gtest-ify test

Add a ManagedNew() method to Arena that calls the relevant destructor at Arena destruction time.

There are some cases coming up in the promise based call work where this becomes super convenient, and I expect it's likely that there are other places that's true too.

* Automated change: Fix sanity tests

* review feedback

* use construct/destruct more

* Automated change: Fix sanity tests

* Automated change: Fix sanity tests

* fix

* Automated change: Fix sanity tests

Co-authored-by: ctiller <ctiller@users.noreply.github.com>
pull/30170/head
Craig Tiller 2 years ago committed by GitHub
parent c835402dd9
commit 15ae89e1fc
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 2
      BUILD
  2. 64
      CMakeLists.txt
  3. 19
      build_autogenerated.yaml
  4. 1
      src/core/ext/filters/client_channel/subchannel.cc
  5. 22
      src/core/lib/resource_quota/arena.cc
  6. 26
      src/core/lib/resource_quota/arena.h
  7. 12
      test/core/gpr/BUILD
  8. 136
      test/core/gpr/arena_test.cc
  9. 15
      test/core/resource_quota/BUILD
  10. 171
      test/core/resource_quota/arena_test.cc
  11. 48
      tools/run_tests/generated/tests.json

@ -1918,8 +1918,10 @@ grpc_cc_library(
hdrs = [ hdrs = [
"src/core/lib/resource_quota/arena.h", "src/core/lib/resource_quota/arena.h",
], ],
external_deps = ["absl/utility"],
tags = ["grpc-autodeps"], tags = ["grpc-autodeps"],
deps = [ deps = [
"construct_destruct",
"context", "context",
"event_engine_memory_allocator", "event_engine_memory_allocator",
"gpr_base", "gpr_base",

64
CMakeLists.txt generated

@ -788,7 +788,6 @@ if(gRPC_BUILD_TESTS)
add_dependencies(buildtests_c alts_tsi_handshaker_test) add_dependencies(buildtests_c alts_tsi_handshaker_test)
add_dependencies(buildtests_c alts_tsi_utils_test) add_dependencies(buildtests_c alts_tsi_utils_test)
add_dependencies(buildtests_c alts_zero_copy_grpc_protector_test) add_dependencies(buildtests_c alts_zero_copy_grpc_protector_test)
add_dependencies(buildtests_c arena_test)
add_dependencies(buildtests_c auth_context_test) add_dependencies(buildtests_c auth_context_test)
add_dependencies(buildtests_c b64_test) add_dependencies(buildtests_c b64_test)
add_dependencies(buildtests_c bad_server_response_test) add_dependencies(buildtests_c bad_server_response_test)
@ -944,6 +943,7 @@ if(gRPC_BUILD_TESTS)
endif() endif()
add_dependencies(buildtests_cxx alts_util_test) add_dependencies(buildtests_cxx alts_util_test)
add_dependencies(buildtests_cxx arena_promise_test) add_dependencies(buildtests_cxx arena_promise_test)
add_dependencies(buildtests_cxx arena_test)
add_dependencies(buildtests_cxx async_end2end_test) add_dependencies(buildtests_cxx async_end2end_test)
add_dependencies(buildtests_cxx auth_property_iterator_test) add_dependencies(buildtests_cxx auth_property_iterator_test)
add_dependencies(buildtests_cxx authorization_matchers_test) add_dependencies(buildtests_cxx authorization_matchers_test)
@ -4633,33 +4633,6 @@ target_link_libraries(alts_zero_copy_grpc_protector_test
) )
endif()
if(gRPC_BUILD_TESTS)
add_executable(arena_test
test/core/gpr/arena_test.cc
)
target_include_directories(arena_test
PRIVATE
${CMAKE_CURRENT_SOURCE_DIR}
${CMAKE_CURRENT_SOURCE_DIR}/include
${_gRPC_ADDRESS_SORTING_INCLUDE_DIR}
${_gRPC_RE2_INCLUDE_DIR}
${_gRPC_SSL_INCLUDE_DIR}
${_gRPC_UPB_GENERATED_DIR}
${_gRPC_UPB_GRPC_GENERATED_DIR}
${_gRPC_UPB_INCLUDE_DIR}
${_gRPC_XXHASH_INCLUDE_DIR}
${_gRPC_ZLIB_INCLUDE_DIR}
)
target_link_libraries(arena_test
${_gRPC_ALLTARGETS_LIBRARIES}
grpc_test_util
)
endif() endif()
if(gRPC_BUILD_TESTS) if(gRPC_BUILD_TESTS)
@ -7692,6 +7665,41 @@ target_link_libraries(arena_promise_test
) )
endif()
if(gRPC_BUILD_TESTS)
add_executable(arena_test
test/core/resource_quota/arena_test.cc
third_party/googletest/googletest/src/gtest-all.cc
third_party/googletest/googlemock/src/gmock-all.cc
)
target_include_directories(arena_test
PRIVATE
${CMAKE_CURRENT_SOURCE_DIR}
${CMAKE_CURRENT_SOURCE_DIR}/include
${_gRPC_ADDRESS_SORTING_INCLUDE_DIR}
${_gRPC_RE2_INCLUDE_DIR}
${_gRPC_SSL_INCLUDE_DIR}
${_gRPC_UPB_GENERATED_DIR}
${_gRPC_UPB_GRPC_GENERATED_DIR}
${_gRPC_UPB_INCLUDE_DIR}
${_gRPC_XXHASH_INCLUDE_DIR}
${_gRPC_ZLIB_INCLUDE_DIR}
third_party/googletest/googletest/include
third_party/googletest/googletest
third_party/googletest/googlemock/include
third_party/googletest/googlemock
${_gRPC_PROTO_GENS_DIR}
)
target_link_libraries(arena_test
${_gRPC_PROTOBUF_LIBRARIES}
${_gRPC_ALLTARGETS_LIBRARIES}
grpc_test_util
)
endif() endif()
if(gRPC_BUILD_TESTS) if(gRPC_BUILD_TESTS)

@ -3320,15 +3320,6 @@ targets:
- test/core/tsi/alts/zero_copy_frame_protector/alts_zero_copy_grpc_protector_test.cc - test/core/tsi/alts/zero_copy_frame_protector/alts_zero_copy_grpc_protector_test.cc
deps: deps:
- grpc_test_util - grpc_test_util
- name: arena_test
build: test
language: c
headers: []
src:
- test/core/gpr/arena_test.cc
deps:
- grpc_test_util
uses_polling: false
- name: auth_context_test - name: auth_context_test
build: test build: test
language: c language: c
@ -4630,6 +4621,16 @@ targets:
- gpr - gpr
- upb - upb
uses_polling: false uses_polling: false
- name: arena_test
gtest: true
build: test
language: c++
headers: []
src:
- test/core/resource_quota/arena_test.cc
deps:
- grpc_test_util
uses_polling: false
- name: async_end2end_test - name: async_end2end_test
gtest: true gtest: true
build: test build: test

@ -25,7 +25,6 @@
#include <cstring> #include <cstring>
#include <memory> #include <memory>
#include <new> #include <new>
#include <type_traits>
#include <utility> #include <utility>
#include "absl/status/statusor.h" #include "absl/status/statusor.h"

@ -20,8 +20,11 @@
#include "src/core/lib/resource_quota/arena.h" #include "src/core/lib/resource_quota/arena.h"
#include <atomic>
#include <new> #include <new>
#include "absl/utility/utility.h"
#include <grpc/support/alloc.h> #include <grpc/support/alloc.h>
#include "src/core/lib/gpr/alloc.h" #include "src/core/lib/gpr/alloc.h"
@ -49,7 +52,7 @@ Arena::~Arena() {
Zone* z = last_zone_; Zone* z = last_zone_;
while (z) { while (z) {
Zone* prev_z = z->prev; Zone* prev_z = z->prev;
z->~Zone(); Destruct(z);
gpr_free_aligned(z); gpr_free_aligned(z);
z = prev_z; z = prev_z;
} }
@ -71,6 +74,16 @@ std::pair<Arena*, void*> Arena::CreateWithAlloc(
} }
size_t Arena::Destroy() { size_t Arena::Destroy() {
ManagedNewObject* p;
// Outer loop: clear the managed new object list.
// We do this repeatedly in case a destructor ends up allocating something.
while ((p = managed_new_head_.exchange(nullptr, std::memory_order_relaxed)) !=
nullptr) {
// Inner loop: destruct a batch of objects.
while (p != nullptr) {
Destruct(absl::exchange(p, p->next));
}
}
size_t size = total_used_.load(std::memory_order_relaxed); size_t size = total_used_.load(std::memory_order_relaxed);
memory_allocator_->Release(total_allocated_.load(std::memory_order_relaxed)); memory_allocator_->Release(total_allocated_.load(std::memory_order_relaxed));
this->~Arena(); this->~Arena();
@ -98,4 +111,11 @@ void* Arena::AllocZone(size_t size) {
return reinterpret_cast<char*>(z) + zone_base_size; return reinterpret_cast<char*>(z) + zone_base_size;
} }
void Arena::ManagedNewObject::Link(std::atomic<ManagedNewObject*>* head) {
next = head->load(std::memory_order_relaxed);
while (!head->compare_exchange_weak(next, this, std::memory_order_acq_rel,
std::memory_order_relaxed)) {
}
}
} // namespace grpc_core } // namespace grpc_core

@ -31,12 +31,12 @@
#include <atomic> #include <atomic>
#include <memory> #include <memory>
#include <new>
#include <utility> #include <utility>
#include <grpc/event_engine/memory_allocator.h> #include <grpc/event_engine/memory_allocator.h>
#include "src/core/lib/gpr/alloc.h" #include "src/core/lib/gpr/alloc.h"
#include "src/core/lib/gprpp/construct_destruct.h"
#include "src/core/lib/promise/context.h" #include "src/core/lib/promise/context.h"
#include "src/core/lib/resource_quota/memory_quota.h" #include "src/core/lib/resource_quota/memory_quota.h"
@ -76,15 +76,36 @@ class Arena {
template <typename T, typename... Args> template <typename T, typename... Args>
T* New(Args&&... args) { T* New(Args&&... args) {
T* t = static_cast<T*>(Alloc(sizeof(T))); T* t = static_cast<T*>(Alloc(sizeof(T)));
new (t) T(std::forward<Args>(args)...); Construct(t, std::forward<Args>(args)...);
return t; return t;
} }
// Like New, but has the arena call p->~T() at arena destruction time.
template <typename T, typename... Args>
T* ManagedNew(Args&&... args) {
auto* p = New<ManagedNewImpl<T>>(std::forward<Args>(args)...);
p->Link(&managed_new_head_);
return &p->t;
}
private: private:
struct Zone { struct Zone {
Zone* prev; Zone* prev;
}; };
struct ManagedNewObject {
ManagedNewObject* next = nullptr;
void Link(std::atomic<ManagedNewObject*>* head);
virtual ~ManagedNewObject() = default;
};
template <typename T>
struct ManagedNewImpl : public ManagedNewObject {
T t;
template <typename... Args>
explicit ManagedNewImpl(Args&&... args) : t(std::forward<Args>(args)...) {}
};
// Initialize an arena. // Initialize an arena.
// Parameters: // Parameters:
// initial_size: The initial size of the whole arena in bytes. These bytes // initial_size: The initial size of the whole arena in bytes. These bytes
@ -118,6 +139,7 @@ class Arena {
// and (2) the allocated memory. The arena itself maintains a pointer to the // and (2) the allocated memory. The arena itself maintains a pointer to the
// last zone; the zone list is reverse-walked during arena destruction only. // last zone; the zone list is reverse-walked during arena destruction only.
std::atomic<Zone*> last_zone_{nullptr}; std::atomic<Zone*> last_zone_{nullptr};
std::atomic<ManagedNewObject*> managed_new_head_{nullptr};
// The backing memory quota // The backing memory quota
MemoryAllocator* const memory_allocator_; MemoryAllocator* const memory_allocator_;
}; };

@ -30,18 +30,6 @@ grpc_cc_test(
], ],
) )
grpc_cc_test(
name = "arena_test",
srcs = ["arena_test.cc"],
language = "C++",
uses_event_engine = False,
uses_polling = False,
deps = [
"//:gpr",
"//test/core/util:grpc_test_util",
],
)
grpc_cc_test( grpc_cc_test(
name = "cpu_test", name = "cpu_test",
srcs = ["cpu_test.cc"], srcs = ["cpu_test.cc"],

@ -1,136 +0,0 @@
/*
*
* Copyright 2017 gRPC authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
*/
#include "src/core/lib/resource_quota/arena.h"
#include <inttypes.h>
#include <string.h>
#include "absl/strings/str_format.h"
#include "absl/strings/str_join.h"
#include <grpc/support/alloc.h>
#include <grpc/support/log.h>
#include <grpc/support/string_util.h>
#include <grpc/support/sync.h>
#include "src/core/lib/gpr/string.h"
#include "src/core/lib/gpr/useful.h"
#include "src/core/lib/gprpp/thd.h"
#include "src/core/lib/resource_quota/resource_quota.h"
#include "test/core/util/test_config.h"
using grpc_core::Arena;
static auto* g_memory_allocator = new grpc_core::MemoryAllocator(
grpc_core::ResourceQuota::Default()->memory_quota()->CreateMemoryAllocator(
"test"));
static void test_noop(void) { Arena::Create(1, g_memory_allocator)->Destroy(); }
static void test(const char* name, size_t init_size, const size_t* allocs,
size_t nallocs) {
std::vector<std::string> parts;
parts.push_back(
absl::StrFormat("test '%s': %" PRIdPTR " <- {", name, init_size));
for (size_t i = 0; i < nallocs; i++) {
parts.push_back(absl::StrFormat("%" PRIdPTR ",", allocs[i]));
}
parts.push_back("}");
std::string s = absl::StrJoin(parts, "");
gpr_log(GPR_INFO, "%s", s.c_str());
Arena* a = Arena::Create(init_size, g_memory_allocator);
void** ps = static_cast<void**>(gpr_zalloc(sizeof(*ps) * nallocs));
for (size_t i = 0; i < nallocs; i++) {
ps[i] = a->Alloc(allocs[i]);
// ensure the returned address is aligned
GPR_ASSERT(((intptr_t)ps[i] & 0xf) == 0);
// ensure no duplicate results
for (size_t j = 0; j < i; j++) {
GPR_ASSERT(ps[i] != ps[j]);
}
// ensure writable
memset(ps[i], 1, allocs[i]);
}
a->Destroy();
gpr_free(ps);
}
#define TEST(name, init_size, ...) \
static const size_t allocs_##name[] = {__VA_ARGS__}; \
test(#name, init_size, allocs_##name, GPR_ARRAY_SIZE(allocs_##name))
#define CONCURRENT_TEST_THREADS 10
size_t concurrent_test_iterations() {
if (sizeof(void*) < 8) return 1000;
return 100000;
}
typedef struct {
gpr_event ev_start;
Arena* arena;
} concurrent_test_args;
static void concurrent_test_body(void* arg) {
concurrent_test_args* a = static_cast<concurrent_test_args*>(arg);
gpr_event_wait(&a->ev_start, gpr_inf_future(GPR_CLOCK_REALTIME));
for (size_t i = 0; i < concurrent_test_iterations(); i++) {
*static_cast<char*>(a->arena->Alloc(1)) = static_cast<char>(i);
}
}
static void concurrent_test(void) {
gpr_log(GPR_DEBUG, "concurrent_test");
concurrent_test_args args;
gpr_event_init(&args.ev_start);
args.arena = Arena::Create(1024, g_memory_allocator);
grpc_core::Thread thds[CONCURRENT_TEST_THREADS];
for (int i = 0; i < CONCURRENT_TEST_THREADS; i++) {
thds[i] =
grpc_core::Thread("grpc_concurrent_test", concurrent_test_body, &args);
thds[i].Start();
}
gpr_event_set(&args.ev_start, reinterpret_cast<void*>(1));
for (auto& th : thds) {
th.Join();
}
args.arena->Destroy();
}
int main(int argc, char* argv[]) {
grpc::testing::TestEnvironment env(&argc, argv);
test_noop();
TEST(0_1, 0, 1);
TEST(1_1, 1, 1);
TEST(1_2, 1, 2);
TEST(1_3, 1, 3);
TEST(1_inc, 1, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11);
TEST(6_123, 6, 1, 2, 3);
concurrent_test();
return 0;
}

@ -19,6 +19,21 @@ licenses(["notice"])
grpc_package(name = "test/core/resource_quota") grpc_package(name = "test/core/resource_quota")
grpc_cc_test(
name = "arena_test",
srcs = ["arena_test.cc"],
external_deps = [
"gtest",
],
language = "C++",
uses_event_engine = False,
uses_polling = False,
deps = [
"//:gpr",
"//test/core/util:grpc_test_util",
],
)
grpc_cc_library( grpc_cc_library(
name = "call_checker", name = "call_checker",
testonly = True, testonly = True,

@ -0,0 +1,171 @@
/*
*
* Copyright 2017 gRPC authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
*/
#include "src/core/lib/resource_quota/arena.h"
#include <inttypes.h>
#include <string.h>
#include <algorithm>
#include <ostream>
#include <string>
#include <vector>
#include "absl/memory/memory.h"
#include "absl/strings/str_join.h"
#include "gtest/gtest.h"
#include <grpc/grpc.h>
#include <grpc/support/sync.h>
#include <grpc/support/time.h>
#include "src/core/lib/gprpp/ref_counted_ptr.h"
#include "src/core/lib/gprpp/thd.h"
#include "src/core/lib/resource_quota/resource_quota.h"
using grpc_core::Arena;
static auto* g_memory_allocator = new grpc_core::MemoryAllocator(
grpc_core::ResourceQuota::Default()->memory_quota()->CreateMemoryAllocator(
"test"));
TEST(ArenaTest, NoOp) { Arena::Create(1, g_memory_allocator)->Destroy(); }
TEST(ArenaTest, ManagedNew) {
Arena* arena = Arena::Create(1, g_memory_allocator);
for (int i = 0; i < 100; i++) {
arena->ManagedNew<std::unique_ptr<int>>(absl::make_unique<int>(i));
}
arena->Destroy();
}
struct AllocShape {
size_t initial_size;
std::vector<size_t> allocs;
};
std::ostream& operator<<(std::ostream& out, const AllocShape& shape) {
out << "AllocShape{initial_size=" << shape.initial_size
<< ", allocs=" << absl::StrJoin(shape.allocs, ",") << "}";
return out;
}
class AllocTest : public ::testing::TestWithParam<AllocShape> {};
TEST_P(AllocTest, Works) {
Arena* a = Arena::Create(GetParam().initial_size, g_memory_allocator);
std::vector<void*> allocated;
for (auto alloc : GetParam().allocs) {
void* p = a->Alloc(alloc);
// ensure the returned address is aligned
EXPECT_EQ(((intptr_t)p & 0xf), 0);
// ensure no duplicate results
for (auto other_p : allocated) {
EXPECT_NE(p, other_p);
}
// ensure writable
memset(p, 1, alloc);
allocated.push_back(p);
}
a->Destroy();
}
INSTANTIATE_TEST_SUITE_P(
AllocTests, AllocTest,
::testing::Values(AllocShape{0, {1}}, AllocShape{1, {1}},
AllocShape{1, {2}}, AllocShape{1, {3}},
AllocShape{1, {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}},
AllocShape{6, {1, 2, 3}}));
#define CONCURRENT_TEST_THREADS 10
size_t concurrent_test_iterations() {
if (sizeof(void*) < 8) return 1000;
return 100000;
}
typedef struct {
gpr_event ev_start;
Arena* arena;
} concurrent_test_args;
TEST(ArenaTest, ConcurrentAlloc) {
concurrent_test_args args;
gpr_event_init(&args.ev_start);
args.arena = Arena::Create(1024, g_memory_allocator);
grpc_core::Thread thds[CONCURRENT_TEST_THREADS];
for (int i = 0; i < CONCURRENT_TEST_THREADS; i++) {
thds[i] = grpc_core::Thread(
"grpc_concurrent_test",
[](void* arg) {
concurrent_test_args* a = static_cast<concurrent_test_args*>(arg);
gpr_event_wait(&a->ev_start, gpr_inf_future(GPR_CLOCK_REALTIME));
for (size_t i = 0; i < concurrent_test_iterations(); i++) {
*static_cast<char*>(a->arena->Alloc(1)) = static_cast<char>(i);
}
},
&args);
thds[i].Start();
}
gpr_event_set(&args.ev_start, reinterpret_cast<void*>(1));
for (auto& th : thds) {
th.Join();
}
args.arena->Destroy();
}
TEST(ArenaTest, ConcurrentManagedNew) {
concurrent_test_args args;
gpr_event_init(&args.ev_start);
args.arena = Arena::Create(1024, g_memory_allocator);
grpc_core::Thread thds[CONCURRENT_TEST_THREADS];
for (int i = 0; i < CONCURRENT_TEST_THREADS; i++) {
thds[i] = grpc_core::Thread(
"grpc_concurrent_test",
[](void* arg) {
concurrent_test_args* a = static_cast<concurrent_test_args*>(arg);
gpr_event_wait(&a->ev_start, gpr_inf_future(GPR_CLOCK_REALTIME));
for (size_t i = 0; i < concurrent_test_iterations(); i++) {
a->arena->ManagedNew<std::unique_ptr<int>>(
absl::make_unique<int>(static_cast<int>(i)));
}
},
&args);
thds[i].Start();
}
gpr_event_set(&args.ev_start, reinterpret_cast<void*>(1));
for (auto& th : thds) {
th.Join();
}
args.arena->Destroy();
}
int main(int argc, char* argv[]) {
::testing::InitGoogleTest(&argc, argv);
return RUN_ALL_TESTS();
}

@ -313,30 +313,6 @@
], ],
"uses_polling": true "uses_polling": true
}, },
{
"args": [],
"benchmark": false,
"ci_platforms": [
"linux",
"mac",
"posix",
"windows"
],
"cpu_cost": 1.0,
"exclude_configs": [],
"exclude_iomgrs": [],
"flaky": false,
"gtest": false,
"language": "c",
"name": "arena_test",
"platforms": [
"linux",
"mac",
"posix",
"windows"
],
"uses_polling": false
},
{ {
"args": [], "args": [],
"benchmark": false, "benchmark": false,
@ -2659,6 +2635,30 @@
], ],
"uses_polling": false "uses_polling": false
}, },
{
"args": [],
"benchmark": false,
"ci_platforms": [
"linux",
"mac",
"posix",
"windows"
],
"cpu_cost": 1.0,
"exclude_configs": [],
"exclude_iomgrs": [],
"flaky": false,
"gtest": true,
"language": "c++",
"name": "arena_test",
"platforms": [
"linux",
"mac",
"posix",
"windows"
],
"uses_polling": false
},
{ {
"args": [], "args": [],
"benchmark": false, "benchmark": false,

Loading…
Cancel
Save