Skip to content

Commit

Permalink
Reuse some allocations in map_keyed and map_indexed (#478)
Browse files Browse the repository at this point in the history
* Reuse some Vec allocations

* Simplify code for map_indexed

* Update js_framework_bench workflow
  • Loading branch information
lukechu10 authored Sep 1, 2022
1 parent 1d13fda commit 74ddddb
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 32 deletions.
1 change: 1 addition & 0 deletions .github/workflows/js_framework_bench.yml
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ jobs:
run: |
cd ./js-framework-benchmark
npm ci
(cd server && npm ci)
(cd webdriver-ts && npm ci)
(cd webdriver-ts-results && npm ci)
Expand Down
58 changes: 26 additions & 32 deletions packages/sycamore-reactive/src/iter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,12 @@ where
{
// Previous state used for diffing.
let mut items = Rc::new(Vec::new());

let mut mapped: Vec<U> = Vec::new();
let mut mapped_tmp: Vec<Option<U>> = Vec::new();

let mut disposers: Vec<Option<ScopeDisposer<'a>>> = Vec::new();
let mut disposers_tmp: Vec<Option<ScopeDisposer<'a>>> = Vec::new();

let signal = create_signal(cx, Vec::new());

Expand All @@ -45,18 +49,16 @@ where
if new_items.is_empty() {
// Fast path for removing all items.
for dis in mem::take(&mut disposers) {
unsafe {
dis.unwrap().dispose();
}
unsafe { dis.unwrap().dispose() };
}
mapped = Vec::new();
} else if items.is_empty() {
// Fast path for new create.
// TODO: do not clone T
mapped.reserve(new_items.len());
disposers.reserve(new_items.len());

for new_item in new_items.iter().cloned() {
let mut tmp = None;
let new_disposer = create_child_scope(cx, |cx| tmp = Some(map_fn(cx, new_item)));
mapped.push(tmp.unwrap());
let new_disposer = create_child_scope(cx, |cx| mapped.push(map_fn(cx, new_item)));
disposers.push(Some(new_disposer));
}
} else {
Expand All @@ -65,14 +67,11 @@ where
"new_items.is_empty() and items.is_empty() are special cased"
);

let mut temp = vec![None; new_items.len()];
let mut temp_disposers = {
let mut tmp = Vec::with_capacity(new_items.len());
for _ in 0..new_items.len() {
tmp.push(None);
}
tmp
};
mapped_tmp.clear();
mapped_tmp.resize(new_items.len(), None);

disposers_tmp.clear();
disposers_tmp.resize_with(new_items.len(), || None);

// Skip common prefix.
let min_len = usize::min(items.len(), new_items.len());
Expand All @@ -90,13 +89,11 @@ where
// Skip common suffix.
let mut end = items.len();
let mut new_end = new_items.len();
#[allow(clippy::suspicious_operation_groupings)]
// FIXME: make code clearer so that clippy won't complain
while end > start && new_end > start && items[end - 1] == new_items[new_end - 1] {
end -= 1;
new_end -= 1;
temp[new_end] = Some(mapped[end].clone());
temp_disposers[new_end] = disposers[end].take();
mapped_tmp[new_end] = Some(mapped[end].clone());
disposers_tmp[new_end] = disposers[end].take();
}
debug_assert!(
if end != 0 && new_end != 0 {
Expand Down Expand Up @@ -128,29 +125,27 @@ where
let item = &items[i];
if let Some(j) = new_indices.get(&key_fn(item)).copied() {
// Moved. j is index of item in new_items.
temp[j] = Some(mapped[i].clone());
temp_disposers[j] = disposers[i].take();
mapped_tmp[j] = Some(mapped[i].clone());
disposers_tmp[j] = disposers[i].take();
new_indices_next[j - start].and_then(|j| new_indices.insert(key_fn(item), j));
} else {
// Create new.
unsafe {
disposers[i].take().unwrap().dispose();
}
unsafe { disposers[i].take().unwrap().dispose() };
}
}

// 2) Set all the new values, pulling from the moved array if copied, otherwise
// entering the new value.
for j in start..new_items.len() {
if matches!(temp.get(j), Some(Some(_))) {
if matches!(mapped_tmp.get(j), Some(Some(_))) {
// Pull from moved array.
if j >= mapped.len() {
debug_assert_eq!(mapped.len(), j);
mapped.push(temp[j].clone().unwrap());
disposers.push(temp_disposers[j].take());
mapped.push(mapped_tmp[j].clone().unwrap());
disposers.push(disposers_tmp[j].take());
} else {
mapped[j] = temp[j].clone().unwrap();
disposers[j] = temp_disposers[j].take();
mapped[j] = mapped_tmp[j].clone().unwrap();
disposers[j] = disposers_tmp[j].take();
}
} else {
// Create new value.
Expand Down Expand Up @@ -236,14 +231,13 @@ where
disposers.reserve(new_count);
}

for (i, new_item) in new_items.iter().enumerate() {
let new_item = new_item.clone();
for (i, new_item) in new_items.iter().cloned().enumerate() {
let item = items.get(i);
// We lift the equality out of the else if branch to satisfy borrow checker.
let eqs = item != Some(&new_item);

let mut tmp = None;
if item.is_none() || eqs {
let mut tmp = None;
let new_disposer =
create_child_scope(cx, |cx| tmp = Some(map_fn(cx, new_item)));
if item.is_none() {
Expand Down

0 comments on commit 74ddddb

Please sign in to comment.