Skip to content

Commit 346480b

Browse files
fix: add sort before hashing attributes
1 parent 69feb78 commit 346480b

File tree

2 files changed

+56
-6
lines changed

2 files changed

+56
-6
lines changed

src/attributes.zig

Lines changed: 37 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -118,14 +118,46 @@ pub const Attributes = struct {
118118
}
119119

120120
pub const HashContext = struct {
121-
pub fn hash(_: HashContext, self: Self) u64 {
121+
fn compareAttributes(context: void, a: Attribute, b: Attribute) bool {
122+
_ = context;
123+
return std.mem.lessThan(u8, a.key, b.key);
124+
}
125+
126+
pub fn hash(_: HashContext, self: Self) !u64 {
122127
var h = std.hash.Wyhash.init(0);
123128
const attrs = self.attributes orelse &[_]Attribute{};
124-
for (attrs) |attr| {
125-
h.update(attr.key);
126-
h.update(attr.value.toStringNoAlloc());
129+
130+
// If there are few attributes, just hash them directly to avoid allocation
131+
if (attrs.len <= 8) {
132+
var buffer: [8]Attribute = undefined;
133+
const to_sort = buffer[0..@min(attrs.len, 8)];
134+
@memcpy(to_sort, attrs[0..to_sort.len]);
135+
136+
if (to_sort.len > 1) {
137+
std.mem.sort(Attribute, to_sort, {}, compareAttributes);
138+
}
139+
140+
for (to_sort) |attr| {
141+
h.update(attr.key);
142+
h.update(attr.value.toStringNoAlloc());
143+
}
144+
return h.final();
145+
} else {
146+
var arena = std.heap.ArenaAllocator.init(std.heap.page_allocator);
147+
defer arena.deinit();
148+
149+
const allocator = arena.allocator();
150+
const sorted_attrs = try allocator.dupe(Attribute, attrs);
151+
if (sorted_attrs.len > 1) {
152+
std.mem.sort(Attribute, sorted_attrs, {}, compareAttributes);
153+
}
154+
155+
for (sorted_attrs) |attr| {
156+
h.update(attr.key);
157+
h.update(attr.value.toStringNoAlloc());
158+
}
159+
return h.final();
127160
}
128-
return h.final();
129161
}
130162
pub fn eql(_: HashContext, a: Self, b: Self) bool {
131163
const aAttrs = a.attributes orelse &[_]Attribute{};

src/scope.zig

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ pub const InstrumentationScope = struct {
1616
pub fn hash(_: HashContext, self: Self) u64 {
1717
const hashContext = Attributes.HashContext{};
1818

19-
const attributesHash = hashContext.hash(Attributes.with(self.attributes));
19+
const attributesHash = hashContext.hash(Attributes.with(self.attributes)) catch @panic("Failed to hash attributes");
2020

2121
var h = std.hash.Wyhash.init(0);
2222

@@ -91,3 +91,21 @@ test "InstrumentationScope should equal correctly" {
9191

9292
try std.testing.expect(hashContext.eql(first, second));
9393
}
94+
95+
test "InstrumentationScope hash should be consistent regardless of attribute order" {
96+
const allocator = std.testing.allocator;
97+
98+
const attrs1 = try Attributes.from(allocator, .{ @as([]const u8, "key1"), @as(u64, 42), @as([]const u8, "key2"), true, @as([]const u8, "key3"), @as([]const u8, "value3") });
99+
defer allocator.free(attrs1.?);
100+
101+
const attrs2 = try Attributes.from(allocator, .{ @as([]const u8, "key3"), @as([]const u8, "value3"), @as([]const u8, "key1"), @as(u64, 42), @as([]const u8, "key2"), true });
102+
defer allocator.free(attrs2.?);
103+
104+
const scope1: InstrumentationScope = .{ .name = "testScope", .attributes = attrs1 };
105+
const scope2: InstrumentationScope = .{ .name = "testScope", .attributes = attrs2 };
106+
107+
const hashContext = InstrumentationScope.HashContext{};
108+
109+
try std.testing.expectEqual(hashContext.hash(scope1), hashContext.hash(scope2));
110+
try std.testing.expect(hashContext.eql(scope1, scope2));
111+
}

0 commit comments

Comments
 (0)