Skip to content

Commit f6a8ba3

Browse files
ckennellycopybara-github
authored andcommitted
Strongly type alignment value in profile samples.
PiperOrigin-RevId: 819260208 Change-Id: Id9c7e03e5a45c4ca8656344b2823315d6e8ac27c
1 parent 963a5cb commit f6a8ba3

File tree

9 files changed

+76
-53
lines changed

9 files changed

+76
-53
lines changed

tcmalloc/deallocation_profiler.cc

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include <functional>
2323
#include <limits>
2424
#include <memory>
25+
#include <new>
2526
#include <optional>
2627
#include <type_traits>
2728
#include <utility>
@@ -671,7 +672,12 @@ void DeallocationProfiler::DeallocationStackTraceTable::Iterate(
671672
const auto bucketize = internal::LifetimeNsToBucketedDuration;
672673
Profile::Sample sample;
673674
sample.sum = sum, sample.requested_size = k.alloc.requested_size,
674-
sample.requested_alignment = k.alloc.requested_alignment,
675+
// TODO(b/404341539): Propagate type to StackTrace.
676+
sample.requested_alignment =
677+
k.alloc.requested_alignment > 0
678+
? std::make_optional(static_cast<std::align_val_t>(
679+
k.alloc.requested_alignment))
680+
: std::nullopt,
675681
sample.allocated_size = allocated_size, sample.profile_id = pair_id++,
676682
// Set the is_censored flag so that when we create a proto
677683
// sample later we can treat the *_lifetime accordingly.

tcmalloc/internal/profile_builder.cc

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -584,7 +584,10 @@ static absl::Status MakeLifetimeProfileProto(const tcmalloc::Profile& profile,
584584
// The following three fields are common across profiles.
585585
add_positive_label(bytes_id, bytes_id, entry.allocated_size);
586586
add_positive_label(request_id, bytes_id, entry.requested_size);
587-
add_positive_label(alignment_id, bytes_id, entry.requested_alignment);
587+
if (entry.requested_alignment.has_value()) {
588+
add_positive_label(alignment_id, bytes_id,
589+
static_cast<size_t>(*entry.requested_alignment));
590+
}
588591

589592
// The following fields are specific to lifetime (deallocation) profiler.
590593
add_positive_label(callstack_pair_id, count_id, entry.profile_id);
@@ -792,7 +795,10 @@ absl::StatusOr<std::unique_ptr<perftools::profiles::Profile>> MakeProfileProto(
792795

793796
add_positive_label(bytes_id, bytes_id, entry.allocated_size);
794797
add_positive_label(request_id, bytes_id, entry.requested_size);
795-
add_positive_label(alignment_id, bytes_id, entry.requested_alignment);
798+
if (entry.requested_alignment.has_value()) {
799+
add_positive_label(alignment_id, bytes_id,
800+
static_cast<size_t>(*entry.requested_alignment));
801+
}
796802
add_positive_label(size_returning_id, 0, entry.requested_size_returning);
797803
add_positive_label(stale_scan_period_id, seconds_id,
798804
data.stale_scan_period.value_or(0));

tcmalloc/internal/profile_builder_test.cc

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
#include <cstdint>
2626
#include <cstdlib>
2727
#include <memory>
28+
#include <new>
2829
#include <optional>
2930
#include <string>
3031
#include <tuple>
@@ -336,7 +337,7 @@ perftools::profiles::Profile MakeTestProfile(
336337
sample.sum = 1234;
337338
sample.count = 2;
338339
sample.requested_size = 2;
339-
sample.requested_alignment = 4;
340+
sample.requested_alignment = std::align_val_t{4};
340341
sample.requested_size_returning = true;
341342
sample.allocated_size = 16;
342343

@@ -385,7 +386,7 @@ perftools::profiles::Profile MakeTestProfile(
385386
sample.sum = 2345;
386387
sample.count = 5;
387388
sample.requested_size = 4;
388-
sample.requested_alignment = 0;
389+
sample.requested_alignment = std::nullopt;
389390
sample.requested_size_returning = false;
390391
sample.allocated_size = 8;
391392
sample.span_start_address = ptr1;
@@ -427,7 +428,7 @@ perftools::profiles::Profile MakeTestProfile(
427428
sample.sum = 2345;
428429
sample.count = 8;
429430
sample.requested_size = 16;
430-
sample.requested_alignment = 0;
431+
sample.requested_alignment = std::nullopt;
431432
sample.requested_size_returning = true;
432433
sample.allocated_size = 16;
433434
// This stack is mostly artificial, but we include a real symbol from the
@@ -451,7 +452,7 @@ perftools::profiles::Profile MakeTestProfile(
451452
sample.sum = 1235;
452453
sample.count = 2;
453454
sample.requested_size = 2;
454-
sample.requested_alignment = 4;
455+
sample.requested_alignment = std::align_val_t{4};
455456
sample.requested_size_returning = true;
456457
sample.allocated_size = 16;
457458

@@ -729,7 +730,7 @@ TEST(ProfileBuilderTest, PeakHeapProfile) {
729730
sample.sum = 1234;
730731
sample.count = 2;
731732
sample.requested_size = 2;
732-
sample.requested_alignment = 4;
733+
sample.requested_alignment = std::align_val_t{4};
733734
sample.requested_size_returning = true;
734735
sample.allocated_size = 16;
735736

@@ -749,7 +750,7 @@ TEST(ProfileBuilderTest, PeakHeapProfile) {
749750
sample.sum = 2345;
750751
sample.count = 5;
751752
sample.requested_size = 4;
752-
sample.requested_alignment = 0;
753+
sample.requested_alignment = std::nullopt;
753754
sample.requested_size_returning = false;
754755
sample.allocated_size = 8;
755756

@@ -814,7 +815,7 @@ TEST(ProfileBuilderTest, LifetimeProfile) {
814815
.count = 2,
815816
// Common information we retain in the lifetime profile.
816817
.requested_size = 2,
817-
.requested_alignment = 4,
818+
.requested_alignment = std::align_val_t{4},
818819
.allocated_size = 16,
819820
// Lifetime specific information in each sample.
820821
.profile_id = 33,

tcmalloc/malloc_extension.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ class Profile final {
148148
size_t requested_size;
149149
// The requested alignment if specified. malloc() implicitly aligns to
150150
// alignof(std::max_align_t) and this value is used.
151-
size_t requested_alignment;
151+
std::optional<std::align_val_t> requested_alignment;
152152
// The actual size allocated considering size, implicit/explicit alignment,
153153
// GWP-ASan.
154154
size_t allocated_size;

tcmalloc/profile_test.cc

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -167,23 +167,23 @@ TEST(AllocationSampleTest, SampleAccuracy) {
167167
// (object size, object alignment, keep objects)
168168
struct Requests {
169169
size_t size;
170-
size_t alignment;
170+
std::optional<std::align_val_t> alignment;
171171
std::optional<tcmalloc::hot_cold_t> hot_cold;
172172
bool expected_hot;
173173
bool keep;
174174
// objects we don't delete as we go
175175
void* list = nullptr;
176176
};
177177
std::vector<Requests> sizes = {
178-
{8, 0, std::nullopt, true, false},
179-
{16, 16, std::nullopt, true, true},
180-
{1024, 0, std::nullopt, true, false},
181-
{64 * 1024, 64, std::nullopt, true, false},
182-
{512 * 1024, 0, std::nullopt, true, true},
183-
{1024 * 1024, 128, std::nullopt, true, true},
184-
{32, 0, tcmalloc::hot_cold_t{0}, false, true},
185-
{64, 0, tcmalloc::hot_cold_t{255}, true, true},
186-
{8192, 0, tcmalloc::hot_cold_t{0}, false, true},
178+
{8, std::nullopt, std::nullopt, true, false},
179+
{16, std::align_val_t{16}, std::nullopt, true, true},
180+
{1024, std::nullopt, std::nullopt, true, false},
181+
{64 * 1024, std::align_val_t{64}, std::nullopt, true, false},
182+
{512 * 1024, std::nullopt, std::nullopt, true, true},
183+
{1024 * 1024, std::align_val_t{128}, std::nullopt, true, true},
184+
{32, std::nullopt, tcmalloc::hot_cold_t{0}, false, true},
185+
{64, std::nullopt, tcmalloc::hot_cold_t{255}, true, true},
186+
{8192, std::nullopt, tcmalloc::hot_cold_t{0}, false, true},
187187
};
188188
absl::btree_set<size_t> sizes_expected;
189189
for (auto s : sizes) {
@@ -196,17 +196,17 @@ TEST(AllocationSampleTest, SampleAccuracy) {
196196
for (auto& s : sizes) {
197197
for (size_t bytes = 0; bytes < kTotalPerSize; bytes += s.size) {
198198
void* obj;
199-
if (s.alignment > 0) {
200-
obj = operator new(s.size, static_cast<std::align_val_t>(s.alignment));
199+
if (s.alignment.has_value()) {
200+
obj = operator new(s.size, *s.alignment);
201201
} else if (s.hot_cold.has_value()) {
202202
obj = operator new(s.size, *s.hot_cold);
203203
} else {
204204
obj = operator new(s.size);
205205
}
206206
if (s.keep) {
207207
tcmalloc_internal::SLL_Push(&s.list, obj);
208-
} else if (s.alignment > 0) {
209-
operator delete(obj, static_cast<std::align_val_t>(s.alignment));
208+
} else if (s.alignment.has_value()) {
209+
operator delete(obj, *s.alignment);
210210
} else {
211211
sized_delete(obj, s.size);
212212
}
@@ -218,7 +218,7 @@ TEST(AllocationSampleTest, SampleAccuracy) {
218218
absl::flat_hash_map<size_t, size_t> m;
219219

220220
// size -> alignment request
221-
absl::flat_hash_map<size_t, size_t> alignment;
221+
absl::flat_hash_map<size_t, std::optional<std::align_val_t>> alignment;
222222

223223
// size -> access_hint
224224
absl::flat_hash_map<size_t, hot_cold_t> access_hint;
@@ -267,8 +267,8 @@ TEST(AllocationSampleTest, SampleAccuracy) {
267267
for (auto& s : sizes) {
268268
while (s.list != nullptr) {
269269
void* obj = tcmalloc_internal::SLL_Pop(&s.list);
270-
if (s.alignment > 0) {
271-
operator delete(obj, static_cast<std::align_val_t>(s.alignment));
270+
if (s.alignment.has_value()) {
271+
operator delete(obj, *s.alignment);
272272
} else {
273273
operator delete(obj);
274274
}

tcmalloc/stack_trace_table.cc

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@
1818
#include <string.h>
1919

2020
#include <cstdint>
21+
#include <new>
22+
#include <optional>
2123

2224
#include "absl/functional/function_ref.h"
2325
#include "tcmalloc/common.h"
@@ -78,7 +80,13 @@ void StackTraceTable::AddTrace(double sample_weight, const StackTrace& t) {
7880
}
7981
s->sample.sum = s->sample.count * allocated_size;
8082
s->sample.requested_size = requested_size;
81-
s->sample.requested_alignment = t.requested_alignment;
83+
// TODO(b/404341539): Propagate type to StackTrace.
84+
if (t.requested_alignment > 0) {
85+
s->sample.requested_alignment =
86+
static_cast<std::align_val_t>(t.requested_alignment);
87+
} else {
88+
s->sample.requested_alignment = std::nullopt;
89+
}
8290
s->sample.requested_size_returning = t.requested_size_returning;
8391
s->sample.allocated_size = allocated_size;
8492
s->sample.alloc_handle = t.sampled_alloc_handle;

tcmalloc/stack_trace_table_test.cc

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919

2020
#include <algorithm>
2121
#include <initializer_list>
22+
#include <new>
23+
#include <optional>
2224
#include <string>
2325
#include <vector>
2426

@@ -42,7 +44,7 @@ struct AllocationEntry {
4244
int64_t sum;
4345
int count;
4446
size_t requested_size;
45-
size_t requested_alignment;
47+
std::optional<std::align_val_t> requested_alignment;
4648
size_t allocated_size;
4749
MallocHook::AllocHandle alloc_handle;
4850
uint8_t access_hint;
@@ -160,7 +162,7 @@ TEST(StackTraceTableTest, StackTraceTable) {
160162
.sum = 1024,
161163
.count = 1,
162164
.requested_size = 512,
163-
.requested_alignment = 16,
165+
.requested_alignment = std::align_val_t{16},
164166
.allocated_size = 1024,
165167
.alloc_handle = AllocHandle{1},
166168
.access_hint = 3,
@@ -185,7 +187,7 @@ TEST(StackTraceTableTest, StackTraceTable) {
185187
.sum = 512,
186188
.count = 1,
187189
.requested_size = 375,
188-
.requested_alignment = 0,
190+
.requested_alignment = std::nullopt,
189191
.allocated_size = 512,
190192
.alloc_handle = AllocHandle{2},
191193
.access_hint = 254,
@@ -221,7 +223,7 @@ TEST(StackTraceTableTest, StackTraceTable) {
221223
.sum = t1_sampled_weight * 1024,
222224
.count = t1_sampled_weight,
223225
.requested_size = 512,
224-
.requested_alignment = 16,
226+
.requested_alignment = std::align_val_t{16},
225227
.allocated_size = 1024,
226228
.alloc_handle = AllocHandle{1},
227229
.access_hint = 3,
@@ -294,7 +296,7 @@ TEST(StackTraceTableTest, StackTraceTable) {
294296
.sum = 17,
295297
.count = 1,
296298
.requested_size = 13,
297-
.requested_alignment = 0,
299+
.requested_alignment = std::nullopt,
298300
.allocated_size = 17,
299301
.alloc_handle = AllocHandle{3},
300302
.access_hint = 3,
@@ -332,7 +334,7 @@ TEST(StackTraceTableTest, StackTraceTable) {
332334
.sum = 1024,
333335
.count = 1,
334336
.requested_size = 512,
335-
.requested_alignment = 32,
337+
.requested_alignment = std::align_val_t{32},
336338
.allocated_size = 1024,
337339
.alloc_handle = AllocHandle{4},
338340
.access_hint = 3,
@@ -370,7 +372,7 @@ TEST(StackTraceTableTest, StackTraceTable) {
370372
.sum = 1024,
371373
.count = 1,
372374
.requested_size = 512,
373-
.requested_alignment = 32,
375+
.requested_alignment = std::align_val_t{32},
374376
.allocated_size = 1024,
375377
.alloc_handle = AllocHandle{5},
376378
.access_hint = 4,

tcmalloc/testing/aligned_new_test.cc

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
#include <stdint.h>
1717

1818
#include <memory>
19+
#include <new>
1920
#include <type_traits>
2021
#include <utility>
2122
#include <vector>
@@ -91,18 +92,20 @@ TYPED_TEST_P(AlignedNew, AlignedTest) {
9192
// __STDCPP_DEFAULT_NEW_ALIGNMENT__.
9293
//
9394
// (size, alignment) -> count
94-
using CountMap = absl::flat_hash_map<std::pair<size_t, size_t>, size_t>;
95+
using CountMap =
96+
absl::flat_hash_map<std::pair<size_t, std::align_val_t>, size_t>;
9597
CountMap counts;
9698

9799
profile.Iterate([&](const Profile::Sample& e) {
98-
counts[{e.requested_size, e.requested_alignment}] += e.count;
100+
counts[{e.requested_size,
101+
e.requested_alignment.value_or(std::align_val_t{0})}] += e.count;
99102
});
100103

101104
if (!tcmalloc_internal::kSanitizerPresent) {
102-
size_t expected_alignment = 0;
105+
std::align_val_t expected_alignment{0};
103106
#if defined(__STDCPP_DEFAULT_NEW_ALIGNMENT__)
104107
if (alignof(TypeParam) > __STDCPP_DEFAULT_NEW_ALIGNMENT__) {
105-
expected_alignment = alignof(TypeParam);
108+
expected_alignment = static_cast<std::align_val_t>(alignof(TypeParam));
106109
}
107110
#endif
108111
EXPECT_GT((counts[{sizeof(TypeParam), expected_alignment}]), 0);

0 commit comments

Comments
 (0)