From 582f16c116e5b515920a21950551721389a0af3d Mon Sep 17 00:00:00 2001 From: Tetralux <1348560+Tetralux@users.noreply.github.com> Date: Fri, 9 Aug 2019 19:05:08 +0000 Subject: [PATCH 01/12] Add ability to remove elements, by swapRemove or orderedRemove, while iterating std.ArrayList. --- std/array_list.zig | 172 ++++++++++++++++++++++++++++++++++++++++--- std/http/headers.zig | 12 +-- 2 files changed, 169 insertions(+), 15 deletions(-) diff --git a/std/array_list.zig b/std/array_list.zig index 31ae02b2910c..7e29bf8c4d6d 100644 --- a/std/array_list.zig +++ b/std/array_list.zig @@ -1,4 +1,5 @@ const std = @import("std.zig"); +const builtin = @import("builtin"); const debug = std.debug; const assert = debug.assert; const testing = std.testing; @@ -201,27 +202,90 @@ pub fn AlignedArrayList(comptime T: type, comptime alignment: ?u29) type { return self.pop(); } + // an iterator which allows you to remove the current element + // while iterating through it. pub const Iterator = struct { - list: *const Self, - // how many items have we returned - count: usize, + list: *Self, + + // index of the next element to return + next_index: usize = 0, + + // did we remove the element last returned by `next`? + removed: bool = false, pub fn next(it: *Iterator) ?T { - if (it.count >= it.list.len) return null; - const val = it.list.at(it.count); - it.count += 1; + // if we removed the previous element, then + // the next element replaced the current one, + // so we get the element at the same index a second time. + // the next element's index was incremented when we + // called `next` before, so we decrement it here, before we get the item. + const index = if (it.removed) it.next_index-1 else it.next_index; + it.removed = false; + + if (index >= it.list.len) return null; + const val = it.list.at(index); + + it.next_index = index+1; + return val; } pub fn reset(it: *Iterator) void { - it.count = 0; + it.next_index = 0; + it.removed = false; + } + + pub fn swapRemove(it: *Iterator) T { + if (!it.removed) { + it.removed = true; + return it.list.swapRemove(it.next_index-1); + } else switch (builtin.mode) { + .ReleaseFast => unreachable, + else => @panic("removed element more than once"), + } + } + + pub fn orderedRemove(it: *Iterator) T { + if (!it.removed) { + it.removed = true; + return it.list.orderedRemove(it.next_index-1); + } else switch (builtin.mode) { + .ReleaseFast => unreachable, + else => @panic("removed element more than once"), + } } }; - pub fn iterator(self: *const Self) Iterator { + pub fn iterator(self: *Self) Iterator { return Iterator{ .list = self, - .count = 0, + }; + } + + // an iterator that only allows you to iterate + // through the elements; you cannot remove the current + // element. + pub const IteratorConst = struct { + list: *const Self, + + // index of the next element to return + next_index: usize = 0, + + pub fn next(it: *IteratorConst) ?T { + if (it.next_index >= it.list.len) return null; + const val = it.list.at(it.next_index); + it.next_index += 1; + return val; + } + + pub fn reset(it: *IteratorConst) void { + it.next_index = 0; + } + }; + + pub fn iteratorConst(self: *const Self) IteratorConst { + return IteratorConst{ + .list = self, }; } }; @@ -409,6 +473,96 @@ test "std.ArrayList.iterator" { testing.expect(it.next().? == 1); } +test "std.ArrayList.iterator.swapRemove" { + var bytes: [1024]u8 = undefined; + const allocator = &std.heap.FixedBufferAllocator.init(bytes[0..]).allocator; + + var list = ArrayList(i32).init(allocator); + defer list.deinit(); + + try list.append(1); + try list.append(2); + try list.append(3); + try list.append(4); + + var it = list.iterator(); + while (it.next()) |next| { + if (next == 2) { + var removed = it.swapRemove(); + testing.expect(removed == 2); + break; + } + } + + it.reset(); + testing.expect(it.next().? == 1); + testing.expect(it.next().? == 4); + testing.expect(it.next().? == 3); + testing.expect(it.next() == null); +} + +test "std.ArrayList.iterator.orderedRemove" { + var bytes: [1024]u8 = undefined; + const allocator = &std.heap.FixedBufferAllocator.init(bytes[0..]).allocator; + + var list = ArrayList(i32).init(allocator); + defer list.deinit(); + + try list.append(1); + try list.append(2); + try list.append(3); + try list.append(4); + + var it = list.iterator(); + while (it.next()) |next| { + if (next == 2) { + var removed = it.orderedRemove(); + testing.expect(removed == 2); + break; + } + } + + it.reset(); + testing.expect(it.next().? == 1); + testing.expect(it.next().? == 3); + testing.expect(it.next().? == 4); + testing.expect(it.next() == null); +} + +test "std.ArrayList.iteratorConst" { + var bytes: [1024]u8 = undefined; + const allocator = &std.heap.FixedBufferAllocator.init(bytes[0..]).allocator; + + var list = ArrayList(i32).init(allocator); + defer list.deinit(); + + try list.append(1); + try list.append(2); + try list.append(3); + + const clist = list; + + var count: i32 = 0; + var it = clist.iteratorConst(); + while (it.next()) |next| { + testing.expect(next == count + 1); + count += 1; + } + + testing.expect(count == 3); + testing.expect(it.next() == null); + it.reset(); + count = 0; + while (it.next()) |next| { + testing.expect(next == count + 1); + count += 1; + if (count == 2) break; + } + + it.reset(); + testing.expect(it.next().? == 1); +} + test "std.ArrayList.insert" { var list = ArrayList(i32).init(debug.global_allocator); defer list.deinit(); diff --git a/std/http/headers.zig b/std/http/headers.zig index a8dfa686298f..a627336e6f56 100644 --- a/std/http/headers.zig +++ b/std/http/headers.zig @@ -133,7 +133,7 @@ pub const Headers = struct { self.index.deinit(); } { - var it = self.data.iterator(); + var it = self.data.iteratorConst(); while (it.next()) |entry| { entry.deinit(); } @@ -146,7 +146,7 @@ pub const Headers = struct { errdefer other.deinit(); try other.data.ensureCapacity(self.data.count()); try other.index.initCapacity(self.index.entries.len); - var it = self.data.iterator(); + var it = self.data.iteratorConst(); while (it.next()) |entry| { try other.append(entry.name, entry.value, entry.never_index); } @@ -160,7 +160,7 @@ pub const Headers = struct { pub const Iterator = HeaderList.Iterator; pub fn iterator(self: Self) Iterator { - return self.data.iterator(); + return self.data.iteratorConst(); } pub fn append(self: *Self, name: []const u8, value: []const u8, never_index: ?bool) !void { @@ -290,7 +290,7 @@ pub const Headers = struct { const dex = self.getIndices(name) orelse return null; const buf = try allocator.alloc(HeaderEntry, dex.count()); - var it = dex.iterator(); + var it = dex.iteratorConst(); var n: usize = 0; while (it.next()) |idx| { buf[n] = self.data.at(idx); @@ -315,7 +315,7 @@ pub const Headers = struct { // adapted from mem.join const total_len = blk: { var sum: usize = dex.count() - 1; // space for separator(s) - var it = dex.iterator(); + var it = dex.iteratorConst(); while (it.next()) |idx| sum += self.data.at(idx).value.len; break :blk sum; @@ -351,7 +351,7 @@ pub const Headers = struct { var it = self.data.iterator(); while (it.next()) |entry| { var dex = &self.index.get(entry.name).?.value; - dex.appendAssumeCapacity(it.count); + dex.appendAssumeCapacity(it.next_index); } } } From 36e8c7db9a49f4a771213a91e013070f5bd15af0 Mon Sep 17 00:00:00 2001 From: Tetralux <1348560+Tetralux@users.noreply.github.com> Date: Sat, 10 Aug 2019 15:54:53 +0000 Subject: [PATCH 02/12] Fix typo that should have been a compile error?.. --- std/http/headers.zig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/std/http/headers.zig b/std/http/headers.zig index a627336e6f56..7f0aae74b5b3 100644 --- a/std/http/headers.zig +++ b/std/http/headers.zig @@ -160,7 +160,7 @@ pub const Headers = struct { pub const Iterator = HeaderList.Iterator; pub fn iterator(self: Self) Iterator { - return self.data.iteratorConst(); + return self.data.iterator(); } pub fn append(self: *Self, name: []const u8, value: []const u8, never_index: ?bool) !void { From e10379dc7d5a1f0020b78e6ad420f3a04967ab8e Mon Sep 17 00:00:00 2001 From: Tetralux <1348560+Tetralux@users.noreply.github.com> Date: Sat, 10 Aug 2019 16:47:12 +0000 Subject: [PATCH 03/12] _Actually_ fix it this time? --- std/http/headers.zig | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/std/http/headers.zig b/std/http/headers.zig index 7f0aae74b5b3..0cb7aa33e96c 100644 --- a/std/http/headers.zig +++ b/std/http/headers.zig @@ -157,10 +157,10 @@ pub const Headers = struct { return self.data.count(); } - pub const Iterator = HeaderList.Iterator; + pub const Iterator = HeaderList.IteratorConst; pub fn iterator(self: Self) Iterator { - return self.data.iterator(); + return self.data.iteratorConst(); } pub fn append(self: *Self, name: []const u8, value: []const u8, never_index: ?bool) !void { From d2369be2e8403482865c961676632d2565b689b6 Mon Sep 17 00:00:00 2001 From: Tetralux <1348560+Tetralux@users.noreply.github.com> Date: Mon, 12 Aug 2019 00:20:29 +0000 Subject: [PATCH 04/12] Make it ~5% faster --- std/array_list.zig | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/std/array_list.zig b/std/array_list.zig index 7e29bf8c4d6d..9e5ea46018a6 100644 --- a/std/array_list.zig +++ b/std/array_list.zig @@ -219,20 +219,23 @@ pub fn AlignedArrayList(comptime T: type, comptime alignment: ?u29) type { // so we get the element at the same index a second time. // the next element's index was incremented when we // called `next` before, so we decrement it here, before we get the item. - const index = if (it.removed) it.next_index-1 else it.next_index; - it.removed = false; + // NOTE: volatile code! don't use a ternary if, or stack variable for new index here! + if (it.removed) { + it.removed = false; + it.next_index -= 1; + } - if (index >= it.list.len) return null; - const val = it.list.at(index); + if (it.next_index >= it.list.len) return null; + const val = it.list.at(it.next_index); - it.next_index = index+1; + it.next_index += 1; return val; } pub fn reset(it: *Iterator) void { it.next_index = 0; - it.removed = false; + it.removed = true; } pub fn swapRemove(it: *Iterator) T { From 75256ee4a70b46e1f419db9308c95b65cbea5fa5 Mon Sep 17 00:00:00 2001 From: Tetralux <1348560+Tetralux@users.noreply.github.com> Date: Mon, 12 Aug 2019 12:24:27 +0000 Subject: [PATCH 05/12] Fix a typo, and fix bug with removing the first element Add tests for removing the first element --- std/array_list.zig | 37 ++++++++++++++++++++++++------------- 1 file changed, 24 insertions(+), 13 deletions(-) diff --git a/std/array_list.zig b/std/array_list.zig index 9e5ea46018a6..d7aa06537c5c 100644 --- a/std/array_list.zig +++ b/std/array_list.zig @@ -214,16 +214,8 @@ pub fn AlignedArrayList(comptime T: type, comptime alignment: ?u29) type { removed: bool = false, pub fn next(it: *Iterator) ?T { - // if we removed the previous element, then - // the next element replaced the current one, - // so we get the element at the same index a second time. - // the next element's index was incremented when we - // called `next` before, so we decrement it here, before we get the item. - // NOTE: volatile code! don't use a ternary if, or stack variable for new index here! - if (it.removed) { - it.removed = false; - it.next_index -= 1; - } + // NOTE: volatile code! this is actually slower if you remove the branch!!! + if (it.removed) it.removed = false; if (it.next_index >= it.list.len) return null; const val = it.list.at(it.next_index); @@ -235,13 +227,14 @@ pub fn AlignedArrayList(comptime T: type, comptime alignment: ?u29) type { pub fn reset(it: *Iterator) void { it.next_index = 0; - it.removed = true; + it.removed = false; } pub fn swapRemove(it: *Iterator) T { if (!it.removed) { it.removed = true; - return it.list.swapRemove(it.next_index-1); + if (it.next_index > 0) it.next_index -= 1; + return it.list.swapRemove(it.next_index); } else switch (builtin.mode) { .ReleaseFast => unreachable, else => @panic("removed element more than once"), @@ -251,7 +244,8 @@ pub fn AlignedArrayList(comptime T: type, comptime alignment: ?u29) type { pub fn orderedRemove(it: *Iterator) T { if (!it.removed) { it.removed = true; - return it.list.orderedRemove(it.next_index-1); + if (it.next_index > 0) it.next_index -= 1; + return it.list.orderedRemove(it.next_index); } else switch (builtin.mode) { .ReleaseFast => unreachable, else => @panic("removed element more than once"), @@ -488,6 +482,14 @@ test "std.ArrayList.iterator.swapRemove" { try list.append(3); try list.append(4); + { + var last_elem = list.at(list.len-1); + var itr = list.iterator(); + _ = itr.swapRemove(); + testing.expectEqual(list.at(0), last_elem); + try list.insert(0, 1); // put the number back in. + } + var it = list.iterator(); while (it.next()) |next| { if (next == 2) { @@ -516,6 +518,15 @@ test "std.ArrayList.iterator.orderedRemove" { try list.append(3); try list.append(4); + // check we can remove the first element + { + var second_elem = list.at(1); + var itr = list.iterator(); + _ = itr.orderedRemove(); + testing.expectEqual(list.at(0), second_elem); + try list.insert(0, 1); // put the number back in. + } + var it = list.iterator(); while (it.next()) |next| { if (next == 2) { From 392ec68ca3a01a21faef2f8b9c4604bb02b4aff7 Mon Sep 17 00:00:00 2001 From: Tetralux <1348560+Tetralux@users.noreply.github.com> Date: Mon, 12 Aug 2019 14:17:13 +0000 Subject: [PATCH 06/12] Match old performance when not removing; get removal procedures within 1% of normal removal procedures. --- std/array_list.zig | 42 ++++++++++++++++++++++++++++++------------ 1 file changed, 30 insertions(+), 12 deletions(-) diff --git a/std/array_list.zig b/std/array_list.zig index d7aa06537c5c..579465d9b203 100644 --- a/std/array_list.zig +++ b/std/array_list.zig @@ -210,13 +210,10 @@ pub fn AlignedArrayList(comptime T: type, comptime alignment: ?u29) type { // index of the next element to return next_index: usize = 0, - // did we remove the element last returned by `next`? - removed: bool = false, + // the last index we removed + removed_index: ?usize = null, pub fn next(it: *Iterator) ?T { - // NOTE: volatile code! this is actually slower if you remove the branch!!! - if (it.removed) it.removed = false; - if (it.next_index >= it.list.len) return null; const val = it.list.at(it.next_index); @@ -227,13 +224,26 @@ pub fn AlignedArrayList(comptime T: type, comptime alignment: ?u29) type { pub fn reset(it: *Iterator) void { it.next_index = 0; - it.removed = false; + it.removed_index = null; } - pub fn swapRemove(it: *Iterator) T { - if (!it.removed) { - it.removed = true; + // + // NOTE: inlining these removal routines is important. + // Allows them to get with 1% of the performance of ArrayList.swapRemove/orderedRemove. + // + + pub inline fn swapRemove(it: *Iterator) T { + const removed_already = blk: { + if (it.removed_index) |idx| { + break :blk idx==it.next_index; + } else { + break :blk false; + } + }; + if (!removed_already) { if (it.next_index > 0) it.next_index -= 1; + it.removed_index = it.next_index; + if (it.next_index >= it.list.len) @panic("attempt to remove element from empty list"); return it.list.swapRemove(it.next_index); } else switch (builtin.mode) { .ReleaseFast => unreachable, @@ -241,10 +251,18 @@ pub fn AlignedArrayList(comptime T: type, comptime alignment: ?u29) type { } } - pub fn orderedRemove(it: *Iterator) T { - if (!it.removed) { - it.removed = true; + pub inline fn orderedRemove(it: *Iterator) T { + const removed_already = blk: { + if (it.removed_index) |idx| { + break :blk idx==it.next_index; + } else { + break :blk false; + } + }; + if (!removed_already) { if (it.next_index > 0) it.next_index -= 1; + if (it.next_index >= it.list.len) @panic("attempt to remove element from empty list"); + it.removed_index = it.next_index; return it.list.orderedRemove(it.next_index); } else switch (builtin.mode) { .ReleaseFast => unreachable, From 29db8632de0fceade5a8904c247fd8f955ffab8c Mon Sep 17 00:00:00 2001 From: Tetralux <1348560+Tetralux@users.noreply.github.com> Date: Tue, 13 Aug 2019 20:07:34 +0000 Subject: [PATCH 07/12] Cleanup --- std/array_list.zig | 28 ++++++++++------------------ 1 file changed, 10 insertions(+), 18 deletions(-) diff --git a/std/array_list.zig b/std/array_list.zig index 579465d9b203..f5274d9d1b79 100644 --- a/std/array_list.zig +++ b/std/array_list.zig @@ -233,17 +233,13 @@ pub fn AlignedArrayList(comptime T: type, comptime alignment: ?u29) type { // pub inline fn swapRemove(it: *Iterator) T { - const removed_already = blk: { - if (it.removed_index) |idx| { - break :blk idx==it.next_index; - } else { - break :blk false; - } - }; - if (!removed_already) { + if (it.list.len == 0) @panic("attempt to remove element from empty list"); + + if (it.removed_index == null or + it.removed_index.? != it.next_index) + { if (it.next_index > 0) it.next_index -= 1; it.removed_index = it.next_index; - if (it.next_index >= it.list.len) @panic("attempt to remove element from empty list"); return it.list.swapRemove(it.next_index); } else switch (builtin.mode) { .ReleaseFast => unreachable, @@ -252,16 +248,12 @@ pub fn AlignedArrayList(comptime T: type, comptime alignment: ?u29) type { } pub inline fn orderedRemove(it: *Iterator) T { - const removed_already = blk: { - if (it.removed_index) |idx| { - break :blk idx==it.next_index; - } else { - break :blk false; - } - }; - if (!removed_already) { + if (it.list.len == 0) @panic("attempt to remove element from empty list"); + + if (it.removed_index == null or + it.removed_index.? != it.next_index) + { if (it.next_index > 0) it.next_index -= 1; - if (it.next_index >= it.list.len) @panic("attempt to remove element from empty list"); it.removed_index = it.next_index; return it.list.orderedRemove(it.next_index); } else switch (builtin.mode) { From 33f17b96cf7310419b40e0ce0a40ca4b78a0cea7 Mon Sep 17 00:00:00 2001 From: Tetralux <1348560+Tetralux@users.noreply.github.com> Date: Wed, 14 Aug 2019 16:15:03 +0000 Subject: [PATCH 08/12] Fix comment; and retry worker job in the process --- std/array_list.zig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/std/array_list.zig b/std/array_list.zig index f5274d9d1b79..3f086f4074f6 100644 --- a/std/array_list.zig +++ b/std/array_list.zig @@ -229,7 +229,7 @@ pub fn AlignedArrayList(comptime T: type, comptime alignment: ?u29) type { // // NOTE: inlining these removal routines is important. - // Allows them to get with 1% of the performance of ArrayList.swapRemove/orderedRemove. + // It gains ~5% in debug mode, and ~8% in release-safe; within 1% of the performance of ArrayList.swapRemove/orderedRemove. // pub inline fn swapRemove(it: *Iterator) T { From 1acafdd513c764e943c2c03c44806a8d7f905aee Mon Sep 17 00:00:00 2001 From: Tetralux <1348560+Tetralux@users.noreply.github.com> Date: Fri, 16 Aug 2019 16:40:04 +0000 Subject: [PATCH 09/12] Make it illegal to call remove without calling next This makes removal procedures alwayts remove the last element returned from next. Before, they'd remove the first element if you called for removal before any calls to next. --- std/array_list.zig | 46 +++++++++++++++++++++++----------------------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/std/array_list.zig b/std/array_list.zig index 3f086f4074f6..b6198fe1ab52 100644 --- a/std/array_list.zig +++ b/std/array_list.zig @@ -207,23 +207,25 @@ pub fn AlignedArrayList(comptime T: type, comptime alignment: ?u29) type { pub const Iterator = struct { list: *Self, - // index of the next element to return - next_index: usize = 0, + // index of the last element we returned, or null + // if we haven't called `next` yet. + cursor: ?usize = null, // the last index we removed removed_index: ?usize = null, pub fn next(it: *Iterator) ?T { - if (it.next_index >= it.list.len) return null; - const val = it.list.at(it.next_index); - - it.next_index += 1; - - return val; + var idx: usize = 0; + if (it.cursor) |cursor| { + idx = cursor + 1; + } + if (idx >= it.list.len) return null; + it.cursor = idx; + return it.list.at(idx); } pub fn reset(it: *Iterator) void { - it.next_index = 0; + it.cursor = null; it.removed_index = null; } @@ -233,14 +235,12 @@ pub fn AlignedArrayList(comptime T: type, comptime alignment: ?u29) type { // pub inline fn swapRemove(it: *Iterator) T { - if (it.list.len == 0) @panic("attempt to remove element from empty list"); - + const cursor = it.cursor orelse @panic("must call next at least once"); if (it.removed_index == null or - it.removed_index.? != it.next_index) - { - if (it.next_index > 0) it.next_index -= 1; - it.removed_index = it.next_index; - return it.list.swapRemove(it.next_index); + it.removed_index.? != cursor + ) { + it.removed_index = cursor; + return it.list.swapRemove(cursor); } else switch (builtin.mode) { .ReleaseFast => unreachable, else => @panic("removed element more than once"), @@ -248,14 +248,12 @@ pub fn AlignedArrayList(comptime T: type, comptime alignment: ?u29) type { } pub inline fn orderedRemove(it: *Iterator) T { - if (it.list.len == 0) @panic("attempt to remove element from empty list"); - + const cursor = it.cursor orelse @panic("must call next at least once"); if (it.removed_index == null or - it.removed_index.? != it.next_index) - { - if (it.next_index > 0) it.next_index -= 1; - it.removed_index = it.next_index; - return it.list.orderedRemove(it.next_index); + it.removed_index.? != cursor + ) { + it.removed_index = cursor; + return it.list.orderedRemove(cursor); } else switch (builtin.mode) { .ReleaseFast => unreachable, else => @panic("removed element more than once"), @@ -495,6 +493,7 @@ test "std.ArrayList.iterator.swapRemove" { { var last_elem = list.at(list.len-1); var itr = list.iterator(); + _ = itr.next(); _ = itr.swapRemove(); testing.expectEqual(list.at(0), last_elem); try list.insert(0, 1); // put the number back in. @@ -532,6 +531,7 @@ test "std.ArrayList.iterator.orderedRemove" { { var second_elem = list.at(1); var itr = list.iterator(); + _ = itr.next(); _ = itr.orderedRemove(); testing.expectEqual(list.at(0), second_elem); try list.insert(0, 1); // put the number back in. From c74040976b6ce5250a927819e22e7b61b41cf702 Mon Sep 17 00:00:00 2001 From: Tetralux <1348560+Tetralux@users.noreply.github.com> Date: Fri, 16 Aug 2019 22:37:06 +0000 Subject: [PATCH 10/12] Seems reasonable --- std/http/headers.zig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/std/http/headers.zig b/std/http/headers.zig index 0cb7aa33e96c..d2ba2c3cbd75 100644 --- a/std/http/headers.zig +++ b/std/http/headers.zig @@ -351,7 +351,7 @@ pub const Headers = struct { var it = self.data.iterator(); while (it.next()) |entry| { var dex = &self.index.get(entry.name).?.value; - dex.appendAssumeCapacity(it.next_index); + dex.appendAssumeCapacity(it.cursor.?+1); } } } From 5195781d5e7b9dbc7427c26c2858c597624e0170 Mon Sep 17 00:00:00 2001 From: Tetralux <1348560+Tetralux@users.noreply.github.com> Date: Sat, 17 Aug 2019 16:57:33 +0000 Subject: [PATCH 11/12] Add comment --- std/array_list.zig | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/std/array_list.zig b/std/array_list.zig index b6198fe1ab52..3f3028d63027 100644 --- a/std/array_list.zig +++ b/std/array_list.zig @@ -233,6 +233,10 @@ pub fn AlignedArrayList(comptime T: type, comptime alignment: ?u29) type { // NOTE: inlining these removal routines is important. // It gains ~5% in debug mode, and ~8% in release-safe; within 1% of the performance of ArrayList.swapRemove/orderedRemove. // + // Also note that you'd think that just checking for the index==0 in the removal routines would be + // good for making it illegal if next has not been called yet. + // But no, it is slower to do this for release-safe, and debug modes. + // pub inline fn swapRemove(it: *Iterator) T { const cursor = it.cursor orelse @panic("must call next at least once"); From ddccf82eed5cdd5eb6be008c979c9e6490723a7b Mon Sep 17 00:00:00 2001 From: Tetralux Date: Fri, 20 Sep 2019 16:40:39 +0000 Subject: [PATCH 12/12] Refactor --- std/array_list.zig | 41 ++++++++++------------------------------- 1 file changed, 10 insertions(+), 31 deletions(-) diff --git a/std/array_list.zig b/std/array_list.zig index 3f3028d63027..f8e1bec2b37f 100644 --- a/std/array_list.zig +++ b/std/array_list.zig @@ -229,39 +229,18 @@ pub fn AlignedArrayList(comptime T: type, comptime alignment: ?u29) type { it.removed_index = null; } - // - // NOTE: inlining these removal routines is important. - // It gains ~5% in debug mode, and ~8% in release-safe; within 1% of the performance of ArrayList.swapRemove/orderedRemove. - // - // Also note that you'd think that just checking for the index==0 in the removal routines would be - // good for making it illegal if next has not been called yet. - // But no, it is slower to do this for release-safe, and debug modes. - // - - pub inline fn swapRemove(it: *Iterator) T { - const cursor = it.cursor orelse @panic("must call next at least once"); - if (it.removed_index == null or - it.removed_index.? != cursor - ) { - it.removed_index = cursor; - return it.list.swapRemove(cursor); - } else switch (builtin.mode) { - .ReleaseFast => unreachable, - else => @panic("removed element more than once"), - } + pub fn swapRemove(it: *Iterator) T { + var cursor = it.cursor.?; // must call .next() at least once + if (it.removed_index) |ri| assert(ri != cursor); // removed the same element more than once + it.removed_index = cursor; + return it.list.swapRemove(cursor); } - pub inline fn orderedRemove(it: *Iterator) T { - const cursor = it.cursor orelse @panic("must call next at least once"); - if (it.removed_index == null or - it.removed_index.? != cursor - ) { - it.removed_index = cursor; - return it.list.orderedRemove(cursor); - } else switch (builtin.mode) { - .ReleaseFast => unreachable, - else => @panic("removed element more than once"), - } + pub fn orderedRemove(it: *Iterator) T { + var cursor = it.cursor.?; // must call .next() at least once + if (it.removed_index) |ri| assert(ri != cursor); // removed the same element more than once + it.removed_index = cursor; + return it.list.orderedRemove(cursor); } };