Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update methods to accept an AsRef<[u8]> type instead of a Vec<u8> #422

Merged
merged 1 commit into from
Nov 15, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions benchmarks/stress2/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,19 +138,19 @@ fn run(tree: Arc<sled::Tree>, shutdown: Arc<AtomicBool>) {

match choice {
v if v <= get_max => {
tree.get(&*key).unwrap();
tree.get(&key).unwrap();
}
v if v > get_max && v <= set_max => {
tree.set(key, bytes(args.flag_val_len)).unwrap();
tree.set(&key, bytes(args.flag_val_len)).unwrap();
}
v if v > set_max && v <= del_max => {
tree.del(&*key).unwrap();
tree.del(&key).unwrap();
}
v if v > del_max && v <= cas_max => {
let old_k = bytes(args.flag_val_len);

let old = if rng.gen::<bool>() {
Some(&*old_k)
Some(old_k.as_slice())
} else {
None
};
Expand All @@ -161,20 +161,20 @@ fn run(tree: Arc<sled::Tree>, shutdown: Arc<AtomicBool>) {
None
};

match tree.cas(key, old, new) {
match tree.cas(&key, old, new) {
Ok(_) | Err(sled::Error::CasFailed(_)) => {}
other => panic!("operational error: {:?}", other),
}
}
v if v > cas_max && v <= scan_max => {
let _ = tree
.scan(&*key)
.scan(&key)
.take(rng.gen_range(0, 15))
.map(|res| res.unwrap())
.collect::<Vec<_>>();
}
_ => {
tree.merge(key, bytes(args.flag_val_len)).unwrap();
tree.merge(&key, bytes(args.flag_val_len)).unwrap();
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions crates/sled/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@
//! ```
//! let t = sled::Tree::start_default("my_db").unwrap();
//!
//! t.set(b"yo!".to_vec(), b"v1".to_vec());
//! t.set(b"yo!", b"v1".to_vec());
//! assert!(t.get(b"yo!").unwrap().unwrap() == &*b"v1".to_vec());
//!
//! t.cas(
//! b"yo!".to_vec(), // key
//! b"yo!", // key
Kerollmops marked this conversation as resolved.
Show resolved Hide resolved
//! Some(b"v1"), // old value, None for not present
//! Some(b"v2".to_vec()), // new value, None for delete
//! ).unwrap();
Expand Down
119 changes: 63 additions & 56 deletions crates/sled/src/tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,9 +154,9 @@ impl Tree {
}

/// Retrieve a value from the `Tree` if it exists.
pub fn get(
pub fn get<K: AsRef<[u8]>>(
&self,
key: &[u8],
key: K,
) -> Result<Option<PinnedValue>, ()> {
let _measure = Measure::new(&M.tree_get);

Expand All @@ -165,7 +165,7 @@ impl Tree {
// the double guard is a hack that maintains
// correctness of the ret value
let double_guard = guard.clone();
let (_, ret) = self.get_internal(key, &guard)?;
let (_, ret) = self.get_internal(key.as_ref(), &guard)?;

guard.flush();

Expand All @@ -186,22 +186,22 @@ impl Tree {
/// let t = sled::Tree::start(config).unwrap();
///
/// // unique creation
/// assert_eq!(t.cas(vec![1], None, Some(vec![1])), Ok(()));
/// // assert_eq!(t.cas(vec![1], None, Some(vec![1])), Err(Error::CasFailed(Some(vec![1]))));
/// assert_eq!(t.cas(&[1], None, Some(vec![1])), Ok(()));
/// // assert_eq!(t.cas(&[1], None, Some(vec![1])), Err(Error::CasFailed(Some(vec![1]))));
///
/// // conditional modification
/// assert_eq!(t.cas(vec![1], Some(&*vec![1]), Some(vec![2])), Ok(()));
/// // assert_eq!(t.cas(vec![1], Some(vec![1]), Some(vec![2])), Err(Error::CasFailed(Some(vec![2]))));
/// assert_eq!(t.cas(&[1], Some(&*vec![1]), Some(vec![2])), Ok(()));
/// // assert_eq!(t.cas(&[1], Some(vec![1]), Some(vec![2])), Err(Error::CasFailed(Some(vec![2]))));
///
/// // conditional deletion
/// assert_eq!(t.cas(vec![1], Some(&*vec![2]), None), Ok(()));
/// assert_eq!(t.get(&*vec![1]), Ok(None));
/// assert_eq!(t.cas(&[1], Some(&[2]), None), Ok(()));
/// assert_eq!(t.get(&[1]), Ok(None));
/// ```
pub fn cas(
pub fn cas<K: AsRef<[u8]>>(
&self,
key: Key,
key: K,
old: Option<&[u8]>,
new: Option<Value>,
new: Option<Value>
) -> Result<(), Option<PinnedValue>> {
let _measure = Measure::new(&M.tree_cas);

Expand All @@ -216,10 +216,10 @@ impl Tree {
loop {
let pin_guard = guard.clone();
let (mut path, cur) = self
.get_internal(&*key, &guard)
.get_internal(key.as_ref(), &guard)
.map_err(|e| e.danger_cast())?;

if old != cur.map(|v| &*v) {
if old.as_ref().map(|o| o.as_ref()) != cur.map(|v| &*v) {
return Err(Error::CasFailed(
cur.map(|c| PinnedValue::new(c, pin_guard)),
));
Expand All @@ -231,7 +231,7 @@ impl Tree {

let (node_id, encoded_key) = {
let node: &Node = leaf_frag.unwrap_base();
(node.id, prefix_encode(node.lo.inner(), &*key))
(node.id, prefix_encode(node.lo.inner(), key.as_ref()))
};
let frag = if let Some(ref n) = new {
Frag::Set(encoded_key, n.clone())
Expand Down Expand Up @@ -261,9 +261,9 @@ impl Tree {

/// Set a key to a new value, returning the old value if it
/// was set.
pub fn set(
pub fn set<K: AsRef<[u8]>>(
&self,
key: Key,
key: K,
value: Value,
) -> Result<Option<PinnedValue>, ()> {
let _measure = Measure::new(&M.tree_set);
Expand All @@ -278,13 +278,13 @@ impl Tree {

loop {
let double_guard = guard.clone();
let mut path = self.path_for_key(&*key, &guard)?;
let mut path = self.path_for_key(key.as_ref(), &guard)?;
let (leaf_frag, leaf_ptr) = path.pop().expect(
"path_for_key should always return a path \
of length >= 2 (root + leaf)",
);
let node: &Node = leaf_frag.unwrap_base();
let encoded_key = prefix_encode(node.lo.inner(), &*key);
let encoded_key = prefix_encode(node.lo.inner(), key.as_ref());

// Search for the existing key, so we can return
// it later.
Expand Down Expand Up @@ -375,24 +375,28 @@ impl Tree {
///
/// let tree = sled::Tree::start(config).unwrap();
///
/// let k = b"k1".to_vec();
/// let k = b"k1";
///
/// tree.set(k.clone(), vec![0]);
/// tree.merge(k.clone(), vec![1]);
/// tree.merge(k.clone(), vec![2]);
/// // assert_eq!(tree.get(&k).unwrap().unwrap(), vec![0, 1, 2]);
/// tree.set(k, vec![0]);
/// tree.merge(k, vec![1]);
/// tree.merge(k, vec![2]);
/// // assert_eq!(tree.get(k).unwrap().unwrap(), vec![0, 1, 2]);
///
/// // sets replace previously merged data,
/// // bypassing the merge function.
/// tree.set(k.clone(), vec![3]);
/// // assert_eq!(tree.get(&k), Ok(Some(vec![3])));
/// tree.set(k, vec![3]);
/// // assert_eq!(tree.get(k), Ok(Some(vec![3])));
///
/// // merges on non-present values will add them
/// tree.del(&k);
/// tree.merge(k.clone(), vec![4]);
/// // assert_eq!(tree.get(&k).unwrap().unwrap(), vec![4]);
/// tree.del(k);
/// tree.merge(k, vec![4]);
/// // assert_eq!(tree.get(k).unwrap().unwrap(), vec![4]);
/// ```
pub fn merge(&self, key: Key, value: Value) -> Result<(), ()> {
pub fn merge<K: AsRef<[u8]>>(
&self,
key: K,
value: Value
) -> Result<(), ()> {
let _measure = Measure::new(&M.tree_merge);

if self.config.read_only {
Expand All @@ -404,14 +408,14 @@ impl Tree {
let guard = pin();

loop {
let mut path = self.path_for_key(&*key, &guard)?;
let mut path = self.path_for_key(key.as_ref(), &guard)?;
let (leaf_frag, leaf_ptr) = path.pop().expect(
"path_for_key should always return a path \
of length >= 2 (root + leaf)",
);
let node: &Node = leaf_frag.unwrap_base();

let encoded_key = prefix_encode(node.lo.inner(), &*key);
let encoded_key = prefix_encode(node.lo.inner(), key.as_ref());
let frag = Frag::Merge(encoded_key, value.clone());

let link = self.pages.link(
Expand Down Expand Up @@ -457,11 +461,14 @@ impl Tree {
/// ```
/// let config = sled::ConfigBuilder::new().temporary(true).build();
/// let t = sled::Tree::start(config).unwrap();
/// t.set(vec![1], vec![1]);
/// t.set(&[1], vec![1]);
/// assert_eq!(t.del(&*vec![1]).unwrap().unwrap(), vec![1]);
/// assert_eq!(t.del(&*vec![1]), Ok(None));
/// ```
pub fn del(&self, key: &[u8]) -> Result<Option<PinnedValue>, ()> {
pub fn del<K: AsRef<[u8]>>(
&self,
key: K
) -> Result<Option<PinnedValue>, ()> {
let _measure = Measure::new(&M.tree_del);

if self.config.read_only {
Expand All @@ -473,13 +480,13 @@ impl Tree {
let double_guard = guard.clone();
let mut ret: Option<&[u8]>;
loop {
let mut path = self.path_for_key(&*key, &guard)?;
let mut path = self.path_for_key(key.as_ref(), &guard)?;
let (leaf_frag, leaf_ptr) = path.pop().expect(
"path_for_key should always return a path \
of length >= 2 (root + leaf)",
);
let node: &Node = leaf_frag.unwrap_base();
let encoded_key = prefix_encode(node.lo.inner(), key);
let encoded_key = prefix_encode(node.lo.inner(), key.as_ref());
match node.data {
Data::Leaf(ref items) => {
let search =
Expand Down Expand Up @@ -529,21 +536,21 @@ impl Tree {
/// ```
/// let config = sled::ConfigBuilder::new().temporary(true).build();
/// let t = sled::Tree::start(config).unwrap();
/// t.set(vec![1], vec![10]);
/// t.set(vec![2], vec![20]);
/// t.set(vec![3], vec![30]);
/// t.set(&[1], vec![10]);
/// t.set(&[2], vec![20]);
/// t.set(&[3], vec![30]);
/// let mut iter = t.scan(&*vec![2]);
/// // assert_eq!(iter.next(), Some(Ok((vec![2], vec![20]))));
/// // assert_eq!(iter.next(), Some(Ok((vec![3], vec![30]))));
/// // assert_eq!(iter.next(), None);
/// ```
pub fn scan(&self, key: &[u8]) -> Iter<'_> {
pub fn scan<K: AsRef<[u8]>>(&self, key: K) -> Iter<'_> {
let _measure = Measure::new(&M.tree_scan);

let guard = pin();

let mut broken = None;
let id = match self.get_internal(key, &guard) {
let id = match self.get_internal(key.as_ref(), &guard) {
Ok((ref mut path, _)) if !path.is_empty() => {
let (leaf_frag, _leaf_ptr) = path.pop().expect(
"path_for_key should always return a path \
Expand All @@ -569,7 +576,7 @@ impl Tree {
Iter {
id,
inner: &self.pages,
last_key: Bound::Exclusive(key.to_vec()),
last_key: Bound::Exclusive(key.as_ref().to_vec()),
broken,
done: false,
guard,
Expand All @@ -583,9 +590,9 @@ impl Tree {
/// ```
/// let config = sled::ConfigBuilder::new().temporary(true).build();
/// let t = sled::Tree::start(config).unwrap();
/// t.set(vec![1], vec![10]);
/// t.set(vec![2], vec![20]);
/// t.set(vec![3], vec![30]);
/// t.set(&[1], vec![10]);
/// t.set(&[2], vec![20]);
/// t.set(&[3], vec![30]);
/// let mut iter = t.iter();
/// // assert_eq!(iter.next(), Some(Ok((vec![1], vec![10]))));
/// // assert_eq!(iter.next(), Some(Ok((vec![2], vec![20]))));
Expand Down Expand Up @@ -814,13 +821,13 @@ impl Tree {
}
}

fn get_internal<'g>(
fn get_internal<'g, K: AsRef<[u8]>>(
&self,
key: &[u8],
key: K,
guard: &'g Guard,
) -> Result<(Path<'g>, Option<&'g [u8]>), ()>
{
let path = self.path_for_key(&*key, guard)?;
let path = self.path_for_key(key.as_ref(), guard)?;

let ret = path.last().and_then(|(last_frag, _tree_ptr)| {
let last_node = last_frag.unwrap_base();
Expand All @@ -829,7 +836,7 @@ impl Tree {
data.leaf_ref().expect("last_node should be a leaf");
let search =
items.binary_search_by(|&(ref k, ref _v)| {
prefix_cmp_encoded(k, &key, last_node.lo.inner())
prefix_cmp_encoded(k, key.as_ref(), last_node.lo.inner())
});
if let Ok(idx) = search {
Some(&*items[idx].1)
Expand All @@ -843,10 +850,10 @@ impl Tree {
}

#[doc(hidden)]
pub fn key_debug_str(&self, key: &[u8]) -> String {
pub fn key_debug_str<K: AsRef<[u8]>>(&self, key: K) -> String {
let guard = pin();

let path = self.path_for_key(key, &guard).expect(
let path = self.path_for_key(key.as_ref(), &guard).expect(
"path_for_key should always return at least 2 nodes, \
even if the key being searched for is not present",
);
Expand All @@ -862,9 +869,9 @@ impl Tree {

/// returns the traversal path, completing any observed
/// partially complete splits or merges along the way.
fn path_for_key<'g>(
fn path_for_key<'g, K: AsRef<[u8]>>(
&self,
key: &[u8],
key: K,
guard: &'g Guard,
) -> Result<Vec<(&'g Frag, TreePtr<'g>)>, ()> {
let _measure = Measure::new(&M.tree_traverse);
Expand Down Expand Up @@ -909,10 +916,10 @@ impl Tree {
let node = frag.unwrap_base();

// TODO this may need to change when handling (half) merges
assert!(node.lo.inner() <= key, "overshot key somehow");
assert!(node.lo.inner() <= key.as_ref(), "overshot key somehow");

// half-complete split detect & completion
if node.hi <= Bound::Inclusive(key.to_vec()) {
if node.hi <= Bound::Inclusive(key.as_ref().to_vec()) {
// we have encountered a child split, without
// having hit the parent split above.
cursor = node.next.expect(
Expand Down Expand Up @@ -963,7 +970,7 @@ impl Tree {
let search = binary_search_lub(
ptrs,
|&(ref k, ref _v)| {
prefix_cmp_encoded(k, &key, &prefix)
prefix_cmp_encoded(k, key.as_ref(), &prefix)
},
);

Expand Down
6 changes: 3 additions & 3 deletions tests/src/tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -198,11 +198,11 @@ pub fn prop_tree_matches_btreemap(
for op in ops.into_iter() {
match op {
Set(k, v) => {
tree.set(k.0.clone(), vec![0, v]).unwrap();
tree.set(&k.0, vec![0, v]).unwrap();
reference.insert(k.clone(), v as u16);
}
Merge(k, v) => {
tree.merge(k.0.clone(), vec![v]).unwrap();
tree.merge(&k.0, vec![v]).unwrap();
let mut entry = reference.entry(k).or_insert(0u16);
*entry += v as u16;
}
Expand All @@ -222,7 +222,7 @@ pub fn prop_tree_matches_btreemap(
let tree_old = tree.get(&*k.0).unwrap();
if let Some(old_tree) = tree_old {
if old_tree == &*vec![0, old] {
tree.set(k.0.clone(), vec![0, new]).unwrap();
tree.set(&k.0, vec![0, new]).unwrap();
}
}

Expand Down
Loading