change opid to (u32,u32) - 10% performance uptick (automerge#473)

rust/automerge/src/automerge.rs

@@ -487,7 +487,7 @@ impl Automerge {
                 // do a direct get here b/c this could be foriegn and not be within the array
                 // bounds
                 let obj = if self.ops.m.actors.cache.get(*idx) == Some(actor) {
-                    ObjId(OpId(*ctr, *idx))
+                    ObjId(OpId::new(*ctr, *idx))
                 } else {
                     // FIXME - make a real error
                     let idx = self
@@ -496,7 +496,7 @@ impl Automerge {
                         .actors
                         .lookup(actor)
                         .ok_or(AutomergeError::Fail)?;
-                    ObjId(OpId(*ctr, idx))
+                    ObjId(OpId::new(*ctr, idx))
                 };
                 if let Some(obj_type) = self.ops.object_type(&obj) {
                     Ok((obj, obj_type))
@@ -859,23 +859,26 @@ impl Automerge {
             .iter_ops()
             .enumerate()
             .map(|(i, c)| {
-                let id = OpId(change.start_op().get() + i as u64, actor);
+                let id = OpId::new(change.start_op().get() + i as u64, actor);
                 let key = match &c.key {
                     EncodedKey::Prop(n) => Key::Map(self.ops.m.props.cache(n.to_string())),
                     EncodedKey::Elem(e) if e.is_head() => Key::Seq(ElemId::head()),
                     EncodedKey::Elem(ElemId(o)) => {
-                        Key::Seq(ElemId(OpId::new(actors[o.actor()], o.counter())))
+                        Key::Seq(ElemId(OpId::new(o.counter(), actors[o.actor()])))
                     }
                 };
                 let obj = if c.obj.is_root() {
                     ObjId::root()
                 } else {
-                    ObjId(OpId(c.obj.opid().counter(), actors[c.obj.opid().actor()]))
+                    ObjId(OpId::new(
+                        c.obj.opid().counter(),
+                        actors[c.obj.opid().actor()],
+                    ))
                 };
                 let pred = c
                     .pred
                     .iter()
-                    .map(|p| OpId::new(actors[p.actor()], p.counter()));
+                    .map(|p| OpId::new(p.counter(), actors[p.actor()]));
                 let pred = self.ops.m.sorted_opids(pred);
                 (
                     obj,

rust/automerge/src/change.rs

@@ -356,7 +356,7 @@ pub(crate) mod gen {
         (0_u64..10)
             .prop_map(|num_ops| {
                 (0..num_ops)
-                    .map(|counter| OpId::new(0, counter))
+                    .map(|counter| OpId::new(counter, 0))
                     .collect::<Vec<_>>()
             })
             .prop_flat_map(move |opids| {

rust/automerge/src/clock.rs

@@ -59,8 +59,8 @@ impl Clock {
     }
 
     pub(crate) fn covers(&self, id: &OpId) -> bool {
-        if let Some(data) = self.0.get(&id.1) {
-            data.max_op >= id.0
+        if let Some(data) = self.0.get(&id.actor()) {
+            data.max_op >= id.counter()
         } else {
             false
         }
@@ -123,16 +123,16 @@ mod tests {
         clock.include(1, ClockData { max_op: 20, seq: 1 });
         clock.include(2, ClockData { max_op: 10, seq: 2 });
 
-        assert!(clock.covers(&OpId(10, 1)));
-        assert!(clock.covers(&OpId(20, 1)));
-        assert!(!clock.covers(&OpId(30, 1)));
+        assert!(clock.covers(&OpId::new(10, 1)));
+        assert!(clock.covers(&OpId::new(20, 1)));
+        assert!(!clock.covers(&OpId::new(30, 1)));
 
-        assert!(clock.covers(&OpId(5, 2)));
-        assert!(clock.covers(&OpId(10, 2)));
-        assert!(!clock.covers(&OpId(15, 2)));
+        assert!(clock.covers(&OpId::new(5, 2)));
+        assert!(clock.covers(&OpId::new(10, 2)));
+        assert!(!clock.covers(&OpId::new(15, 2)));
 
-        assert!(!clock.covers(&OpId(1, 3)));
-        assert!(!clock.covers(&OpId(100, 3)));
+        assert!(!clock.covers(&OpId::new(1, 3)));
+        assert!(!clock.covers(&OpId::new(100, 3)));
     }
 
     #[test]

rust/automerge/src/columnar/column_range/key.rs

@@ -167,11 +167,11 @@ impl<'a> KeyIter<'a> {
                 Ok(Some(Key::Prop(string)))
             }
             (Some(None) | None, Some(Some(0)), Some(None) | None) => {
-                Ok(Some(Key::Elem(ElemId(OpId(0, 0)))))
+                Ok(Some(Key::Elem(ElemId(OpId::new(0, 0)))))
             }
             (Some(Some(actor)), Some(Some(ctr)), Some(None) | None) => match ctr.try_into() {
                 //Ok(ctr) => Some(Ok(Key::Elem(ElemId(OpId(ctr, actor as usize))))),
-                Ok(ctr) => Ok(Some(Key::Elem(ElemId(OpId::new(actor as usize, ctr))))),
+                Ok(ctr) => Ok(Some(Key::Elem(ElemId(OpId::new(ctr, actor as usize))))),
                 Err(_) => Err(DecodeColumnError::invalid_value(
                     "counter",
                     "negative value for counter",

rust/automerge/src/columnar/column_range/obj_id.rs

@@ -133,7 +133,7 @@ impl<'a> ObjIdIter<'a> {
             .map_err(|e| DecodeColumnError::decode_raw("counter", e))?;
         match (actor, counter) {
             (None | Some(None), None | Some(None)) => Ok(Some(ObjId::root())),
-            (Some(Some(a)), Some(Some(c))) => Ok(Some(ObjId(OpId(c, a as usize)))),
+            (Some(Some(a)), Some(Some(c))) => Ok(Some(ObjId(OpId::new(c, a as usize)))),
             (_, Some(Some(0))) => Ok(Some(ObjId::root())),
             (Some(None) | None, _) => Err(DecodeColumnError::unexpected_null("actor")),
             (_, Some(None) | None) => Err(DecodeColumnError::unexpected_null("counter")),

rust/automerge/src/columnar/column_range/opid.rs

@@ -105,7 +105,7 @@ impl<'a> OpIdIter<'a> {
             .map_err(|e| DecodeColumnError::decode_raw("counter", e))?;
         match (actor, counter) {
             (Some(Some(a)), Some(Some(c))) => match c.try_into() {
-                Ok(c) => Ok(Some(OpId(c, a as usize))),
+                Ok(c) => Ok(Some(OpId::new(c, a as usize))),
                 Err(_) => Err(DecodeColumnError::invalid_value(
                     "counter",
                     "negative value encountered",

rust/automerge/src/columnar/column_range/opid_list.rs

@@ -203,7 +203,7 @@ impl<'a> OpIdListIter<'a> {
                 .map_err(|e| DecodeColumnError::decode_raw("counter", e))?;
             match (actor, counter) {
                 (Some(Some(a)), Some(Some(ctr))) => match ctr.try_into() {
-                    Ok(ctr) => p.push(OpId(ctr, a as usize)),
+                    Ok(ctr) => p.push(OpId::new(ctr, a as usize)),
                     Err(_e) => {
                         return Err(DecodeColumnError::invalid_value(
                             "counter",

rust/automerge/src/columnar/encoding/properties.rs

@@ -139,7 +139,7 @@ pub(crate) fn option_splice_scenario<
 }
 
 pub(crate) fn opid() -> impl Strategy<Value = OpId> + Clone {
-    (0..(i64::MAX as usize), 0..(i64::MAX as u64)).prop_map(|(actor, ctr)| OpId(ctr, actor))
+    (0..(i64::MAX as usize), 0..(i64::MAX as u64)).prop_map(|(actor, ctr)| OpId::new(ctr, actor))
 }
 
 pub(crate) fn elemid() -> impl Strategy<Value = ElemId> + Clone {

rust/automerge/src/op_set.rs

@@ -55,7 +55,11 @@ impl OpSetInternal {
         if id == types::ROOT {
             ExId::Root
         } else {
-            ExId::Id(id.0, self.m.actors.cache[id.1].clone(), id.1)
+            ExId::Id(
+                id.counter(),
+                self.m.actors.cache[id.actor()].clone(),
+                id.actor(),
+            )
         }
     }
 
@@ -355,13 +359,7 @@ impl OpSetMetadata {
     }
 
     pub(crate) fn lamport_cmp(&self, left: OpId, right: OpId) -> Ordering {
-        match (left, right) {
-            (OpId(0, _), OpId(0, _)) => Ordering::Equal,
-            (OpId(0, _), OpId(_, _)) => Ordering::Less,
-            (OpId(_, _), OpId(0, _)) => Ordering::Greater,
-            (OpId(a, x), OpId(b, y)) if a == b => self.actors[x].cmp(&self.actors[y]),
-            (OpId(a, _), OpId(b, _)) => a.cmp(&b),
-        }
+        left.lamport_cmp(&right, &self.actors.cache)
     }
 
     pub(crate) fn sorted_opids<I: Iterator<Item = OpId>>(&self, opids: I) -> OpIds {

rust/automerge/src/op_tree.rs

@@ -325,7 +325,7 @@ mod tests {
     use super::*;
 
     fn op() -> Op {
-        let zero = OpId(0, 0);
+        let zero = OpId::new(0, 0);
         Op {
             id: zero,
             action: amp::OpType::Put(0.into()),

rust/automerge/src/op_tree/iter.rs

@@ -262,7 +262,7 @@ mod tests {
     fn op(counter: u64) -> Op {
         Op {
             action: OpType::Put(ScalarValue::Uint(counter)),
-            id: OpId(counter, 0),
+            id: OpId::new(counter, 0),
             key: Key::Map(0),
             succ: Default::default(),
             pred: Default::default(),

rust/automerge/src/transaction/inner.rs

@@ -240,7 +240,7 @@ impl TransactionInner {
     }
 
     fn next_id(&mut self) -> OpId {
-        OpId(self.start_op.get() + self.pending_ops() as u64, self.actor)
+        OpId::new(self.start_op.get() + self.pending_ops() as u64, self.actor)
     }
 
     fn next_insert(&mut self, key: Key, value: ScalarValue) -> Op {

rust/automerge/src/types.rs

@@ -3,10 +3,12 @@ use crate::legacy as amp;
 use serde::{Deserialize, Serialize};
 use std::borrow::Cow;
 use std::cmp::Eq;
+use std::cmp::Ordering;
 use std::fmt;
 use std::fmt::Display;
 use std::str::FromStr;
 use tinyvec::{ArrayVec, TinyVec};
+//use crate::indexed_cache::IndexedCache;
 
 mod opids;
 pub(crate) use opids::OpIds;
@@ -253,17 +255,6 @@ pub(crate) trait Exportable {
     fn export(&self) -> Export;
 }
 
-impl OpId {
-    #[inline]
-    pub(crate) fn counter(&self) -> u64 {
-        self.0
-    }
-    #[inline]
-    pub(crate) fn actor(&self) -> usize {
-        self.1
-    }
-}
-
 impl Exportable for ObjId {
     fn export(&self) -> Export {
         if self.0 == ROOT {
@@ -421,11 +412,28 @@ impl Key {
 }
 
 #[derive(Debug, Clone, PartialOrd, Ord, Eq, PartialEq, Copy, Hash, Default)]
-pub(crate) struct OpId(pub(crate) u64, pub(crate) usize);
+pub(crate) struct OpId(u32, u32);
 
 impl OpId {
-    pub(crate) fn new(actor: usize, counter: u64) -> Self {
-        Self(counter, actor)
+    pub(crate) fn new(counter: u64, actor: usize) -> Self {
+        Self(counter as u32, actor as u32)
+    }
+
+    #[inline]
+    pub(crate) fn counter(&self) -> u64 {
+        self.0 as u64
+    }
+
+    #[inline]
+    pub(crate) fn actor(&self) -> usize {
+        self.1 as usize
+    }
+
+    #[inline]
+    pub(crate) fn lamport_cmp(&self, other: &OpId, actors: &[ActorId]) -> Ordering {
+        self.0
+            .cmp(&other.0)
+            .then_with(|| actors[self.1 as usize].cmp(&actors[other.1 as usize]))
     }
 }
 

rust/automerge/src/types/opids.rs

@@ -129,7 +129,8 @@ mod tests {
 
     fn gen_opid(actors: Vec<ActorId>) -> impl Strategy<Value = OpId> {
         (0..actors.len()).prop_flat_map(|actor_idx| {
-            (Just(actor_idx), 0..u64::MAX).prop_map(|(actor_idx, counter)| OpId(counter, actor_idx))
+            (Just(actor_idx), 0..u64::MAX)
+                .prop_map(|(actor_idx, counter)| OpId::new(counter, actor_idx))
         })
     }
 
@@ -190,7 +191,7 @@ mod tests {
             (OpId(0, _), OpId(0, _)) => Ordering::Equal,
             (OpId(0, _), OpId(_, _)) => Ordering::Less,
             (OpId(_, _), OpId(0, _)) => Ordering::Greater,
-            (OpId(a, x), OpId(b, y)) if a == b => actors[*x].cmp(&actors[*y]),
+            (OpId(a, x), OpId(b, y)) if a == b => actors[*x as usize].cmp(&actors[*y as usize]),
             (OpId(a, _), OpId(b, _)) => a.cmp(b),
         }
     }