Revert "perf(engine): keep empty numeric maps on a direct lookup path"

This reverts commit a63dd94045f3a4b95dbfc917d5d8afa5c22f1897.
This commit is contained in:
boris
2026-09-13 14:16:06 +08:00
committed by boris
parent e8abf43cd4
commit 0a6fab9038
2 changed files with 7 additions and 32 deletions
+4 -23
View File
@@ -19,7 +19,6 @@ pub struct NumericFactorMap {
#[derive(Clone)]
enum Storage {
Empty,
Owned(Vec<(CompactString, f64)>),
Shared(SharedRow),
}
@@ -51,13 +50,12 @@ fn compact_key(key: Cow<'static, str>) -> CompactString {
impl NumericFactorMap {
pub const fn new() -> Self {
Self {
storage: Storage::Empty,
storage: Storage::Owned(Vec::new()),
}
}
pub fn len(&self) -> usize {
match &self.storage {
Storage::Empty => 0,
Storage::Owned(entries) => entries.len(),
Storage::Shared(row) => row.len(),
}
@@ -67,7 +65,6 @@ impl NumericFactorMap {
}
pub fn clear(&mut self) {
match &mut self.storage {
Storage::Empty => {},
Storage::Owned(entries) => entries.clear(),
Storage::Shared(_) => *self = Self::new(),
}
@@ -80,7 +77,6 @@ impl NumericFactorMap {
pub(crate) fn has_normalized_finite_entries(&self) -> bool {
match &self.storage {
Storage::Empty => true,
Storage::Owned(entries) => entries.iter().all(|(name, value)| {
normalized_name(name) && value.is_finite()
}),
@@ -89,23 +85,18 @@ impl NumericFactorMap {
}
fn owned_entries(&mut self) -> &mut Vec<(CompactString, f64)> {
if matches!(self.storage, Storage::Empty) {
self.storage = Storage::Owned(Vec::new());
}
if matches!(self.storage, Storage::Shared(_)) {
let entries = self.iter().map(|(key, value)| (key.clone(), *value)).collect();
self.storage = Storage::Owned(entries);
}
match &mut self.storage {
Storage::Owned(entries) => entries,
Storage::Empty => unreachable!("empty storage was initialized"),
Storage::Shared(_) => unreachable!("shared row was materialized"),
}
}
pub fn get(&self, key: &str) -> Option<&f64> {
match &self.storage {
Storage::Empty => None,
Storage::Owned(entries) => entries
.binary_search_by(|(name, _)| name.as_str().cmp(key))
.ok()
@@ -115,9 +106,7 @@ impl NumericFactorMap {
}
pub fn get_mut(&mut self, key: &str) -> Option<&mut f64> {
if matches!(self.storage, Storage::Empty)
|| matches!(self.storage, Storage::Shared(_)) && !self.contains_key(key)
{
if !self.contains_key(key) {
return None;
}
let entries = self.owned_entries();
@@ -156,9 +145,7 @@ impl NumericFactorMap {
}
pub fn remove(&mut self, key: &str) -> Option<f64> {
if matches!(self.storage, Storage::Empty)
|| matches!(self.storage, Storage::Shared(_)) && !self.contains_key(key)
{
if !self.contains_key(key) {
return None;
}
let entries = self.owned_entries();
@@ -174,7 +161,6 @@ impl NumericFactorMap {
pub fn iter(&self) -> Iter<'_> {
match &self.storage {
Storage::Empty => Iter(IterStorage::Owned([].iter())),
Storage::Owned(entries) => Iter(IterStorage::Owned(entries.iter())),
Storage::Shared(row) => Iter(IterStorage::Shared(row.iter())),
}
@@ -242,7 +228,6 @@ impl IntoIterator for NumericFactorMap {
type IntoIter = std::vec::IntoIter<Self::Item>;
fn into_iter(self) -> Self::IntoIter {
match self.storage {
Storage::Empty => Vec::new().into_iter(),
Storage::Owned(entries) => entries.into_iter(),
Storage::Shared(row) => row.iter().map(|(key, value)| (key.clone(), *value))
.collect::<Vec<_>>().into_iter(),
@@ -270,13 +255,9 @@ impl FromIterator<(CompactString, f64)> for NumericFactorMap {
false
}
});
if entries.is_empty() {
Self::new()
} else {
Self { storage: Storage::Owned(entries) }
}
}
}
impl Extend<(Cow<'static, str>, f64)> for NumericFactorMap {
fn extend<T: IntoIterator<Item = (Cow<'static, str>, f64)>>(&mut self, iter: T) {
self.extend(iter.into_iter().map(|(key, value)| (compact_key(key), value)));
@@ -386,7 +367,7 @@ mod tests {
let normalized = rows.iter().map(NumericFactorMap::has_normalized_finite_entries).collect::<Vec<_>>();
assert_eq!(NumericFactorMap::share_rows(rows.iter_mut()), rows.len());
for (index, row) in rows.iter().enumerate() {
assert!(row.is_empty() || matches!(row.storage, Storage::Shared(_)));
assert!(matches!(row.storage, Storage::Shared(_)));
assert_eq!(row_bits(row), before[index]);
assert_eq!(serde_json::to_string(row).unwrap(), serialized[index]);
assert_eq!(row.has_normalized_finite_entries(), normalized[index]);
@@ -100,9 +100,7 @@ pub(super) fn share<'a>(rows: impl IntoIterator<Item = &'a mut NumericFactorMap>
return 0;
}
let Some(owned_bytes) = rows.iter().try_fold(0usize, |sum, row| {
let Storage::Owned(entries) = &row.storage else {
return matches!(row.storage, Storage::Empty).then_some(sum);
};
let Storage::Owned(entries) = &row.storage else { return None };
sum.checked_add(entries.capacity().checked_mul(std::mem::size_of::<(CompactString, f64)>())?)
}) else { return 0 };
let mut names = BTreeMap::<&str, &CompactString>::new();
@@ -168,11 +166,7 @@ pub(super) fn share<'a>(rows: impl IntoIterator<Item = &'a mut NumericFactorMap>
});
let count = rows.len();
for (index, row) in rows.into_iter().enumerate() {
row.storage = if data.lengths[index] == 0 {
Storage::Empty
} else {
Storage::Shared(SharedRow { data: Arc::clone(&data), index })
};
row.storage = Storage::Shared(SharedRow { data: Arc::clone(&data), index });
}
count
}