Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

std/hash_map: fix ensureUnusedCapacity() over-allocating #9365

Merged
merged 2 commits into from Jul 12, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 14 additions & 1 deletion lib/std/hash_map.zig
Expand Up @@ -848,7 +848,7 @@ pub fn HashMapUnmanaged(
return ensureUnusedCapacityContext(self, allocator, additional_size, undefined);
}
pub fn ensureUnusedCapacityContext(self: *Self, allocator: *Allocator, additional_size: Size, ctx: Context) !void {
return ensureTotalCapacityContext(self, allocator, self.capacity() + additional_size, ctx);
return ensureTotalCapacityContext(self, allocator, self.count() + additional_size, ctx);
}

pub fn clearRetainingCapacity(self: *Self) void {
Expand Down Expand Up @@ -1956,6 +1956,19 @@ test "std.hash_map getOrPutAdapted" {
}
}

test "std.hash_map ensureUnusedCapacity" {
var map = AutoHashMap(u64, u64).init(testing.allocator);
defer map.deinit();

try map.ensureUnusedCapacity(32);
const capacity = map.capacity();
try map.ensureUnusedCapacity(32);

// Repeated ensureUnusedCapacity() calls with no insertions between
// should not change the capacity.
try testing.expectEqual(capacity, map.capacity());
}

test "compile everything" {
std.testing.refAllDecls(AutoHashMap(i32, i32));
std.testing.refAllDecls(StringHashMap([]const u8));
Expand Down
6 changes: 3 additions & 3 deletions src/codegen/c.zig
Expand Up @@ -39,11 +39,11 @@ const BlockData = struct {
};

pub const CValueMap = std.AutoHashMap(*Inst, CValue);
pub const TypedefMap = std.HashMap(
pub const TypedefMap = std.ArrayHashMap(
Type,
struct { name: []const u8, rendered: []u8 },
Type.HashContext,
std.hash_map.default_max_load_percentage,
Type.HashContext32,
true,
);

fn formatTypeAsCIdentifier(
Expand Down
2 changes: 1 addition & 1 deletion src/codegen/spirv.zig
Expand Up @@ -18,7 +18,7 @@ const Inst = ir.Inst;
pub const Word = u32;
pub const ResultId = u32;

pub const TypeMap = std.HashMap(Type, u32, Type.HashContext, std.hash_map.default_max_load_percentage);
pub const TypeMap = std.HashMap(Type, u32, Type.HashContext64, std.hash_map.default_max_load_percentage);
pub const InstMap = std.AutoHashMap(*Inst, ResultId);

const IncomingBlock = struct {
Expand Down
2 changes: 1 addition & 1 deletion src/link.zig
Expand Up @@ -168,7 +168,7 @@ pub const File = struct {
};

/// For DWARF .debug_info.
pub const DbgInfoTypeRelocsTable = std.HashMapUnmanaged(Type, DbgInfoTypeReloc, Type.HashContext, std.hash_map.default_max_load_percentage);
pub const DbgInfoTypeRelocsTable = std.HashMapUnmanaged(Type, DbgInfoTypeReloc, Type.HashContext64, std.hash_map.default_max_load_percentage);

/// For DWARF .debug_info.
pub const DbgInfoTypeReloc = struct {
Expand Down
13 changes: 5 additions & 8 deletions src/link/C.zig
Expand Up @@ -89,8 +89,7 @@ pub fn freeDecl(self: *C, decl: *Module.Decl) void {
fn deinitDecl(gpa: *Allocator, decl: *Module.Decl) void {
decl.link.c.code.deinit(gpa);
decl.fn_link.c.fwd_decl.deinit(gpa);
var it = decl.fn_link.c.typedefs.valueIterator();
while (it.next()) |value| {
for (decl.fn_link.c.typedefs.values()) |value| {
gpa.free(value.rendered);
}
decl.fn_link.c.typedefs.deinit(gpa);
Expand All @@ -108,8 +107,7 @@ pub fn updateDecl(self: *C, module: *Module, decl: *Module.Decl) !void {
const code = &decl.link.c.code;
fwd_decl.shrinkRetainingCapacity(0);
{
var it = typedefs.valueIterator();
while (it.next()) |value| {
for (typedefs.values()) |value| {
module.gpa.free(value.rendered);
}
}
Expand All @@ -135,8 +133,7 @@ pub fn updateDecl(self: *C, module: *Module, decl: *Module.Decl) !void {
object.blocks.deinit(module.gpa);
object.code.deinit();
object.dg.fwd_decl.deinit();
var it = object.dg.typedefs.valueIterator();
while (it.next()) |value| {
for (object.dg.typedefs.values()) |value| {
module.gpa.free(value.rendered);
}
object.dg.typedefs.deinit();
Expand Down Expand Up @@ -207,7 +204,7 @@ pub fn flushModule(self: *C, comp: *Compilation) !void {
}

var fn_count: usize = 0;
var typedefs = std.HashMap(Type, void, Type.HashContext, std.hash_map.default_max_load_percentage).init(comp.gpa);
var typedefs = std.HashMap(Type, void, Type.HashContext64, std.hash_map.default_max_load_percentage).init(comp.gpa);
defer typedefs.deinit();

// Typedefs, forward decls and non-functions first.
Expand All @@ -217,7 +214,7 @@ pub fn flushModule(self: *C, comp: *Compilation) !void {
if (!decl.has_tv) continue;
const buf = buf: {
if (decl.val.castTag(.function)) |_| {
try typedefs.ensureUnusedCapacity(decl.fn_link.c.typedefs.count());
try typedefs.ensureUnusedCapacity(@intCast(u32, decl.fn_link.c.typedefs.count()));
var it = decl.fn_link.c.typedefs.iterator();
while (it.next()) |new| {
const gop = typedefs.getOrPutAssumeCapacity(new.key_ptr.*);
Expand Down
13 changes: 12 additions & 1 deletion src/type.zig
Expand Up @@ -602,7 +602,7 @@ pub const Type = extern union {
return hasher.final();
}

pub const HashContext = struct {
pub const HashContext64 = struct {
pub fn hash(self: @This(), t: Type) u64 {
_ = self;
return t.hash();
Expand All @@ -613,6 +613,17 @@ pub const Type = extern union {
}
};

pub const HashContext32 = struct {
pub fn hash(self: @This(), t: Type) u32 {
_ = self;
return @truncate(u32, t.hash());
}
pub fn eql(self: @This(), a: Type, b: Type) bool {
_ = self;
return a.eql(b);
}
};

pub fn copy(self: Type, allocator: *Allocator) error{OutOfMemory}!Type {
if (self.tag_if_small_enough < Tag.no_payload_count) {
return Type{ .tag_if_small_enough = self.tag_if_small_enough };
Expand Down