Skip to content

Commit

Permalink
GeneralPurposeAllocator.searchBucket: check current bucket before sea…
Browse files Browse the repository at this point in the history
…rching the list

Follow up to #17383. This is a minor optimization that only matters when a small allocation is resized/free'd soon after it is allocated.

The only real difference I was able to observe with this was via a synthetic benchmark that allocates a full bucket and then frees all but one of the slots, over and over in a loop:

Debug build:

Benchmark 1 (9 runs): gpa-degen-master.exe
  measurement          mean ± σ            min … max           outliers         delta
  wall_time           575ms ± 5.19ms     569ms …  583ms          0 ( 0%)        0%
  peak_rss           43.8MB ± 1.37KB    43.8MB … 43.8MB          1 (11%)        0%
Benchmark 2 (10 runs): gpa-degen-search-cur.exe
  measurement          mean ± σ            min … max           outliers         delta
  wall_time           532ms ± 5.55ms     520ms …  539ms          0 ( 0%)        ⚡-  7.5% ±  0.9%
  peak_rss           43.8MB ± 65.2KB    43.8MB … 44.0MB          1 (10%)          +  0.0% ±  0.1%

ReleaseFast build:

Benchmark 1 (129 runs): gpa-degen-master-release.exe
  measurement          mean ± σ            min … max           outliers         delta
  wall_time          38.9ms ± 1.12ms    36.7ms … 42.4ms          8 ( 6%)        0%
  peak_rss           23.2MB ± 2.39KB    23.2MB … 23.2MB          0 ( 0%)        0%
Benchmark 2 (151 runs): gpa-degen-search-cur-release.exe
  measurement          mean ± σ            min … max           outliers         delta
  wall_time          33.2ms ±  999us    31.9ms … 36.3ms         20 (13%)        ⚡- 14.7% ±  0.6%
  peak_rss           23.2MB ± 2.26KB    23.2MB … 23.2MB          0 ( 0%)          +  0.0% ±  0.0%
  • Loading branch information
squeek502 authored and andrewrk committed Oct 4, 2023
1 parent 11489bb commit ec0f76c
Showing 1 changed file with 14 additions and 10 deletions.
24 changes: 14 additions & 10 deletions lib/std/heap/general_purpose_allocator.zig
Original file line number Diff line number Diff line change
Expand Up @@ -541,10 +541,14 @@ pub fn GeneralPurposeAllocator(comptime config: Config) type {
fn searchBucket(
buckets: *Buckets,
addr: usize,
current_bucket: ?*BucketHeader,
) ?*BucketHeader {
const search_page = mem.alignBackward(usize, addr, page_size);
const search_page: [*]align(page_size) u8 = @ptrFromInt(mem.alignBackward(usize, addr, page_size));
if (current_bucket != null and current_bucket.?.page == search_page) {
return current_bucket;
}
var search_header: BucketHeader = undefined;
search_header.page = @ptrFromInt(search_page);
search_header.page = search_page;
const entry = buckets.getEntryFor(&search_header);
return if (entry.node) |node| node.key else null;
}
Expand Down Expand Up @@ -712,15 +716,15 @@ pub fn GeneralPurposeAllocator(comptime config: Config) type {
var bucket_index = math.log2(size_class_hint);
var size_class: usize = size_class_hint;
const bucket = while (bucket_index < small_bucket_count) : (bucket_index += 1) {
if (searchBucket(&self.buckets[bucket_index], @intFromPtr(old_mem.ptr))) |bucket| {
if (searchBucket(&self.buckets[bucket_index], @intFromPtr(old_mem.ptr), self.cur_buckets[bucket_index])) |bucket| {
break bucket;
}
size_class *= 2;
} else blk: {
if (config.retain_metadata) {
if (!self.large_allocations.contains(@intFromPtr(old_mem.ptr))) {
// object not in active buckets or a large allocation, so search empty buckets
if (searchBucket(&self.empty_buckets, @intFromPtr(old_mem.ptr))) |bucket| {
if (searchBucket(&self.empty_buckets, @intFromPtr(old_mem.ptr), null)) |bucket| {
// bucket is empty so is_used below will always be false and we exit there
break :blk bucket;
} else {
Expand Down Expand Up @@ -830,15 +834,15 @@ pub fn GeneralPurposeAllocator(comptime config: Config) type {
var bucket_index = math.log2(size_class_hint);
var size_class: usize = size_class_hint;
const bucket = while (bucket_index < small_bucket_count) : (bucket_index += 1) {
if (searchBucket(&self.buckets[bucket_index], @intFromPtr(old_mem.ptr))) |bucket| {
if (searchBucket(&self.buckets[bucket_index], @intFromPtr(old_mem.ptr), self.cur_buckets[bucket_index])) |bucket| {
break bucket;
}
size_class *= 2;
} else blk: {
if (config.retain_metadata) {
if (!self.large_allocations.contains(@intFromPtr(old_mem.ptr))) {
// object not in active buckets or a large allocation, so search empty buckets
if (searchBucket(&self.empty_buckets, @intFromPtr(old_mem.ptr))) |bucket| {
if (searchBucket(&self.empty_buckets, @intFromPtr(old_mem.ptr), null)) |bucket| {
// bucket is empty so is_used below will always be false and we exit there
break :blk bucket;
} else {
Expand Down Expand Up @@ -1387,10 +1391,10 @@ test "double frees" {
const index: usize = 6;
const size_class: usize = @as(usize, 1) << 6;
const small = try allocator.alloc(u8, size_class);
try std.testing.expect(GPA.searchBucket(&gpa.buckets[index], @intFromPtr(small.ptr)) != null);
try std.testing.expect(GPA.searchBucket(&gpa.buckets[index], @intFromPtr(small.ptr), gpa.cur_buckets[index]) != null);
allocator.free(small);
try std.testing.expect(GPA.searchBucket(&gpa.buckets[index], @intFromPtr(small.ptr)) == null);
try std.testing.expect(GPA.searchBucket(&gpa.empty_buckets, @intFromPtr(small.ptr)) != null);
try std.testing.expect(GPA.searchBucket(&gpa.buckets[index], @intFromPtr(small.ptr), gpa.cur_buckets[index]) == null);
try std.testing.expect(GPA.searchBucket(&gpa.empty_buckets, @intFromPtr(small.ptr), null) != null);

// detect a large allocation double free
const large = try allocator.alloc(u8, 2 * page_size);
Expand All @@ -1408,7 +1412,7 @@ test "double frees" {
// check that flushing retained metadata doesn't disturb live allocations
gpa.flushRetainedMetadata();
try std.testing.expect(gpa.empty_buckets.root == null);
try std.testing.expect(GPA.searchBucket(&gpa.buckets[index], @intFromPtr(normal_small.ptr)) != null);
try std.testing.expect(GPA.searchBucket(&gpa.buckets[index], @intFromPtr(normal_small.ptr), gpa.cur_buckets[index]) != null);
try std.testing.expect(gpa.large_allocations.contains(@intFromPtr(normal_large.ptr)));
try std.testing.expect(!gpa.large_allocations.contains(@intFromPtr(large.ptr)));
}
Expand Down

0 comments on commit ec0f76c

Please sign in to comment.