LSM: Revert groove to use update + insert and vendor hasUniqueRepresentation

src/lsm/groove.zig

@@ -863,10 +863,12 @@ pub fn GrooveType(
             }
         };
 
-        /// Insert the value into the objects tree and associated index trees, asserting that it doesn't
-        /// already exist.
+        /// Insert the value into the objects tree and associated index trees. It's up to the
+        /// caller to ensure it doesn't already exist.
         pub fn insert(groove: *Groove, object: *const Object) void {
-            assert(!groove.objects_cache.has(@field(object, primary_field)));
+            if (constants.verify) {
+                assert(!groove.objects_cache.has(@field(object, primary_field)));
+            }
 
             groove.objects_cache.upsert(object);
 
@@ -886,37 +888,55 @@ pub fn GrooveType(
             }
         }
 
-        /// Insert the value (or update it, if it exists).
+        /// Update the value. Requires the old object to be provided.
         /// Update the object and index trees by diff'ing the old and new values.
-        pub fn upsert(groove: *Groove, new: *const Object) void {
-            // Explicit stack copy needed - otherwise old will == new!
-            const old: Object = (groove.objects_cache.get(@field(new, primary_field)) orelse {
-                return groove.insert(new);
-            }).*;
+        pub fn update(
+            groove: *Groove,
+            values: struct { old: *const Object, new: *const Object },
+        ) void {
+            const old = values.old;
+            const new = values.new;
 
-            assert(@field(old, primary_field) == @field(new, primary_field));
-            assert(old.timestamp == new.timestamp);
+            if (constants.verify) {
+                const old_from_cache = groove.objects_cache.get(@field(old, primary_field)).?;
+                assert(@field(old_from_cache, primary_field) == @field(old, primary_field));
+                assert(old_from_cache.timestamp == old.timestamp);
+            }
 
-            groove.objects_cache.upsert(new);
+            // Sanity check to ensure the caller didn't accidentally pass in an alias.
+            assert(new != old);
 
-            // The ID can't change, so no need to update the ID tree.
+            if (has_id) {
+                assert(old.id == new.id);
+            }
+            assert(old.timestamp == new.timestamp);
 
-            // Update the object tree entry if any of the fields (even ignored) are different.
-            if (!std.mem.eql(u8, std.mem.asBytes(&old), std.mem.asBytes(new))) {
-                // Unlike the index trees, the new and old values in the object tree share the
-                // same key. Therefore put() is sufficient to overwrite the old value.
-                groove.objects.put(new);
+            // The ID can't change, so no need to update the ID tree. Update the object tree entry
+            // if any of the fields (even ignored) are different. We assume the caller will pass in
+            // an object that has changes.
+            // Unlike the index trees, the new and old values in the object tree share the same
+            // key. Therefore put() is sufficient to overwrite the old value.
+            if (constants.verify) {
+                const tombstone = ObjectTreeHelpers(Object).tombstone;
+                const key_from_value = ObjectTreeHelpers(Object).key_from_value;
+
+                assert(!stdx.equal_bytes(Object, old, new));
+                assert(key_from_value(old) == key_from_value(new));
+                assert(!tombstone(old) and !tombstone(new));
             }
 
+            groove.objects.put(new);
+            groove.objects_cache.upsert(new);
+
             inline for (std.meta.fields(IndexTrees)) |field| {
                 const Helper = IndexTreeFieldHelperType(field.name);
-                const old_index = Helper.derive_index(&old);
+                const old_index = Helper.derive_index(old);
                 const new_index = Helper.derive_index(new);
 
                 // Only update the indexes that change.
                 if (!std.meta.eql(old_index, new_index)) {
                     if (old_index) |index| {
-                        const old_index_value = Helper.derive_value(&old, index);
+                        const old_index_value = Helper.derive_value(old, index);
                         @field(groove.indexes, field.name).remove(&old_index_value);
                     }
 

src/state_machine.zig

@@ -876,8 +876,8 @@ pub fn StateMachineType(
                 dr_account_new.debits_posted += amount;
                 cr_account_new.credits_posted += amount;
             }
-            self.forest.grooves.accounts.upsert(&dr_account_new);
-            self.forest.grooves.accounts.upsert(&cr_account_new);
+            self.forest.grooves.accounts.update(.{ .old = dr_account, .new = &dr_account_new });
+            self.forest.grooves.accounts.update(.{ .old = cr_account, .new = &cr_account_new });
 
             self.commit_timestamp = t.timestamp;
             return .ok;
@@ -1006,8 +1006,8 @@ pub fn StateMachineType(
                 cr_account_new.credits_posted += amount;
             }
 
-            self.forest.grooves.accounts.upsert(&dr_account_new);
-            self.forest.grooves.accounts.upsert(&cr_account_new);
+            self.forest.grooves.accounts.update(.{ .old = dr_account, .new = &dr_account_new });
+            self.forest.grooves.accounts.update(.{ .old = cr_account, .new = &cr_account_new });
 
             self.commit_timestamp = t.timestamp;
             return .ok;
@@ -1399,11 +1399,18 @@ fn check(test_table: []const u8) !void {
                 assert(operation == null);
 
                 var account = context.state_machine.forest.grooves.accounts.get(b.account).?.*;
-                account.debits_pending = b.debits_pending;
-                account.debits_posted = b.debits_posted;
-                account.credits_pending = b.credits_pending;
-                account.credits_posted = b.credits_posted;
-                context.state_machine.forest.grooves.accounts.upsert(&account);
+                var account_new = account;
+
+                account_new.debits_pending = b.debits_pending;
+                account_new.debits_posted = b.debits_posted;
+                account_new.credits_pending = b.credits_pending;
+                account_new.credits_posted = b.credits_posted;
+                if (!stdx.equal_bytes(Account, &account_new, &account)) {
+                    context.state_machine.forest.grooves.accounts.update(.{
+                        .old = &account,
+                        .new = &account_new,
+                    });
+                }
             },
 
             .tick => |ticks| {

src/stdx.zig

@@ -242,7 +242,7 @@ else
 ///   - `T` doesn't have any non-deterministic padding,
 ///   - `T` doesn't embed any pointers.
 pub fn equal_bytes(comptime T: type, a: *const T, b: *const T) bool {
-    comptime assert(std.meta.trait.hasUniqueRepresentation(T));
+    comptime assert(hasUniqueRepresentation(T));
     comptime assert(!has_pointers(T));
     comptime assert(@sizeOf(T) * 8 == @bitSizeOf(T));
 
@@ -339,7 +339,7 @@ test no_padding {
 pub inline fn hash_inline(value: anytype) u64 {
     comptime {
         assert(no_padding(@TypeOf(value)));
-        assert(std.meta.trait.hasUniqueRepresentation(@TypeOf(value)));
+        assert(hasUniqueRepresentation(@TypeOf(value)));
     }
     return low_level_hash(0, switch (@typeInfo(@TypeOf(value))) {
         .Struct, .Int => std.mem.asBytes(&value),
@@ -520,3 +520,46 @@ pub fn fstatfs(fd: i32, statfs_buf: *StatFs) usize {
         @intFromPtr(statfs_buf),
     );
 }
+
+// TODO(Zig): https://github.com/ziglang/zig/issues/17592.
+/// True if every value of the type `T` has a unique bit pattern representing it.
+/// In other words, `T` has no unused bits and no padding.
+pub fn hasUniqueRepresentation(comptime T: type) bool {
+    switch (@typeInfo(T)) {
+        else => return false, // TODO can we know if it's true for some of these types ?
+
+        .AnyFrame,
+        .Enum,
+        .ErrorSet,
+        .Fn,
+        => return true,
+
+        .Bool => return false,
+
+        .Int => |info| return @sizeOf(T) * 8 == info.bits,
+
+        .Pointer => |info| return info.size != .Slice,
+
+        .Array => |info| return comptime hasUniqueRepresentation(info.child),
+
+        .Struct => |info| {
+            // Packed structs are always unique.
+            if (info.backing_integer != null) {
+                return true;
+            }
+
+            var sum_size = @as(usize, 0);
+
+            inline for (info.fields) |field| {
+                const FieldType = field.type;
+                if (comptime !hasUniqueRepresentation(FieldType)) return false;
+                sum_size += @sizeOf(FieldType);
+            }
+
+            return @sizeOf(T) == sum_size;
+        },
+
+        .Vector => |info| return comptime hasUniqueRepresentation(info.child) and
+            @sizeOf(T) == @sizeOf(info.child) * info.len,
+    }
+}

src/testing/state_machine.zig

@@ -167,7 +167,7 @@ pub fn StateMachineType(
                     const thing = state_machine.forest.grooves.things.get(op);
                     assert(thing == null);
 
-                    state_machine.forest.grooves.things.upsert(&.{
+                    state_machine.forest.grooves.things.insert(&.{
                         .timestamp = timestamp,
                         .id = op,
                         .value = @as(u64, @truncate(vsr.checksum(input))),

src/tidy.zig

@@ -64,6 +64,11 @@ fn banned(source: []const u8) ?[]const u8 {
     if (std.mem.indexOf(u8, source, "std." ++ "BoundedArray") != null) {
         return "use stdx." ++ "BoundedArray instead of std version";
     }
+
+    if (std.mem.indexOf(u8, source, "trait." ++ "hasUniqueRepresentation") != null) {
+        return "use stdx." ++ "hasUniqueRepresentation instead of std version";
+    }
+
     return null;
 }