Skip to content

Commit

Permalink
Turn ZeroVec into a struct (#2599)
Browse files Browse the repository at this point in the history
* Add alternate owned/borrowed ZeroVec methods

* Use everywhere

* Replace with internal pointer

* fix

* fix

* rm old docs

* clip

* fix

* oops

* Add to_mut_slice()

* fix

* merge fix
  • Loading branch information
Manishearth authored Sep 21, 2022
1 parent f53a87e commit 0e395d9
Show file tree
Hide file tree
Showing 17 changed files with 262 additions and 183 deletions.
2 changes: 1 addition & 1 deletion components/collections/src/codepointinvlist/cpinvlist.rs
Original file line number Diff line number Diff line change
Expand Up @@ -823,7 +823,7 @@ mod tests {
postcard::from_bytes::<CodePointInversionList>(&set_serialized)?;

assert_eq!(&set, &set_deserialized);
assert!(matches!(set_deserialized.inv_list, ZeroVec::Borrowed(_)));
assert!(!set_deserialized.inv_list.is_owned());

Ok(())
}
Expand Down
12 changes: 3 additions & 9 deletions components/collections/src/codepointtrie/cptrie.rs
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,7 @@ impl<'trie, T: TrieValue> CodePointTrie<'trie, T> {
let converted_data = self.data.try_into_converted()?;
let error_ule = self.error_value.to_unaligned();
let slice = &[error_ule];
let error_vec = ZeroVec::<T>::Borrowed(slice);
let error_vec = ZeroVec::<T>::new_borrowed(slice);
let error_converted = error_vec.try_into_converted::<P>()?;
#[allow(clippy::expect_used)] // we know this cannot fail
Ok(CodePointTrie {
Expand Down Expand Up @@ -1106,14 +1106,8 @@ mod tests {
assert_eq!(&trie.index, &trie_deserialized.index);
assert_eq!(&trie.data, &trie_deserialized.data);

assert!(matches!(
trie_deserialized.index,
zerovec::ZeroVec::Borrowed(_)
));
assert!(matches!(
trie_deserialized.data,
zerovec::ZeroVec::Borrowed(_)
));
assert!(!trie_deserialized.index.is_owned());
assert!(!trie_deserialized.data.is_owned());

Ok(())
}
Expand Down
4 changes: 2 additions & 2 deletions components/datetime/src/pattern/runtime/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ pub fn maybe_replace_first(pattern: &mut Pattern, f: impl Fn(&PatternItem) -> Op
.find_map(|(i, item)| f(&item).map(|result| (i, result)));
#[allow(clippy::indexing_slicing)] // i was produced by enumerate
if let Some((i, result)) = result {
pattern.items.to_mut()[i] = result.to_unaligned();
pattern.items.to_mut_slice()[i] = result.to_unaligned();
}
}

Expand All @@ -47,7 +47,7 @@ pub fn maybe_replace(pattern: &mut Pattern, f: impl Fn(&PatternItem) -> Option<P
.find_map(|(i, item)| f(&item).map(|result| (i, result)));
#[allow(clippy::indexing_slicing)] // i was produced by enumerate
if let Some((i, result)) = result {
let owned = pattern.items.to_mut();
let owned = pattern.items.to_mut_slice();
owned[i] = result.to_unaligned();
owned.iter_mut().skip(i).for_each(|item| {
if let Some(new_item) = f(&PatternItem::from_unaligned(*item)) {
Expand Down
2 changes: 1 addition & 1 deletion components/datetime/src/pattern/runtime/pattern.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ impl FromStr for Pattern<'_> {
impl Default for Pattern<'_> {
fn default() -> Self {
Self {
items: ZeroVec::Owned(Vec::new()),
items: ZeroVec::new(),
time_granularity: TimeGranularity::default(),
}
}
Expand Down
2 changes: 1 addition & 1 deletion components/plurals/src/rules/runtime/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -476,7 +476,7 @@ mod test {
operand: Operand::I,
},
modulo: 0,
range_list: ZeroVec::Borrowed(&[RangeOrValue::Value(1).to_unaligned()])
range_list: ZeroVec::new_borrowed(&[RangeOrValue::Value(1).to_unaligned()])
}
);

Expand Down
5 changes: 2 additions & 3 deletions provider/datagen/src/transform/cldr/calendar/japanese.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,12 +79,11 @@ impl crate::DatagenProvider {
.map_err(|e| DataError::custom("Era codes").with_display_context(&e))?;
if start_date.year >= 1868 || japanext {
ret.dates_to_eras
.to_mut()
.push((start_date, code).to_unaligned());
.with_mut(|v| v.push((start_date, code).to_unaligned()));
}
}

ret.dates_to_eras.to_mut().sort_unstable();
ret.dates_to_eras.to_mut_slice().sort_unstable();

// Integrity check
//
Expand Down
2 changes: 1 addition & 1 deletion provider/datagen/src/transform/segmenter/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -610,7 +610,7 @@ impl crate::DatagenProvider {

Ok(RuleBreakDataV1 {
property_table: RuleBreakPropertyTable(property_trie),
break_state_table: RuleBreakStateTable(ZeroVec::Owned(break_state_table)),
break_state_table: RuleBreakStateTable(ZeroVec::new_owned(break_state_table)),
property_count: property_length as u8,
last_codepoint_property: (simple_properties_count - 1) as i8,
sot_property: (property_length - 2) as u8,
Expand Down
6 changes: 3 additions & 3 deletions utils/tinystr/src/ule.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,9 @@ mod test {
fn test_zerovec() {
let mut vec = ZeroVec::<TinyAsciiStr<7>>::new();

vec.to_mut().push("foobar".parse().unwrap());
vec.to_mut().push("baz".parse().unwrap());
vec.to_mut().push("quux".parse().unwrap());
vec.with_mut(|v| v.push("foobar".parse().unwrap()));
vec.with_mut(|v| v.push("baz".parse().unwrap()));
vec.with_mut(|v| v.push("quux".parse().unwrap()));

let bytes = vec.as_bytes();

Expand Down
2 changes: 1 addition & 1 deletion utils/zerovec/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ assert_eq!(deserialized.nums.first(), Some(211));
assert_eq!(deserialized.chars.get(1), Some('冇'));
assert_eq!(deserialized.strs.get(1), Some("world"));
// The deserialization will not have allocated anything
assert!(matches!(deserialized.nums, ZeroVec::Borrowed(_)));
assert!(!deserialized.nums.is_owned());
```

Use custom types inside of ZeroVec:
Expand Down
4 changes: 2 additions & 2 deletions utils/zerovec/benches/zerovec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,15 +94,15 @@ fn sum_benches(c: &mut Criterion) {

c.bench_function("zerovec/sum/sample/zerovec_aligned", |b| {
b.iter(|| {
ZeroVec::<u32>::Borrowed(black_box(aligned_ule_slice))
ZeroVec::<u32>::new_borrowed(black_box(aligned_ule_slice))
.iter()
.fold(0u32, |sum, val| sum.wrapping_add(val))
});
});

c.bench_function("zerovec/sum/sample/zerovec_unaligned", |b| {
b.iter(|| {
ZeroVec::<u32>::Borrowed(black_box(unalign_ule_slice))
ZeroVec::<u32>::new_borrowed(black_box(unalign_ule_slice))
.iter()
.fold(0u32, |sum, val| sum.wrapping_add(val))
});
Expand Down
2 changes: 1 addition & 1 deletion utils/zerovec/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@
//! assert_eq!(deserialized.chars.get(1), Some('冇'));
//! assert_eq!(deserialized.strs.get(1), Some("world"));
//! // The deserialization will not have allocated anything
//! assert!(matches!(deserialized.nums, ZeroVec::Borrowed(_)));
//! assert!(!deserialized.nums.is_owned());
//! # } // feature = "serde"
//! ```
//!
Expand Down
28 changes: 13 additions & 15 deletions utils/zerovec/src/map/vecs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -291,32 +291,34 @@ where
{
type OwnedType = T;
fn zvl_insert(&mut self, index: usize, value: &T) {
self.to_mut().insert(index, value.to_unaligned())
self.with_mut(|v| v.insert(index, value.to_unaligned()))
}
fn zvl_remove(&mut self, index: usize) -> T {
T::from_unaligned(self.to_mut().remove(index))
T::from_unaligned(self.with_mut(|v| v.remove(index)))
}
fn zvl_replace(&mut self, index: usize, value: &T) -> T {
let vec = self.to_mut();
debug_assert!(index < vec.len());
#[allow(clippy::indexing_slicing)]
T::from_unaligned(mem::replace(&mut vec[index], value.to_unaligned()))
let unaligned = self.with_mut(|vec| {
debug_assert!(index < vec.len());
mem::replace(&mut vec[index], value.to_unaligned())
});
T::from_unaligned(unaligned)
}
fn zvl_push(&mut self, value: &T) {
self.to_mut().push(value.to_unaligned())
self.with_mut(|v| v.push(value.to_unaligned()))
}
fn zvl_with_capacity(cap: usize) -> Self {
if cap == 0 {
ZeroVec::new()
} else {
ZeroVec::Owned(Vec::with_capacity(cap))
ZeroVec::new_owned(Vec::with_capacity(cap))
}
}
fn zvl_clear(&mut self) {
self.to_mut().clear()
self.with_mut(|v| v.clear())
}
fn zvl_reserve(&mut self, addl: usize) {
self.to_mut().reserve(addl)
self.with_mut(|v| v.reserve(addl))
}

fn owned_as_t(o: &Self::OwnedType) -> &T {
Expand All @@ -327,18 +329,14 @@ where
b.as_zerovec()
}
fn zvl_as_borrowed_inner(&self) -> Option<&'a ZeroSlice<T>> {
if let ZeroVec::Borrowed(b) = *self {
Some(ZeroSlice::from_ule_slice(b))
} else {
None
}
self.as_maybe_borrowed()
}

#[allow(clippy::indexing_slicing)] // documented panic
fn zvl_permute(&mut self, permutation: &mut [usize]) {
assert_eq!(permutation.len(), self.zvl_len());

let vec = self.to_mut();
let vec = self.to_mut_slice();

for cycle_start in 0..permutation.len() {
let mut curr = cycle_start;
Expand Down
35 changes: 19 additions & 16 deletions utils/zerovec/src/map2d/map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -235,14 +235,14 @@ where
/// Removes key0_index from the keys0 array and the joiner array
fn remove_key0_index(&mut self, key0_index: usize) {
self.keys0.zvl_remove(key0_index);
self.joiner.to_mut().remove(key0_index);
self.joiner.with_mut(|v| v.remove(key0_index));
}

/// Shifts all joiner ranges from key0_index onward one index up
fn joiner_expand(&mut self, key0_index: usize) {
#[allow(clippy::expect_used)] // slice overflow
self.joiner
.to_mut()
.to_mut_slice()
.iter_mut()
.skip(key0_index)
.for_each(|ref mut v| {
Expand All @@ -252,16 +252,16 @@ where
.checked_add(1)
.expect("Attempted to add more than 2^32 elements to a ZeroMap2d")
.to_unaligned()
});
})
}

/// Shifts all joiner ranges from key0_index onward one index down
fn joiner_shrink(&mut self, key0_index: usize) {
self.joiner
.to_mut()
.to_mut_slice()
.iter_mut()
.skip(key0_index)
.for_each(|ref mut v| **v = (v.as_unsigned_int() - 1).to_unaligned());
.for_each(|ref mut v| **v = (v.as_unsigned_int() - 1).to_unaligned())
}
}

Expand Down Expand Up @@ -386,7 +386,7 @@ where
) -> Option<(&'b K0, &'b K1, &'b V)> {
if self.is_empty() {
self.keys0.zvl_push(key0);
self.joiner.to_mut().push(1u32.to_unaligned());
self.joiner.with_mut(|v| v.push(1u32.to_unaligned()));
self.keys1.zvl_push(key1);
self.values.zvl_push(value);
return None;
Expand Down Expand Up @@ -426,10 +426,11 @@ where
#[allow(clippy::unwrap_used)]
if key0_cmp == Ordering::Greater {
self.keys0.zvl_push(key0);
self.joiner.to_mut().push(joiner_value.to_unaligned());
self.joiner
.with_mut(|v| v.push(joiner_value.to_unaligned()));
} else {
// This unwrap is protected because we are not empty
*self.joiner.to_mut().last_mut().unwrap() = joiner_value.to_unaligned();
*self.joiner.to_mut_slice().last_mut().unwrap() = joiner_value.to_unaligned();
}
self.keys1.zvl_push(key1);
self.values.zvl_push(value);
Expand Down Expand Up @@ -553,8 +554,7 @@ where
};
self.keys0.zvl_insert(key0_index, key0);
self.joiner
.to_mut()
.insert(key0_index, joiner_value.to_unaligned());
.with_mut(|v| v.insert(key0_index, joiner_value.to_unaligned()));
(key0_index, (joiner_value as usize)..(joiner_value as usize))
}
}
Expand Down Expand Up @@ -704,13 +704,16 @@ mod test {
fn stress_test() {
let mut zm2d = ZeroMap2d::<u16, str, str>::new();

assert_eq!(format!("{:?}", zm2d), "ZeroMap2d { keys0: ZeroVec::Borrowed([]), joiner: ZeroVec::Borrowed([]), keys1: [], values: [] }");
assert_eq!(
format!("{:?}", zm2d),
"ZeroMap2d { keys0: ZeroVec([]), joiner: ZeroVec([]), keys1: [], values: [] }"
);
assert_eq!(zm2d.get0(&0), None);

let result = zm2d.try_append(&3, "ccc", "CCC");
assert!(matches!(result, None));

assert_eq!(format!("{:?}", zm2d), "ZeroMap2d { keys0: ZeroVec::Owned([3]), joiner: ZeroVec::Owned([1]), keys1: [\"ccc\"], values: [\"CCC\"] }");
assert_eq!(format!("{:?}", zm2d), "ZeroMap2d { keys0: ZeroVec([3]), joiner: ZeroVec([1]), keys1: [\"ccc\"], values: [\"CCC\"] }");
assert_eq!(zm2d.get0(&0), None);
assert_eq!(zm2d.get0(&3).unwrap().get1(""), None);
assert_eq!(zm2d.get_2d(&3, "ccc"), Some("CCC"));
Expand All @@ -719,7 +722,7 @@ mod test {
let result = zm2d.try_append(&3, "eee", "EEE");
assert!(matches!(result, None));

assert_eq!(format!("{:?}", zm2d), "ZeroMap2d { keys0: ZeroVec::Owned([3]), joiner: ZeroVec::Owned([2]), keys1: [\"ccc\", \"eee\"], values: [\"CCC\", \"EEE\"] }");
assert_eq!(format!("{:?}", zm2d), "ZeroMap2d { keys0: ZeroVec([3]), joiner: ZeroVec([2]), keys1: [\"ccc\", \"eee\"], values: [\"CCC\", \"EEE\"] }");
assert_eq!(zm2d.get0(&0), None);
assert_eq!(zm2d.get0(&3).unwrap().get1(""), None);
assert_eq!(zm2d.get_2d(&3, "ccc"), Some("CCC"));
Expand All @@ -743,7 +746,7 @@ mod test {
let result = zm2d.try_append(&9, "yyy", "YYY");
assert!(matches!(result, None));

assert_eq!(format!("{:?}", zm2d), "ZeroMap2d { keys0: ZeroVec::Owned([3, 5, 7, 9]), joiner: ZeroVec::Owned([2, 3, 6, 7]), keys1: [\"ccc\", \"eee\", \"ddd\", \"ddd\", \"eee\", \"www\", \"yyy\"], values: [\"CCC\", \"EEE\", \"DD1\", \"DD2\", \"EEE\", \"WWW\", \"YYY\"] }");
assert_eq!(format!("{:?}", zm2d), "ZeroMap2d { keys0: ZeroVec([3, 5, 7, 9]), joiner: ZeroVec([2, 3, 6, 7]), keys1: [\"ccc\", \"eee\", \"ddd\", \"ddd\", \"eee\", \"www\", \"yyy\"], values: [\"CCC\", \"EEE\", \"DD1\", \"DD2\", \"EEE\", \"WWW\", \"YYY\"] }");
assert_eq!(zm2d.get0(&0), None);
assert_eq!(zm2d.get0(&3).unwrap().get1(""), None);
assert_eq!(zm2d.get_2d(&3, "ccc"), Some("CCC"));
Expand Down Expand Up @@ -774,7 +777,7 @@ mod test {
zm2d.insert(&6, "mmm", "MM1");
zm2d.insert(&6, "nnn", "NNN");

assert_eq!(format!("{:?}", zm2d), "ZeroMap2d { keys0: ZeroVec::Owned([3, 5, 6, 7, 9]), joiner: ZeroVec::Owned([3, 4, 7, 10, 11]), keys1: [\"ccc\", \"eee\", \"mmm\", \"ddd\", \"ddd\", \"mmm\", \"nnn\", \"ddd\", \"eee\", \"www\", \"yyy\"], values: [\"CCC\", \"EEE\", \"MM0\", \"DD1\", \"DD3\", \"MM1\", \"NNN\", \"DD2\", \"EEE\", \"WWW\", \"YYY\"] }");
assert_eq!(format!("{:?}", zm2d), "ZeroMap2d { keys0: ZeroVec([3, 5, 6, 7, 9]), joiner: ZeroVec([3, 4, 7, 10, 11]), keys1: [\"ccc\", \"eee\", \"mmm\", \"ddd\", \"ddd\", \"mmm\", \"nnn\", \"ddd\", \"eee\", \"www\", \"yyy\"], values: [\"CCC\", \"EEE\", \"MM0\", \"DD1\", \"DD3\", \"MM1\", \"NNN\", \"DD2\", \"EEE\", \"WWW\", \"YYY\"] }");
assert_eq!(zm2d.get0(&0), None);
assert_eq!(zm2d.get0(&3).unwrap().get1(""), None);
assert_eq!(zm2d.get_2d(&3, "ccc"), Some("CCC"));
Expand Down Expand Up @@ -814,6 +817,6 @@ mod test {
let result = zm2d.remove(&9, "yyy"); // last element
assert_eq!(result, Some(String::from("YYY").into_boxed_str()));

assert_eq!(format!("{:?}", zm2d), "ZeroMap2d { keys0: ZeroVec::Owned([3, 6, 7]), joiner: ZeroVec::Owned([1, 4, 7]), keys1: [\"eee\", \"ddd\", \"mmm\", \"nnn\", \"ddd\", \"eee\", \"www\"], values: [\"EEE\", \"DD3\", \"MM1\", \"NNN\", \"DD2\", \"EEE\", \"WWW\"] }");
assert_eq!(format!("{:?}", zm2d), "ZeroMap2d { keys0: ZeroVec([3, 6, 7]), joiner: ZeroVec([1, 4, 7]), keys1: [\"eee\", \"ddd\", \"mmm\", \"nnn\", \"ddd\", \"eee\", \"www\"], values: [\"EEE\", \"DD3\", \"MM1\", \"NNN\", \"DD2\", \"EEE\", \"WWW\"] }");
}
}
4 changes: 2 additions & 2 deletions utils/zerovec/src/zerofrom_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ where
{
#[inline]
fn zero_from(other: &'zf ZeroVec<'_, T>) -> Self {
ZeroVec::Borrowed(other.as_ule_slice())
ZeroVec::new_borrowed(other.as_ule_slice())
}
}

Expand All @@ -24,7 +24,7 @@ where
{
#[inline]
fn zero_from(other: &'zf ZeroSlice<T>) -> Self {
ZeroVec::Borrowed(other.as_ule_slice())
ZeroVec::new_borrowed(other.as_ule_slice())
}
}

Expand Down
Loading

3 comments on commit 0e395d9

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Heap – ubuntu-latest'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.

Benchmark suite Current: 0e395d9 Previous: f53a87e Ratio
icu_collections/unicode_bmp_blocks_selector – Heap at End of Program Execution 16 bytes 0 bytes Infinity

This comment was automatically generated by workflow using github-action-benchmark.

CC: @sffc @zbraniecki @echeran

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Heap – windows-latest'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.

Benchmark suite Current: 0e395d9 Previous: f53a87e Ratio
icu_collections/unicode_bmp_blocks_selector – Heap at End of Program Execution 16 bytes 0 bytes Infinity

This comment was automatically generated by workflow using github-action-benchmark.

CC: @sffc @zbraniecki @echeran

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Heap – macos-latest'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.

Benchmark suite Current: 0e395d9 Previous: f53a87e Ratio
icu_collections/unicode_bmp_blocks_selector – Heap at End of Program Execution 16 bytes 0 bytes Infinity

This comment was automatically generated by workflow using github-action-benchmark.

CC: @sffc @zbraniecki @echeran

Please sign in to comment.