Replace HashMap with BTreeMap in valence_nbt (#98)

* Replace HashMap with BTreeMap in valence_nbt

Turns out that `BTreeMap`s are a bit faster when the element count is low.

This change also makes debugging compounds a bit easier since the elements are displayed in sorted order.

* Simplify read_list function slightly
This commit is contained in:
Ryan Johnson 2022-10-01 15:36:04 -07:00 committed by GitHub
parent 9c62bc1b90
commit 2cd8bd2195
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 45 additions and 41 deletions

View file

@ -6,7 +6,7 @@ repository = "https://github.com/valence-rs/valence_nbt"
readme = "README.md" readme = "README.md"
license = "MIT" license = "MIT"
keywords = ["nbt", "minecraft", "serialization"] keywords = ["nbt", "minecraft", "serialization"]
version = "0.1.2" version = "0.2.0"
authors = ["Ryan Johnson <ryanj00a@gmail.com>"] authors = ["Ryan Johnson <ryanj00a@gmail.com>"]
edition = "2021" edition = "2021"

View file

@ -12,7 +12,7 @@ pub struct Compound {
} }
#[cfg(not(feature = "preserve_order"))] #[cfg(not(feature = "preserve_order"))]
type Map = std::collections::HashMap<String, Value>; type Map = std::collections::BTreeMap<String, Value>;
#[cfg(feature = "preserve_order")] #[cfg(feature = "preserve_order")]
type Map = indexmap::IndexMap<String, Value>; type Map = indexmap::IndexMap<String, Value>;
@ -24,6 +24,13 @@ impl Compound {
pub fn with_capacity(cap: usize) -> Self { pub fn with_capacity(cap: usize) -> Self {
Self { Self {
#[cfg(not(feature = "preserve_order"))]
map: {
// BTreeMap does not have with_capacity.
let _ = cap;
Map::new()
},
#[cfg(feature = "preserve_order")]
map: Map::with_capacity(cap), map: Map::with_capacity(cap),
} }
} }
@ -88,14 +95,22 @@ impl Compound {
self.map.remove_entry(k) self.map.remove_entry(k)
} }
// TODO: append function for potential BTreeMap usage? pub fn append(&mut self, other: &mut Self) {
#[cfg(not(feature = "preserve_order"))]
self.map.append(&mut other.map);
#[cfg(feature = "preserve_order")]
for (k, v) in std::mem::replace(&mut other.map, Map::default()) {
self.map.insert(k, v);
}
}
pub fn entry<K>(&mut self, k: K) -> Entry pub fn entry<K>(&mut self, k: K) -> Entry
where where
K: Into<String>, K: Into<String>,
{ {
#[cfg(not(feature = "preserve_order"))] #[cfg(not(feature = "preserve_order"))]
use std::collections::hash_map::Entry as EntryImpl; use std::collections::btree_map::Entry as EntryImpl;
#[cfg(feature = "preserve_order")] #[cfg(feature = "preserve_order")]
use indexmap::map::Entry as EntryImpl; use indexmap::map::Entry as EntryImpl;
@ -219,7 +234,7 @@ impl<'a> Entry<'a> {
pub struct VacantEntry<'a> { pub struct VacantEntry<'a> {
#[cfg(not(feature = "preserve_order"))] #[cfg(not(feature = "preserve_order"))]
ve: std::collections::hash_map::VacantEntry<'a, String, Value>, ve: std::collections::btree_map::VacantEntry<'a, String, Value>,
#[cfg(feature = "preserve_order")] #[cfg(feature = "preserve_order")]
ve: indexmap::map::VacantEntry<'a, String, Value>, ve: indexmap::map::VacantEntry<'a, String, Value>,
} }
@ -236,7 +251,7 @@ impl<'a> VacantEntry<'a> {
pub struct OccupiedEntry<'a> { pub struct OccupiedEntry<'a> {
#[cfg(not(feature = "preserve_order"))] #[cfg(not(feature = "preserve_order"))]
oe: std::collections::hash_map::OccupiedEntry<'a, String, Value>, oe: std::collections::btree_map::OccupiedEntry<'a, String, Value>,
#[cfg(feature = "preserve_order")] #[cfg(feature = "preserve_order")]
oe: indexmap::map::OccupiedEntry<'a, String, Value>, oe: indexmap::map::OccupiedEntry<'a, String, Value>,
} }
@ -336,7 +351,7 @@ impl<'a> IntoIterator for &'a Compound {
#[derive(Clone)] #[derive(Clone)]
pub struct Iter<'a> { pub struct Iter<'a> {
#[cfg(not(feature = "preserve_order"))] #[cfg(not(feature = "preserve_order"))]
iter: std::collections::hash_map::Iter<'a, String, Value>, iter: std::collections::btree_map::Iter<'a, String, Value>,
#[cfg(feature = "preserve_order")] #[cfg(feature = "preserve_order")]
iter: indexmap::map::Iter<'a, String, Value>, iter: indexmap::map::Iter<'a, String, Value>,
} }
@ -356,7 +371,7 @@ impl<'a> IntoIterator for &'a mut Compound {
pub struct IterMut<'a> { pub struct IterMut<'a> {
#[cfg(not(feature = "preserve_order"))] #[cfg(not(feature = "preserve_order"))]
iter: std::collections::hash_map::IterMut<'a, String, Value>, iter: std::collections::btree_map::IterMut<'a, String, Value>,
#[cfg(feature = "preserve_order")] #[cfg(feature = "preserve_order")]
iter: indexmap::map::IterMut<'a, String, Value>, iter: indexmap::map::IterMut<'a, String, Value>,
} }
@ -376,7 +391,7 @@ impl IntoIterator for Compound {
pub struct IntoIter { pub struct IntoIter {
#[cfg(not(feature = "preserve_order"))] #[cfg(not(feature = "preserve_order"))]
iter: std::collections::hash_map::IntoIter<String, Value>, iter: std::collections::btree_map::IntoIter<String, Value>,
#[cfg(feature = "preserve_order")] #[cfg(feature = "preserve_order")]
iter: indexmap::map::IntoIter<String, Value>, iter: indexmap::map::IntoIter<String, Value>,
} }
@ -386,7 +401,7 @@ impl_iterator_traits!((IntoIter) => (String, Value));
#[derive(Clone)] #[derive(Clone)]
pub struct Keys<'a> { pub struct Keys<'a> {
#[cfg(not(feature = "preserve_order"))] #[cfg(not(feature = "preserve_order"))]
iter: std::collections::hash_map::Keys<'a, String, Value>, iter: std::collections::btree_map::Keys<'a, String, Value>,
#[cfg(feature = "preserve_order")] #[cfg(feature = "preserve_order")]
iter: indexmap::map::Keys<'a, String, Value>, iter: indexmap::map::Keys<'a, String, Value>,
} }
@ -396,7 +411,7 @@ impl_iterator_traits!((Keys<'a>) => &'a String);
#[derive(Clone)] #[derive(Clone)]
pub struct Values<'a> { pub struct Values<'a> {
#[cfg(not(feature = "preserve_order"))] #[cfg(not(feature = "preserve_order"))]
iter: std::collections::hash_map::Values<'a, String, Value>, iter: std::collections::btree_map::Values<'a, String, Value>,
#[cfg(feature = "preserve_order")] #[cfg(feature = "preserve_order")]
iter: indexmap::map::Values<'a, String, Value>, iter: indexmap::map::Values<'a, String, Value>,
} }
@ -405,7 +420,7 @@ impl_iterator_traits!((Values<'a>) => &'a Value);
pub struct ValuesMut<'a> { pub struct ValuesMut<'a> {
#[cfg(not(feature = "preserve_order"))] #[cfg(not(feature = "preserve_order"))]
iter: std::collections::hash_map::ValuesMut<'a, String, Value>, iter: std::collections::btree_map::ValuesMut<'a, String, Value>,
#[cfg(feature = "preserve_order")] #[cfg(feature = "preserve_order")]
iter: indexmap::map::ValuesMut<'a, String, Value>, iter: indexmap::map::ValuesMut<'a, String, Value>,
} }

View file

@ -160,55 +160,44 @@ impl DecodeState<'_, '_> {
"TAG_End list with nonzero length of {len}" "TAG_End list with nonzero length of {len}"
))), ))),
}, },
Tag::Byte => Ok(self Tag::Byte => Ok(self.read_list(Tag::Byte, 1, |st| st.read_byte())?.into()),
.read_list::<_, _, 1>(Tag::Byte, |st| st.read_byte())? Tag::Short => Ok(self.read_list(Tag::Short, 2, |st| st.read_short())?.into()),
.into()), Tag::Int => Ok(self.read_list(Tag::Int, 4, |st| st.read_int())?.into()),
Tag::Short => Ok(self Tag::Long => Ok(self.read_list(Tag::Long, 8, |st| st.read_long())?.into()),
.read_list::<_, _, 2>(Tag::Short, |st| st.read_short())? Tag::Float => Ok(self.read_list(Tag::Float, 4, |st| st.read_float())?.into()),
.into()),
Tag::Int => Ok(self
.read_list::<_, _, 4>(Tag::Int, |st| st.read_int())?
.into()),
Tag::Long => Ok(self
.read_list::<_, _, 8>(Tag::Long, |st| st.read_long())?
.into()),
Tag::Float => Ok(self
.read_list::<_, _, 4>(Tag::Float, |st| st.read_float())?
.into()),
Tag::Double => Ok(self Tag::Double => Ok(self
.read_list::<_, _, 8>(Tag::Double, |st| st.read_double())? .read_list(Tag::Double, 8, |st| st.read_double())?
.into()), .into()),
Tag::ByteArray => Ok(self Tag::ByteArray => Ok(self
.read_list::<_, _, 4>(Tag::ByteArray, |st| st.read_byte_array())? .read_list(Tag::ByteArray, 4, |st| st.read_byte_array())?
.into()), .into()),
Tag::String => Ok(self Tag::String => Ok(self
.read_list::<_, _, 2>(Tag::String, |st| st.read_string())? .read_list(Tag::String, 2, |st| st.read_string())?
.into()), .into()),
Tag::List => self.check_depth(|st| { Tag::List => self
Ok(st .check_depth(|st| Ok(st.read_list(Tag::List, 5, |st| st.read_any_list())?.into())),
.read_list::<_, _, 5>(Tag::List, |st| st.read_any_list())?
.into())
}),
Tag::Compound => self.check_depth(|st| { Tag::Compound => self.check_depth(|st| {
Ok(st Ok(st
.read_list::<_, _, 1>(Tag::Compound, |st| st.read_compound())? .read_list(Tag::Compound, 1, |st| st.read_compound())?
.into()) .into())
}), }),
Tag::IntArray => Ok(self Tag::IntArray => Ok(self
.read_list::<_, _, 4>(Tag::IntArray, |st| st.read_int_array())? .read_list(Tag::IntArray, 4, |st| st.read_int_array())?
.into()), .into()),
Tag::LongArray => Ok(self Tag::LongArray => Ok(self
.read_list::<_, _, 4>(Tag::LongArray, |st| st.read_long_array())? .read_list(Tag::LongArray, 4, |st| st.read_long_array())?
.into()), .into()),
} }
} }
/// Assumes the element tag has already been read. /// Assumes the element tag has already been read.
/// ///
/// `MIN_ELEM_SIZE` is the minimum size of the list element when encoded. /// `min_elem_size` is the minimum size of the list element when encoded.
fn read_list<T, F, const MIN_ELEM_SIZE: usize>( #[inline]
fn read_list<T, F>(
&mut self, &mut self,
elem_type: Tag, elem_type: Tag,
min_elem_size: usize,
mut read_elem: F, mut read_elem: F,
) -> Result<Vec<T>> ) -> Result<Vec<T>>
where where
@ -224,7 +213,7 @@ impl DecodeState<'_, '_> {
// Ensure we don't reserve more than the maximum amount of memory required given // Ensure we don't reserve more than the maximum amount of memory required given
// the size of the remaining input. // the size of the remaining input.
if len as u64 * MIN_ELEM_SIZE as u64 > self.slice.len() as u64 { if len as u64 * min_elem_size as u64 > self.slice.len() as u64 {
return Err(Error::new_owned(format!( return Err(Error::new_owned(format!(
"{elem_type} list of length {len} exceeds remainder of input" "{elem_type} list of length {len} exceeds remainder of input"
))); )));