Skip to content

Commit 02ffd8a

Browse files
committed
Fix a few bugs
1 parent 75e7ec9 commit 02ffd8a

File tree

9 files changed

+180
-39
lines changed

9 files changed

+180
-39
lines changed

AGENTS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ Priorities, in order:
3939
- `src/ordered/btree_map.zig`: `BTreeMap` (cache-friendly B-tree with configurable branching factor).
4040
- `src/ordered/skip_list_map.zig`: `SkipListMap` (probabilistic skip list with a per-instance PRNG).
4141
- `src/ordered/trie_map.zig`: `TrieMap` (prefix tree, specialised for `[]const u8` keys).
42-
- `src/ordered/cartesian_tree_map.zig`: `CartesianTreeMap` (treap combining BST ordering with max-heap priorities).
42+
- `src/ordered/cartesian_tree_map.zig`: `CartesianTreeMap` (treap combining BST ordering with max-heap priorities; takes an explicit key-comparison function).
4343
- `examples/`: Self-contained example programs (`e1_btree_map.zig` through `e6_cartesian_tree_map.zig`) built as executables via `build.zig`.
4444
- `benches/`: Benchmark programs (`b1_btree_map.zig` through `b6_cartesian_tree_map.zig`) built in `ReleaseFast`.
4545
- `benches/util/timer.zig`: Internal compatibility shim for the removed `std.time.Timer`, backed by `std.Io.Timestamp`.

README.md

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
[![Benchmarks](https://img.shields.io/github/actions/workflow/status/CogitatorTech/ordered/benches.yml?label=benchmarks&style=flat&labelColor=282c34&logo=github)](https://github.com/CogitatorTech/ordered/actions/workflows/benches.yml)
1111
[![Zig Version](https://img.shields.io/badge/Zig-0.16.0-orange?logo=zig&labelColor=282c34)](https://ziglang.org/download/)
1212
[![Release](https://img.shields.io/github/release/CogitatorTech/ordered.svg?label=release&style=flat&labelColor=282c34&logo=github)](https://github.com/CogitatorTech/ordered/releases/latest)
13-
<br>
1413
[![Docs](https://img.shields.io/badge/docs-read-blue?style=flat&labelColor=282c34&logo=read-the-docs)](https://CogitatorTech.github.io/ordered/)
1514
[![Examples](https://img.shields.io/badge/examples-view-green?style=flat&labelColor=282c34&logo=zig)](https://github.com/CogitatorTech/ordered/tree/main/examples)
1615
[![License](https://img.shields.io/badge/license-MIT-007ec6?label=license&style=flat&labelColor=282c34&logo=open-source-initiative)](https://github.com/CogitatorTech/ordered/blob/main/LICENSE)
@@ -77,7 +76,7 @@ Run the following command in the root directory of your project to download Orde
7776
zig fetch --save=ordered "https://github.com/CogitatorTech/ordered/archive/<branch_or_tag>.tar.gz"
7877
```
7978

80-
Replace `<branch_or_tag>` with the desired branch or release tag, like `main` (for the development version) or `v0.2.0`.
79+
Replace `<branch_or_tag>` with the desired branch or release tag, like `main` (for the development version) or `v0.3.0`.
8180
This command will download Ordered and add it to Zig's global cache and update your project's `build.zig.zon` file.
8281

8382
Zig version supported by each tagged release:

benches/b6_cartesian_tree_map.zig

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,10 @@ const std = @import("std");
22
const ordered = @import("ordered");
33
const Timer = @import("util/timer.zig").Timer;
44

5+
fn i32Compare(lhs: i32, rhs: i32) std.math.Order {
6+
return std.math.order(lhs, rhs);
7+
}
8+
59
pub fn main() !void {
610
var gpa = std.heap.DebugAllocator(.{}){};
711
defer _ = gpa.deinit();
@@ -21,7 +25,7 @@ pub fn main() !void {
2125
}
2226

2327
fn benchmarkPut(allocator: std.mem.Allocator, size: usize) !void {
24-
var tree = ordered.CartesianTreeMap(i32, i32).init(allocator);
28+
var tree = ordered.CartesianTreeMap(i32, i32, i32Compare).init(allocator);
2529
defer tree.deinit();
2630

2731
var timer = try Timer.start();
@@ -43,7 +47,7 @@ fn benchmarkPut(allocator: std.mem.Allocator, size: usize) !void {
4347
}
4448

4549
fn benchmarkGet(allocator: std.mem.Allocator, size: usize) !void {
46-
var tree = ordered.CartesianTreeMap(i32, i32).init(allocator);
50+
var tree = ordered.CartesianTreeMap(i32, i32, i32Compare).init(allocator);
4751
defer tree.deinit();
4852

4953
var i: i32 = 0;
@@ -72,7 +76,7 @@ fn benchmarkGet(allocator: std.mem.Allocator, size: usize) !void {
7276
}
7377

7478
fn benchmarkRemove(allocator: std.mem.Allocator, size: usize) !void {
75-
var tree = ordered.CartesianTreeMap(i32, i32).init(allocator);
79+
var tree = ordered.CartesianTreeMap(i32, i32, i32Compare).init(allocator);
7680
defer tree.deinit();
7781

7882
var i: i32 = 0;
@@ -99,7 +103,7 @@ fn benchmarkRemove(allocator: std.mem.Allocator, size: usize) !void {
99103
}
100104

101105
fn benchmarkIterator(allocator: std.mem.Allocator, size: usize) !void {
102-
var tree = ordered.CartesianTreeMap(i32, i32).init(allocator);
106+
var tree = ordered.CartesianTreeMap(i32, i32, i32Compare).init(allocator);
103107
defer tree.deinit();
104108

105109
var i: i32 = 0;

examples/e6_cartesian_tree_map.zig

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,15 @@
11
const std = @import("std");
22
const ordered = @import("ordered");
33

4+
fn i32Compare(lhs: i32, rhs: i32) std.math.Order {
5+
return std.math.order(lhs, rhs);
6+
}
7+
48
pub fn main() !void {
59
const allocator = std.heap.page_allocator;
610

711
std.debug.print("## CartesianTreeMap Example ##\n", .{});
8-
var cartesian_tree = ordered.CartesianTreeMap(i32, []const u8).init(allocator);
12+
var cartesian_tree = ordered.CartesianTreeMap(i32, []const u8, i32Compare).init(allocator);
913
defer cartesian_tree.deinit();
1014

1115
try cartesian_tree.put(50, "fifty");

src/ordered/btree_map.zig

Lines changed: 78 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,20 @@ pub fn BTreeMap(
4848
comptime compare: fn (lhs: K, rhs: K) std.math.Order,
4949
comptime BRANCHING_FACTOR: u16,
5050
) type {
51-
std.debug.assert(BRANCHING_FACTOR >= 3);
52-
const MIN_KEYS = (BRANCHING_FACTOR - 1) / 2;
51+
comptime {
52+
if (BRANCHING_FACTOR < 4) {
53+
@compileError("BTreeMap: BRANCHING_FACTOR must be at least 4");
54+
}
55+
}
56+
// Minimum keys per non-root node.
57+
//
58+
// We pick this so that `2 * MIN_KEYS + 1` (the size of a merge of two
59+
// MIN_KEYS siblings plus the pulled-down separator) always fits in the
60+
// `BRANCHING_FACTOR - 1`-element key array. For even B this simplifies
61+
// to `(B - 1) / 2` — identical to the common textbook formula. For odd
62+
// B it relaxes MIN_KEYS by one compared to the textbook, which avoids
63+
// an over-full node after merge at the cost of slightly looser packing.
64+
const MIN_KEYS = (BRANCHING_FACTOR - 2) / 2;
5365

5466
return struct {
5567
const Self = @This();
@@ -529,6 +541,11 @@ pub fn BTreeMap(
529541

530542
fn init(allocator: std.mem.Allocator, root: ?*Node) !Iterator {
531543
var stack: std.ArrayList(StackFrame) = .empty;
544+
// If any append below fails after the first, the local
545+
// `stack` goes out of scope without being moved into the
546+
// returned Iterator; its heap buffer would leak without
547+
// this errdefer.
548+
errdefer stack.deinit(allocator);
532549

533550
if (root) |r| {
534551
try stack.append(allocator, StackFrame{ .node = r, .index = 0 });
@@ -783,3 +800,62 @@ test "BTreeMap: negative keys" {
783800
try std.testing.expectEqual(@as(i32, 10), map.get(-10).?.*);
784801
try std.testing.expectEqual(@as(i32, -5), map.get(5).?.*);
785802
}
803+
804+
test "regression: BTreeMap deletion works with odd BRANCHING_FACTOR" {
805+
// Bug B1: the old `MIN_KEYS = (B-1)/2` formula combined with the split
806+
// layout left odd-B right siblings underfull; a subsequent deletion that
807+
// merged two MIN_KEYS siblings plus a separator overflowed the
808+
// `[B-1]K` keys array, panicking at merge time. This test exercises
809+
// the exact path that used to crash for B=5.
810+
const allocator = std.testing.allocator;
811+
var map = BTreeMap(i32, i32, i32Compare, 5).init(allocator);
812+
defer map.deinit();
813+
814+
// Fill, split, then fill again so both halves are at MIN_KEYS.
815+
try map.put(0, 0);
816+
try map.put(1, 1);
817+
try map.put(2, 2);
818+
try map.put(3, 3);
819+
try map.put(4, 4);
820+
try std.testing.expectEqual(@as(usize, 5), map.count());
821+
822+
// Delete from the right side: this forces the merge path.
823+
try std.testing.expectEqual(@as(i32, 4), map.remove(4).?);
824+
try std.testing.expectEqual(@as(usize, 4), map.count());
825+
826+
// Verify the remaining tree is intact and ordered.
827+
try std.testing.expectEqual(@as(i32, 0), map.get(0).?.*);
828+
try std.testing.expectEqual(@as(i32, 1), map.get(1).?.*);
829+
try std.testing.expectEqual(@as(i32, 2), map.get(2).?.*);
830+
try std.testing.expectEqual(@as(i32, 3), map.get(3).?.*);
831+
try std.testing.expect(map.get(4) == null);
832+
}
833+
834+
test "regression: BTreeMap sequential delete with odd B stays valid" {
835+
// Larger stress: insert 200 items with B=7 (odd, previously broken),
836+
// delete half in random-ish order, then confirm the remainder is still
837+
// accessible and ordered.
838+
const allocator = std.testing.allocator;
839+
var map = BTreeMap(i32, i32, i32Compare, 7).init(allocator);
840+
defer map.deinit();
841+
842+
var i: i32 = 0;
843+
while (i < 200) : (i += 1) {
844+
try map.put(i, i * 2);
845+
}
846+
try std.testing.expectEqual(@as(usize, 200), map.count());
847+
848+
// Delete every other item in ascending order.
849+
i = 0;
850+
while (i < 200) : (i += 2) {
851+
const v = map.remove(i);
852+
try std.testing.expectEqual(@as(i32, i * 2), v.?);
853+
}
854+
try std.testing.expectEqual(@as(usize, 100), map.count());
855+
856+
// Remaining items must still be reachable with correct values.
857+
i = 1;
858+
while (i < 200) : (i += 2) {
859+
try std.testing.expectEqual(@as(i32, i * 2), map.get(i).?.*);
860+
}
861+
}

0 commit comments

Comments
 (0)