Merge pull request #627 from AtalayaLabs/claude/performance-optimization-analysis-2zhpoj
This commit is contained in:
+13
@@ -354,6 +354,19 @@ name = "bench_micro_allocs"
|
||||
path = "examples/bench_micro_allocs.rs"
|
||||
required-features = ["bench"]
|
||||
|
||||
# Round-18 battery ────────────────────────────────────────────────────────────
|
||||
|
||||
# Round-18 calendar-event edit CPU/alloc micro-pack — `update_ical_property` /
|
||||
# `remove_ical_property` mutate the `ical_data` body in place (`replace_range` /
|
||||
# `insert`) instead of re-`format!`ing the whole body per changed property, and
|
||||
# build the single `\nNAME:` search needle on the stack (dropping the redundant
|
||||
# `\r\nNAME:` needle). A multi-field REST edit went from one full-body (up to
|
||||
# ~11 KB) allocation per property to none. No Postgres.
|
||||
[[example]]
|
||||
name = "bench_round18_micro"
|
||||
path = "examples/bench_round18_micro.rs"
|
||||
required-features = ["bench"]
|
||||
|
||||
# Round-17 battery ────────────────────────────────────────────────────────────
|
||||
|
||||
# Round-17 dedup + CardDAV CPU/alloc micro-pack — `hash_chunk_sequence` by-value
|
||||
|
||||
@@ -0,0 +1,183 @@
|
||||
# Round 18 — calendar-event in-place iCal edit, ResourceList incremental id-index
|
||||
|
||||
Benchmark-gated, same rule as ROUND2–17: every change ships with a
|
||||
BEFORE/AFTER benchmark and an equivalence/safety gate; an AFTER that doesn't
|
||||
beat its BEFORE is rolled back (never applied). The roll-back rule is encoded
|
||||
directly into each harness — a `GATE FAIL … rollback` non-zero exit (Rust) or a
|
||||
failing `expect(afterMs).toBeLessThan(beforeMs / K)` assertion (vitest) — so a
|
||||
regression fails CI rather than shipping.
|
||||
|
||||
This round picks up two items carried on the **ROUND17 deferred list**:
|
||||
|
||||
- the **REST calendar-event edit** that re-`format!`'d the whole `ical_data`
|
||||
body once per changed property (backend, no Postgres — counting-allocator
|
||||
example), and
|
||||
- **`ResourceList.itemIndexById`**, which re-scanned the whole accumulated list
|
||||
per infinite-scroll page (frontend, vitest-benchmarked).
|
||||
|
||||
Reproduce:
|
||||
|
||||
```
|
||||
cargo run --release --features bench --example bench_round18_micro
|
||||
cd frontend && npx vitest run src/lib/components/round18.bench.test.ts
|
||||
```
|
||||
|
||||
## Summary
|
||||
|
||||
| # | change | key metric | before → after |
|
||||
|--:|---|---|---|
|
||||
| **C1** | `CalendarEvent::update_ical_property` / `remove_ical_property` rewrote the ENTIRE `ical_data` body with `format!("{}{}{}")` on every call and allocated **two** search needles per call (`\nNAME:` + the redundant `\r\nNAME:`). `calendar_storage_adapter::update_event` fans a multi-field edit out into one `update_ical_property` per changed field, so a full edit paid one full-body (up to ~11 KB) allocation **per property**. Now the body is mutated in place (`replace_range` for an existing property, four `insert`/`insert_str` for a new one) and the single `\nNAME:` needle is built on the stack. | 9-op edit, 1187-byte body | **70 → 2 allocs/op (68 fewer, ~35×) · 2.46× wall** (4255 → 1727 ns/op) |
|
||||
| **F1** | `ResourceList` derived `itemIndexById = new Map(items.map((i,idx)=>[i.id,idx]))` — a fresh Map over the WHOLE accumulated list every infinite-scroll page (O(N)/page, Σ O(N²)), and being a new instance each page it also re-fired the reap-stale `$effect` (another O(N) id `Set`/page for a reap an append can never cause). New `ItemIndexBuilder` extends a persistent Map with the fresh page only and reuses the reference across appends. | 40 pages × 50 | **74.1 → 6.4 ms · 11.5× faster** index build across the drain |
|
||||
|
||||
## [C1] calendar-event edit — in-place iCal property rewrite
|
||||
|
||||
`calendar_storage_adapter::update_event` hydrates the stored event, then applies
|
||||
each present field of the `UpdateEventDto` independently:
|
||||
|
||||
```rust
|
||||
if let Some(summary) = update.summary { event.update_summary(summary)?; }
|
||||
if let Some(description) = update.description { event.update_description(Some(description)); }
|
||||
if let Some(location) = update.location { event.update_location(Some(location)); }
|
||||
// …start/end (update_time_range), all_day (rewrites DTSTART+DTEND again), rrule…
|
||||
```
|
||||
|
||||
Every one of those funnels into `CalendarEvent::update_ical_property` (an absent
|
||||
field cleared → `remove_ical_property`), and the shipped-before body of that
|
||||
method rebuilt the **entire** `ical_data` String on each call:
|
||||
|
||||
```rust
|
||||
let search_str = format!("\n{}:", property_name); // needle 1
|
||||
let search_str_alt = format!("\r\n{}:", property_name); // needle 2 (redundant)
|
||||
let pos = self.ical_data.find(&search_str).or_else(|| self.ical_data.find(&search_str_alt));
|
||||
// …
|
||||
let before = &self.ical_data[..value_start];
|
||||
let after = &self.ical_data[value_end..];
|
||||
self.ical_data = format!("{}{}{}", before, value, after); // a whole fresh body String
|
||||
```
|
||||
|
||||
So a REST edit that changes summary + description + location + start + end +
|
||||
all-day + rrule allocated **one full-body String per property** — and calendar
|
||||
bodies run to ~11 KB once attendees / VALARMs are present — plus two throwaway
|
||||
needles per call.
|
||||
|
||||
Two observations drive the fix:
|
||||
|
||||
1. **The `\r\nNAME:` needle is redundant.** `\nNAME:` is a *suffix* of
|
||||
`\r\nNAME:`, so `find("\nNAME:")` already matches a CRLF-terminated property
|
||||
line (returning the `\n` offset) — the `.or_else(find("\r\nNAME:"))` branch
|
||||
can never be reached. One needle suffices, and since iCal property names are
|
||||
short ASCII it is built into a 64-byte **stack** buffer (`line_needle`) — zero
|
||||
heap needle.
|
||||
2. **The rewrite can be in place.** `replace_range(value_start..value_end, value)`
|
||||
is byte-for-byte what `before + value + after` produced, but it mutates the
|
||||
body's own buffer (growing once only when the new value is longer) instead of
|
||||
allocating a fresh body. The absent-property branch inserts the four pieces at
|
||||
one point in reverse (`\n`, value, `:`, name) after a single `reserve`, so a
|
||||
new property costs no fresh-body and no value-sized fragment either.
|
||||
|
||||
Because both arms edit the **same byte spans**, the emitted body is identical —
|
||||
including the pre-existing quirk that editing a line on a CRLF body drops that
|
||||
line's `\r` (the old span already included it; `replace_range` over the same
|
||||
span preserves the behaviour exactly). The bench's equivalence gate asserts the
|
||||
full 9-op edit is byte-identical, and the existing `calendar_event` unit tests
|
||||
(`update_summary` / `update_time_range` / `update_all_day` round-trips) pin the
|
||||
observable semantics.
|
||||
|
||||
Measured (`bench_round18_micro`, counting allocator, no Postgres). Both arms pay
|
||||
one identical `base.to_string()` reset per op (a shared constant), so the
|
||||
**fewer-allocs** figure is the pure per-edit saving.
|
||||
|
||||
```
|
||||
## [C1] calendar-event multi-field edit (9 ops, 1187-byte body)
|
||||
| arm | ns/op | allocs/op |
|
||||
| BEFORE update_event in-place property rewrite | 4255.4 | 70.00 |
|
||||
| AFTER update_event in-place property rewrite | 1726.5 | 2.00 |
|
||||
# 2.46x wall, 68.00 fewer allocs/op
|
||||
```
|
||||
|
||||
The AFTER arm's two allocations per whole 9-op edit are the shared
|
||||
`base.to_string()` reset and a single buffer grow (the longer DESCRIPTION value +
|
||||
the inserted RRULE), versus 70 for the old per-property `format!` fan-out — a
|
||||
35× cut, byte-identical output.
|
||||
|
||||
## [F1] ResourceList `itemIndexById` — incremental id→index Map
|
||||
|
||||
`ResourceList` pages its list in via infinite scroll (`items = [...items,
|
||||
...page]`) and derived, on every change:
|
||||
|
||||
```js
|
||||
const itemIndexById = $derived(new Map(items.map((i, idx) => [i.id, idx])));
|
||||
```
|
||||
|
||||
`selectedItems` reads that Map to project the current selection in list order.
|
||||
The derive is a full O(N) rebuild of a **fresh** Map over the whole accumulated
|
||||
list every page — Σ O(N²) across a P-page drain with a selection active — and,
|
||||
being a new instance each page, it also re-fired the reap-stale `$effect` (which
|
||||
reference-diffs it), and that effect built *another* throwaway O(N) `Set` of ids
|
||||
per page for a reap that an append can never trigger (an append only adds ids).
|
||||
|
||||
`ItemIndexBuilder` (new, `$lib/utils/itemIndex.ts`, mirroring
|
||||
`ResourceSectionsBuilder`) uses the shared O(1) `isAppendExtension` witness: on
|
||||
an append it indexes only the fresh tail into a persistent Map and returns the
|
||||
**same reference**; any other change (reload, deletion, non-append) rebuilds into
|
||||
a **new** Map. That reference contract is exactly what the two consumers want:
|
||||
|
||||
- `selectedItems` re-derives on every `items` change regardless (it indexes
|
||||
`items[idx]`), so it always reads the freshly-extended Map — a stable
|
||||
reference on append costs it nothing;
|
||||
- the reap-stale `$effect` now tests membership against that Map instead of a
|
||||
fresh `Set`, and a stable reference on append means it **doesn't re-run** there
|
||||
(nothing to reap) while a rebuild — the delete/reload case — yields a new
|
||||
reference and **does** re-run it, precisely when stale ids must be dropped.
|
||||
|
||||
Gates (`round18.bench.test.ts`): the builder is asserted deep-equal to the
|
||||
verbatim `buildItemIndex` reference at **every** page of a 12-page drain (and on
|
||||
the final index of a 20-page one), a later duplicate id resolves to its highest
|
||||
index (matching `Map`'s last-wins), and the reference-contract gate pins
|
||||
same-ref-on-append / new-ref-on-rebuild. The perf gate requires the incremental
|
||||
drain to beat the rebuild-per-page by ≥5×.
|
||||
|
||||
Measured: a 40-page × 50-item drain builds the index in **6.4 ms vs 74.1 ms —
|
||||
11.5× faster** — and no longer churns a fresh Map + id-Set per page.
|
||||
|
||||
## Not shipped — deferred to a later round
|
||||
|
||||
Surfaced during the Round-18 audit but not landed (each needs its own decision,
|
||||
fixture, or a different reactivity treatment):
|
||||
|
||||
- **Frontend — flat dotfile filter (`ResourceList` inline `items.filter` +
|
||||
`utils/dotfileFilter::filterDotfiles`, on photos):** the ROUND17 note flagged
|
||||
it as an O(N²) per-page rescan when *hide dotfiles* is on. Unlike the favorites
|
||||
`Set` (ROUND14 §F2) or this round's id-`Map`, the filtered result is a **flat
|
||||
array** that feeds `.filter`/rendering — reactivity needs a *fresh* array
|
||||
reference each page, and building one by `prev.concat(freshFiltered)` is itself
|
||||
O(N) (a mutate-in-place same-reference array would stop `photoRows` /
|
||||
`visibleItems` consumers from recomputing). There is no clean O(N)→O(page) win
|
||||
without partitioning the list; deferred pending a design (e.g. filtering per
|
||||
page in the loader and accumulating in page state, which changes the
|
||||
toggle-refilter semantics).
|
||||
- **Frontend — `VirtualRows.offsets` prefix-sum (photos timeline):** rebuilt in
|
||||
full per page. An incremental prefix-sum needs (a) a version-counter to force
|
||||
`band`/`totalHeight` to recompute without a fresh `offsets` array reference
|
||||
(Svelte deriveds short-circuit on `===`), and (b) `PhotoTimeline` to guarantee
|
||||
row-object identity at the append boundary (a page that grows the last group
|
||||
re-lays-out its trailing strip row, breaking `isAppendExtension`). Both are
|
||||
real but want their own round; the raw numeric prefix-sum is also cheap (rows ≪
|
||||
photos), so this is lower-priority than the item-list rescans.
|
||||
- **Backend query-shape (needs Postgres, carried from ROUND17):**
|
||||
`music_storage_adapter::list_public_playlists` 1 + N `COUNT(*)` fold; contact
|
||||
REST listings over-fetch the multi-KB `vcard` TEXT the `ContactDto` mappers
|
||||
never read (wants a *lite* row mapper).
|
||||
|
||||
## Environment / methodology
|
||||
|
||||
- `cargo run --release --features bench --example bench_round18_micro` —
|
||||
counting global allocator, no Postgres. Tunable: `BENCH_ITERS` (200000).
|
||||
- `cd frontend && npx vitest run src/lib/components/round18.bench.test.ts` —
|
||||
equivalence, reference-contract, and wall-time perf gates.
|
||||
- Each section is BEFORE (verbatim replica of the shipped-before shape) vs AFTER
|
||||
(verbatim replica of the shipped-after shape) with a byte/-value equivalence
|
||||
gate; the shipped source now matches each AFTER arm.
|
||||
- Roll-back rule encoded per section: the Rust harness `std::process::exit(1)`s
|
||||
with `GATE FAIL … rollback` if an AFTER arm fails to reduce allocations; the
|
||||
vitest perf gate fails the test if the incremental arm isn't ≥5× faster.
|
||||
@@ -0,0 +1,331 @@
|
||||
//! Round-18 calendar-event edit CPU/alloc micro-pack (no Postgres).
|
||||
//!
|
||||
//! Same rule as ROUND2–17: each section is BEFORE (verbatim replica of the
|
||||
//! shipped-before shape) vs AFTER (verbatim replica of the shipped-after
|
||||
//! shape), with a byte-for-byte equivalence gate and a `GATE FAIL … rollback`
|
||||
//! check that exits non-zero if the AFTER arm fails to reduce allocations — the
|
||||
//! round's roll-back rule encoded into the benchmark.
|
||||
//!
|
||||
//! [C1] `CalendarEvent::update_ical_property` / `remove_ical_property`
|
||||
//! rewrote the ENTIRE `ical_data` body with `format!("{}{}{}")` on every
|
||||
//! call, and allocated TWO search needles per call (`\nNAME:` and the
|
||||
//! redundant `\r\nNAME:` — the CRLF form can never match where the LF
|
||||
//! form doesn't, since `\nNAME:` is its suffix). Because
|
||||
//! `calendar_storage_adapter::update_event` applies each changed field
|
||||
//! independently, a multi-field REST edit paid one full-body (up to
|
||||
//! ~11 KB) String allocation PER changed property, plus two needles.
|
||||
//! The shipped-after form mutates the body in place (`replace_range` for
|
||||
//! an existing property, four `insert`/`insert_str` for a new one) and
|
||||
//! builds the single `\nNAME:` needle on the stack — zero heap needle,
|
||||
//! no fresh-body allocation. The edited spans are byte-for-byte the same
|
||||
//! the `format!` reconstruction produced (`replace_range(a..b, v)` ≡
|
||||
//! `before + v + after`), so the emitted body is identical — including
|
||||
//! the pre-existing quirk that editing a CRLF line drops its `\r`.
|
||||
//!
|
||||
//! Run:
|
||||
//! cargo run --release --features bench --example bench_round18_micro
|
||||
//! Tunables (env): BENCH_ITERS (200000)
|
||||
|
||||
use std::alloc::{GlobalAlloc, Layout, System};
|
||||
use std::env;
|
||||
use std::hint::black_box;
|
||||
use std::sync::atomic::{AtomicU64, Ordering};
|
||||
use std::time::Instant;
|
||||
|
||||
static ALLOC_CALLS: AtomicU64 = AtomicU64::new(0);
|
||||
|
||||
struct CountingAlloc;
|
||||
|
||||
unsafe impl GlobalAlloc for CountingAlloc {
|
||||
unsafe fn alloc(&self, layout: Layout) -> *mut u8 {
|
||||
ALLOC_CALLS.fetch_add(1, Ordering::Relaxed);
|
||||
unsafe { System.alloc(layout) }
|
||||
}
|
||||
unsafe fn dealloc(&self, ptr: *mut u8, layout: Layout) {
|
||||
unsafe { System.dealloc(ptr, layout) }
|
||||
}
|
||||
unsafe fn realloc(&self, ptr: *mut u8, layout: Layout, new_size: usize) -> *mut u8 {
|
||||
ALLOC_CALLS.fetch_add(1, Ordering::Relaxed);
|
||||
unsafe { System.realloc(ptr, layout, new_size) }
|
||||
}
|
||||
unsafe fn alloc_zeroed(&self, layout: Layout) -> *mut u8 {
|
||||
ALLOC_CALLS.fetch_add(1, Ordering::Relaxed);
|
||||
unsafe { System.alloc_zeroed(layout) }
|
||||
}
|
||||
}
|
||||
|
||||
#[global_allocator]
|
||||
static GLOBAL: CountingAlloc = CountingAlloc;
|
||||
|
||||
fn env_or<T: std::str::FromStr>(key: &str, default: T) -> T {
|
||||
env::var(key)
|
||||
.ok()
|
||||
.and_then(|v| v.parse().ok())
|
||||
.unwrap_or(default)
|
||||
}
|
||||
|
||||
struct Measured {
|
||||
wall_ns_per_op: f64,
|
||||
allocs_per_op: f64,
|
||||
}
|
||||
|
||||
fn measure<F: FnMut()>(iters: usize, mut f: F) -> Measured {
|
||||
let a0 = ALLOC_CALLS.load(Ordering::Relaxed);
|
||||
let t = Instant::now();
|
||||
for _ in 0..iters {
|
||||
f();
|
||||
}
|
||||
let wall = t.elapsed().as_nanos() as f64 / iters as f64;
|
||||
let allocs = (ALLOC_CALLS.load(Ordering::Relaxed) - a0) as f64 / iters as f64;
|
||||
Measured {
|
||||
wall_ns_per_op: wall,
|
||||
allocs_per_op: allocs,
|
||||
}
|
||||
}
|
||||
|
||||
fn print_row(label: &str, m: &Measured) {
|
||||
println!(
|
||||
"| {:<44} | {:>12.1} | {:>10.2} |",
|
||||
label, m.wall_ns_per_op, m.allocs_per_op
|
||||
);
|
||||
}
|
||||
|
||||
fn header_footer(name: &str, before: &Measured, after: &Measured) {
|
||||
println!("| arm | ns/op | allocs/op |");
|
||||
print_row(&format!("BEFORE {name}"), before);
|
||||
print_row(&format!("AFTER {name}"), after);
|
||||
println!(
|
||||
"# {:.2}x wall, {:.2} fewer allocs/op",
|
||||
before.wall_ns_per_op / after.wall_ns_per_op,
|
||||
before.allocs_per_op - after.allocs_per_op
|
||||
);
|
||||
}
|
||||
|
||||
fn gate_allocs(tag: &str, before: &Measured, after: &Measured) {
|
||||
if after.allocs_per_op >= before.allocs_per_op {
|
||||
eprintln!("GATE FAIL [{tag}]: AFTER did not reduce allocations — rollback");
|
||||
std::process::exit(1);
|
||||
}
|
||||
}
|
||||
|
||||
// ────────────────────────────────────────────────────────────────────────────
|
||||
// [C1] calendar-event edit — per-property full-body format! vs in-place edit
|
||||
// ────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
/// An edit step, mirroring the `event.update_*(…)` calls that
|
||||
/// `calendar_storage_adapter::update_event` fans out into `update_ical_property`
|
||||
/// (present → replace) and `remove_ical_property` (a cleared `Option`).
|
||||
enum Op<'a> {
|
||||
Update(&'a str, &'a str),
|
||||
Remove(&'a str),
|
||||
}
|
||||
|
||||
// ── BEFORE: verbatim replica of the shipped-before methods ──────────────────
|
||||
|
||||
fn before_update(ical: &mut String, property_name: &str, value: &str) {
|
||||
let search_str = format!("\n{}:", property_name);
|
||||
let search_str_alt = format!("\r\n{}:", property_name);
|
||||
|
||||
let pos = ical
|
||||
.find(&search_str)
|
||||
.or_else(|| ical.find(&search_str_alt));
|
||||
|
||||
if let Some(pos) = pos {
|
||||
let value_start = pos + search_str.len();
|
||||
let value_end = ical[value_start..]
|
||||
.find('\n')
|
||||
.map(|p| value_start + p)
|
||||
.unwrap_or_else(|| ical.len());
|
||||
let before = &ical[..value_start];
|
||||
let after = &ical[value_end..];
|
||||
*ical = format!("{}{}{}", before, value, after);
|
||||
} else {
|
||||
let end_pos = ical.find("END:VEVENT").unwrap_or(ical.len());
|
||||
let before = &ical[..end_pos];
|
||||
let after = &ical[end_pos..];
|
||||
*ical = format!("{}{}:{}\n{}", before, property_name, value, after);
|
||||
}
|
||||
}
|
||||
|
||||
fn before_remove(ical: &mut String, property_name: &str) {
|
||||
let search_str = format!("\n{}:", property_name);
|
||||
let search_str_alt = format!("\r\n{}:", property_name);
|
||||
|
||||
let pos = ical
|
||||
.find(&search_str)
|
||||
.or_else(|| ical.find(&search_str_alt));
|
||||
|
||||
if let Some(pos) = pos {
|
||||
let value_end = ical[pos + 1..]
|
||||
.find('\n')
|
||||
.map(|p| pos + 1 + p)
|
||||
.unwrap_or_else(|| ical.len());
|
||||
let before = &ical[..pos];
|
||||
let after = &ical[value_end..];
|
||||
*ical = format!("{}{}", before, after);
|
||||
}
|
||||
}
|
||||
|
||||
// ── AFTER: verbatim replica of the shipped-after methods ────────────────────
|
||||
|
||||
fn line_needle<'a>(buf: &'a mut [u8; 64], name: &str) -> Option<&'a str> {
|
||||
let n = name.len();
|
||||
if n + 2 > buf.len() {
|
||||
return None;
|
||||
}
|
||||
buf[0] = b'\n';
|
||||
buf[1..1 + n].copy_from_slice(name.as_bytes());
|
||||
buf[1 + n] = b':';
|
||||
std::str::from_utf8(&buf[..n + 2]).ok()
|
||||
}
|
||||
|
||||
fn after_update(ical: &mut String, property_name: &str, value: &str) {
|
||||
let mut buf = [0u8; 64];
|
||||
let needle_owned;
|
||||
let needle: &str = match line_needle(&mut buf, property_name) {
|
||||
Some(n) => n,
|
||||
None => {
|
||||
needle_owned = format!("\n{property_name}:");
|
||||
&needle_owned
|
||||
}
|
||||
};
|
||||
|
||||
if let Some(pos) = ical.find(needle) {
|
||||
let value_start = pos + needle.len();
|
||||
let value_end = ical[value_start..]
|
||||
.find('\n')
|
||||
.map_or(ical.len(), |p| value_start + p);
|
||||
ical.replace_range(value_start..value_end, value);
|
||||
} else {
|
||||
let end_pos = ical.find("END:VEVENT").unwrap_or(ical.len());
|
||||
ical.reserve(property_name.len() + value.len() + 2);
|
||||
ical.insert(end_pos, '\n');
|
||||
ical.insert_str(end_pos, value);
|
||||
ical.insert(end_pos, ':');
|
||||
ical.insert_str(end_pos, property_name);
|
||||
}
|
||||
}
|
||||
|
||||
fn after_remove(ical: &mut String, property_name: &str) {
|
||||
let mut buf = [0u8; 64];
|
||||
let needle_owned;
|
||||
let needle: &str = match line_needle(&mut buf, property_name) {
|
||||
Some(n) => n,
|
||||
None => {
|
||||
needle_owned = format!("\n{property_name}:");
|
||||
&needle_owned
|
||||
}
|
||||
};
|
||||
|
||||
if let Some(pos) = ical.find(needle) {
|
||||
let value_end = ical[pos + 1..]
|
||||
.find('\n')
|
||||
.map_or(ical.len(), |p| pos + 1 + p);
|
||||
ical.replace_range(pos..value_end, "");
|
||||
}
|
||||
}
|
||||
|
||||
fn apply_before(base: &str, ops: &[Op]) -> String {
|
||||
let mut ical = base.to_string();
|
||||
for op in ops {
|
||||
match op {
|
||||
Op::Update(n, v) => before_update(&mut ical, n, v),
|
||||
Op::Remove(n) => before_remove(&mut ical, n),
|
||||
}
|
||||
}
|
||||
ical
|
||||
}
|
||||
|
||||
fn apply_after(base: &str, ops: &[Op]) -> String {
|
||||
let mut ical = base.to_string();
|
||||
for op in ops {
|
||||
match op {
|
||||
Op::Update(n, v) => after_update(&mut ical, n, v),
|
||||
Op::Remove(n) => after_remove(&mut ical, n),
|
||||
}
|
||||
}
|
||||
ical
|
||||
}
|
||||
|
||||
/// A realistic stored VEVENT body (CRLF-terminated, ~1.5 KB with attendees and
|
||||
/// a VALARM) — the shape `calendar_storage_adapter` hydrates before applying an
|
||||
/// `UpdateEventDto`.
|
||||
fn base_body() -> String {
|
||||
let mut b = String::from(
|
||||
"BEGIN:VCALENDAR\r\nVERSION:2.0\r\nPRODID:-//OxiCloud//NONSGML Calendar//EN\r\n\
|
||||
BEGIN:VEVENT\r\nUID:evt-round18@oxicloud.test\r\nDTSTAMP:20260101T100000Z\r\n\
|
||||
DTSTART:20260101T120000Z\r\nDTEND:20260101T130000Z\r\n\
|
||||
SUMMARY:Original quarterly planning sync\r\n\
|
||||
DESCRIPTION:The original description body for the event, moderately long.\r\n\
|
||||
LOCATION:Room A, Ground Floor\r\nCATEGORIES:work,planning,quarterly\r\n\
|
||||
ORGANIZER;CN=Alice Example:mailto:alice@oxicloud.test\r\n",
|
||||
);
|
||||
// A handful of attendees + a VALARM to bring the body to a realistic size,
|
||||
// so BEFORE's per-property full-body `format!` copies real bytes.
|
||||
for i in 0..8 {
|
||||
b.push_str(&format!(
|
||||
"ATTENDEE;CN=Guest {i};PARTSTAT=NEEDS-ACTION;RSVP=TRUE:mailto:guest{i}@oxicloud.test\r\n"
|
||||
));
|
||||
}
|
||||
b.push_str(
|
||||
"BEGIN:VALARM\r\nACTION:DISPLAY\r\nDESCRIPTION:Reminder\r\nTRIGGER:-PT15M\r\nEND:VALARM\r\n\
|
||||
END:VEVENT\r\nEND:VCALENDAR\r\n",
|
||||
);
|
||||
b
|
||||
}
|
||||
|
||||
fn section_calendar_edit() {
|
||||
let iters: usize = env_or("BENCH_ITERS", 200_000);
|
||||
let base = base_body();
|
||||
|
||||
// A full multi-field REST edit: five existing properties replaced (SUMMARY,
|
||||
// DESCRIPTION, LOCATION, DTSTART, DTEND — the last two rewritten twice, as
|
||||
// the time-range + all-day updates both do), one new property inserted
|
||||
// (RRULE, absent from the body), one cleared (CATEGORIES). Exactly the
|
||||
// `update_ical_property` / `remove_ical_property` fan-out of `update_event`.
|
||||
let ops = [
|
||||
Op::Update("SUMMARY", "Updated quarterly planning sync"),
|
||||
Op::Update(
|
||||
"DESCRIPTION",
|
||||
"A revised, noticeably longer description so the replacement value differs in length from the original and exercises the grow path.",
|
||||
),
|
||||
Op::Update("LOCATION", "Conference Room 42, Building B"),
|
||||
Op::Update("DTSTART", "20260202T090000Z"),
|
||||
Op::Update("DTEND", "20260202T100000Z"),
|
||||
Op::Update("DTSTART", "20260202T000000Z"),
|
||||
Op::Update("DTEND", "20260202T010000Z"),
|
||||
Op::Update("RRULE", "FREQ=WEEKLY;COUNT=10"),
|
||||
Op::Remove("CATEGORIES"),
|
||||
];
|
||||
|
||||
// Equivalence gate: the emitted body is byte-for-byte identical.
|
||||
let b = apply_before(&base, &ops);
|
||||
let a = apply_after(&base, &ops);
|
||||
assert_eq!(b, a, "C1 emitted body differs between BEFORE and AFTER");
|
||||
|
||||
let before = measure(iters, || {
|
||||
black_box(apply_before(black_box(&base), black_box(&ops)));
|
||||
});
|
||||
let after = measure(iters, || {
|
||||
black_box(apply_after(black_box(&base), black_box(&ops)));
|
||||
});
|
||||
|
||||
println!(
|
||||
"\n## [C1] calendar-event multi-field edit ({} ops, {}-byte body)",
|
||||
ops.len(),
|
||||
base.len()
|
||||
);
|
||||
println!("# both arms pay one identical `base.to_string()` reset per op (constant, shared)");
|
||||
header_footer("update_event in-place property rewrite", &before, &after);
|
||||
gate_allocs("C1", &before, &after);
|
||||
}
|
||||
|
||||
fn main() {
|
||||
println!("#################################################################");
|
||||
println!("# Round-18 calendar-event edit CPU/alloc micro-pack");
|
||||
println!("#################################################################");
|
||||
|
||||
section_calendar_edit();
|
||||
|
||||
println!("\nGATE PASS (all sections)");
|
||||
}
|
||||
@@ -79,6 +79,7 @@
|
||||
import { formatDate, iconNameFromClass, fileIconKindClass } from '$lib/utils/display';
|
||||
import { gridColumns } from '$lib/utils/grid';
|
||||
import { ResourceSectionsBuilder } from '$lib/utils/resourceSections';
|
||||
import { ItemIndexBuilder } from '$lib/utils/itemIndex';
|
||||
import { fileThumbnailUrl, thumbSizeForView } from '$lib/api/endpoints/files';
|
||||
import {
|
||||
canThumbnailClientSide,
|
||||
@@ -468,13 +469,18 @@
|
||||
// stale-selection cleanup only fires when items truly leave the
|
||||
// dataset (reload, delete, etc.), not when the filter hides them.
|
||||
//
|
||||
// Index rebuilt only when `items` changes; the projection is then
|
||||
// O(k · log k) in the selection size k instead of a full O(N) re-scan
|
||||
// of the list on every toggle (O(N²)-ish across a shift-range gesture
|
||||
// once the batch toolbar is mounted — benches/ROUND11.md §S1). The
|
||||
// index sort preserves item order, so the toolbar sees the same array
|
||||
// the old filter produced.
|
||||
const itemIndexById = $derived(new Map(items.map((i, idx) => [i.id, idx])));
|
||||
// Index extended over the freshly-appended page only (never re-scanned in
|
||||
// full) via `ItemIndexBuilder`: an infinite-scroll drain with a selection
|
||||
// active collapses from Σ O(N²) Map rebuilds to O(N) total, and the Map
|
||||
// reference is reused across appends so the reap-stale effect below no
|
||||
// longer re-fires (nor re-allocates an O(N) id Set) on a page that removed
|
||||
// nothing — its reference only changes on a rebuild (reload / deletion),
|
||||
// exactly when a reap is warranted. The projection is then O(k · log k) in
|
||||
// the selection size k, not a full O(N) re-scan on every toggle
|
||||
// (benches/ROUND11.md §S1, benches/ROUND18.md §F1). The index sort preserves
|
||||
// item order, so the toolbar sees the same array the old filter produced.
|
||||
const itemIndex = new ItemIndexBuilder<FileItem | FolderItem>();
|
||||
const itemIndexById = $derived(itemIndex.sync(items));
|
||||
const selectedItems = $derived.by(() => {
|
||||
const picked: { idx: number; item: FileItem | FolderItem }[] = [];
|
||||
for (const id of selected) {
|
||||
@@ -487,15 +493,20 @@
|
||||
|
||||
// Drop selection ids that are no longer present after a reload.
|
||||
$effect(() => {
|
||||
// With nothing selected (the common case) every infinite-scroll page
|
||||
// re-fired this effect and built a throwaway O(N) id Set for a loop
|
||||
// that never runs — skip straight out. `selected.size` is reactive,
|
||||
// so the effect re-fires when a selection appears.
|
||||
// With nothing selected (the common case) the loop never runs — skip
|
||||
// straight out. `selected.size` is reactive, so the effect re-fires
|
||||
// when a selection appears.
|
||||
if (selected.size === 0) return;
|
||||
const ids = new Set(items.map((i) => i.id));
|
||||
// Test membership against the incremental `itemIndexById` rather than a
|
||||
// throwaway O(N) id Set rebuilt per page. Its reference is stable across
|
||||
// infinite-scroll appends (which never remove an id — nothing to reap)
|
||||
// so this effect no longer re-fires on every page; the reference changes
|
||||
// only on a rebuild (reload / deletion), which is exactly when a stale
|
||||
// selection must be dropped (benches/ROUND18.md §F1).
|
||||
const index = itemIndexById;
|
||||
let changed = false;
|
||||
for (const id of selected) {
|
||||
if (!ids.has(id)) {
|
||||
if (!index.has(id)) {
|
||||
selected.delete(id);
|
||||
changed = true;
|
||||
}
|
||||
|
||||
@@ -0,0 +1,134 @@
|
||||
// Round-18 frontend micro-pack (benches/ROUND18.md §F1).
|
||||
//
|
||||
// Each section is BEFORE (verbatim replica of the shipped-before shape) vs
|
||||
// AFTER (the shipped incremental builder), with an equivalence gate, a
|
||||
// reference-contract gate, and a wall-time perf gate — the same discipline as
|
||||
// the Rust micro-packs: an AFTER that doesn't beat its BEFORE fails the gate.
|
||||
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import { buildItemIndex, ItemIndexBuilder } from '$lib/utils/itemIndex';
|
||||
|
||||
// ────────────────────────────────────────────────────────────────────────────
|
||||
// [F1] ResourceList `itemIndexById` — rebuild-a-fresh-Map-per-page vs incremental
|
||||
// ────────────────────────────────────────────────────────────────────────────
|
||||
//
|
||||
// Audit finding (ROUND17 deferred list): ResourceList derived
|
||||
// `itemIndexById = new Map(items.map((i, idx) => [i.id, idx]))`. Every
|
||||
// infinite-scroll page (`items = [...items, ...page]`) rebuilt a brand-new Map
|
||||
// over the WHOLE accumulated list — O(N) per page, Σ O(N²) across a P-page
|
||||
// drain — and, being a fresh instance each page, re-fired the reap-stale
|
||||
// `$effect` that reference-diffs it (allocating another O(N) id Set for a reap
|
||||
// an append can never trigger). `ItemIndexBuilder` extends the persistent Map
|
||||
// with the fresh page only and returns the same reference on an append.
|
||||
|
||||
interface Item {
|
||||
id: string;
|
||||
}
|
||||
|
||||
/** A page of `{ id }` items (50/page, the default page size). */
|
||||
function pageOf(start: number, n: number): Item[] {
|
||||
return Array.from({ length: n }, (_, i) => ({ id: `it-${start + i}` }));
|
||||
}
|
||||
|
||||
/** BEFORE: rebuild a fresh Map over the whole accumulated list each page. */
|
||||
function rebuildPerPage(pages: Item[][]): Map<string, number> {
|
||||
let acc: Item[] = [];
|
||||
let index = new Map<string, number>();
|
||||
for (const page of pages) {
|
||||
acc = [...acc, ...page]; // the component's `items = [...items, ...page]`
|
||||
index = new Map(acc.map((i, idx) => [i.id, idx])); // new instance + O(N) rebuild
|
||||
}
|
||||
return index;
|
||||
}
|
||||
|
||||
/** AFTER: one persistent builder, extend with only the fresh page's ids. */
|
||||
function incrementalPerPage(pages: Item[][]): Map<string, number> {
|
||||
const builder = new ItemIndexBuilder<Item>();
|
||||
let acc: Item[] = [];
|
||||
let index = new Map<string, number>();
|
||||
for (const page of pages) {
|
||||
acc = [...acc, ...page];
|
||||
index = builder.sync(acc);
|
||||
}
|
||||
return index;
|
||||
}
|
||||
|
||||
describe('round18 §F1 — ResourceList itemIndexById incremental Map', () => {
|
||||
it('final index is identical to the full rebuild (equivalence gate)', () => {
|
||||
const pages = Array.from({ length: 20 }, (_, p) => pageOf(p * 50, 50));
|
||||
const acc = pages.flat();
|
||||
const before = rebuildPerPage(pages);
|
||||
const after = incrementalPerPage(pages);
|
||||
const reference = buildItemIndex(acc);
|
||||
expect(after.size).toBe(before.size);
|
||||
for (const [id, idx] of reference) expect(after.get(id)).toBe(idx);
|
||||
for (const [id, idx] of after) expect(before.get(id)).toBe(idx);
|
||||
});
|
||||
|
||||
it('the index stays deep-equal to the reference at EVERY page (equivalence gate)', () => {
|
||||
const builder = new ItemIndexBuilder<Item>();
|
||||
let acc: Item[] = [];
|
||||
for (let p = 0; p < 12; p++) {
|
||||
acc = [...acc, ...pageOf(p * 50, 50)];
|
||||
const got = builder.sync(acc);
|
||||
const want = buildItemIndex(acc);
|
||||
expect(got.size).toBe(want.size);
|
||||
for (const [id, idx] of want) expect(got.get(id)).toBe(idx);
|
||||
}
|
||||
});
|
||||
|
||||
it('a later duplicate id resolves to its highest index, matching Map (equivalence gate)', () => {
|
||||
// The old `new Map(items.map(...))` keeps the last (highest-index)
|
||||
// occurrence of a duplicate id; the incremental extend must too.
|
||||
const builder = new ItemIndexBuilder<Item>();
|
||||
const dup: Item = { id: 'dup' };
|
||||
const p1 = [dup, { id: 'a' }];
|
||||
const p2 = [{ id: 'b' }, dup]; // 'dup' re-appears at index 3
|
||||
builder.sync(p1);
|
||||
const got = builder.sync([...p1, ...p2]);
|
||||
const want = buildItemIndex([...p1, ...p2]);
|
||||
expect(got.get('dup')).toBe(want.get('dup'));
|
||||
expect(got.get('dup')).toBe(3);
|
||||
});
|
||||
|
||||
it('reuses the Map reference on append, mints a new one on rebuild (reference-contract gate)', () => {
|
||||
const builder = new ItemIndexBuilder<Item>();
|
||||
const p1 = pageOf(0, 50);
|
||||
const first = builder.sync(p1);
|
||||
// Append: same reference (so the reap-stale effect does NOT re-fire —
|
||||
// an append removes nothing).
|
||||
const appended = builder.sync([...p1, ...pageOf(50, 50)]);
|
||||
expect(appended).toBe(first);
|
||||
// Deletion (shorter, non-append prefix): fresh reference (so the
|
||||
// reap-stale effect DOES re-fire and drops the removed id).
|
||||
const afterDelete = builder.sync(p1.slice(0, 40));
|
||||
expect(afterDelete).not.toBe(first);
|
||||
expect(afterDelete.has('it-49')).toBe(false);
|
||||
// Reload with a different first element (non-append): fresh reference.
|
||||
const reloaded = builder.sync(pageOf(1000, 50));
|
||||
expect(reloaded).not.toBe(afterDelete);
|
||||
});
|
||||
|
||||
it('a P-page drain builds the index ≥5x faster incrementally (perf gate)', () => {
|
||||
const PAGES = 40;
|
||||
const PER = 50; // 2 000 items total
|
||||
const pages = Array.from({ length: PAGES }, (_, p) => pageOf(p * PER, PER));
|
||||
|
||||
const run = (f: (p: Item[][]) => Map<string, number>): number => {
|
||||
const t0 = performance.now();
|
||||
for (let r = 0; r < 20; r++) f(pages);
|
||||
return performance.now() - t0;
|
||||
};
|
||||
|
||||
// Warm-up (JIT) then measure.
|
||||
run(rebuildPerPage);
|
||||
run(incrementalPerPage);
|
||||
const beforeMs = run(rebuildPerPage);
|
||||
const afterMs = run(incrementalPerPage);
|
||||
|
||||
console.info(
|
||||
`§F1 ${PAGES} pages × ${PER}: rebuild-per-page ${beforeMs.toFixed(1)} ms vs incremental ${afterMs.toFixed(1)} ms (${(beforeMs / afterMs).toFixed(1)}x)`
|
||||
);
|
||||
expect(afterMs).toBeLessThan(beforeMs / 5);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,83 @@
|
||||
/**
|
||||
* Incremental `id → position` index for `ResourceList`, extracted so the O(N²)
|
||||
* accumulation of its `itemIndexById` `$derived` (and the reap-stale effect's
|
||||
* per-page `new Set(items.map(…))`) can be replaced with an append-aware
|
||||
* builder — and unit/benchmark-tested off the Svelte reactive graph.
|
||||
*
|
||||
* `ResourceList` pages its list in via infinite scroll (`items = [...items,
|
||||
* ...page]`) and rebuilt `new Map(items.map((i, idx) => [i.id, idx]))` on every
|
||||
* page — O(N) per page, Σ ≈ O(N²) across a P-page drain, and a fresh Map each
|
||||
* page (so the reap-stale effect that reference-diffs it re-ran on every append
|
||||
* too, allocating another O(N) id Set for a reap that an append can never
|
||||
* trigger). This is the same class ROUND6 fixed for the files listing, ROUND14
|
||||
* §F2 for favorites, and ROUND15/16 for the grouped/shared lanes.
|
||||
*
|
||||
* Because a fresh page only ever *appends* (server order is stable; existing
|
||||
* rows keep their index), {@link ItemIndexBuilder} extends the persistent Map
|
||||
* with just the new tail on an append and returns the SAME Map reference; any
|
||||
* other change (reload, deletion, non-append) rebuilds into a NEW Map. That
|
||||
* reference contract is load-bearing for the two `ResourceList` consumers:
|
||||
*
|
||||
* - `selectedItems` re-derives on every `items` change regardless (it indexes
|
||||
* `items[idx]`), so it always reads the freshly-extended Map — a stable ref
|
||||
* on append costs it nothing.
|
||||
* - the reap-stale `$effect` reference-diffs the Map, so a stable ref on
|
||||
* append means it does NOT re-run there (an append never removes an id, so
|
||||
* there is nothing to reap), while a rebuild (delete / reload) yields a new
|
||||
* ref and DOES re-run it — exactly when stale selections must be dropped.
|
||||
*
|
||||
* The pure {@link buildItemIndex} is the verbatim reference (what the old
|
||||
* `itemIndexById` derive produced); the benchmark gate holds the builder equal
|
||||
* to it at every page.
|
||||
*/
|
||||
|
||||
import { isAppendExtension } from './appendExtension';
|
||||
|
||||
/** Minimal shape the index needs: a stable string `id`. */
|
||||
export interface HasId {
|
||||
id: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* Verbatim reference: the `Map<id, index>` the old `itemIndexById` `$derived`
|
||||
* produced — `new Map(items.map((i, idx) => [i.id, idx]))`. On a duplicate id
|
||||
* the highest index wins (last insertion), matching `Map`'s own semantics.
|
||||
*/
|
||||
export function buildItemIndex<T extends HasId>(items: readonly T[]): Map<string, number> {
|
||||
const index = new Map<string, number>();
|
||||
for (let i = 0; i < items.length; i++) index.set(items[i].id, i);
|
||||
return index;
|
||||
}
|
||||
|
||||
/**
|
||||
* Append-aware `id → index` builder. Call {@link sync} with the current item
|
||||
* list on every change; it detects the common case — the list grew by appending
|
||||
* a page — and indexes only the fresh tail, reusing the persistent Map (same
|
||||
* reference). Any other change rebuilds into a new Map, so the result is always
|
||||
* deep-equal to {@link buildItemIndex} and the reference changes exactly when a
|
||||
* reap-stale pass is warranted.
|
||||
*/
|
||||
export class ItemIndexBuilder<T extends HasId> {
|
||||
/** Last synced list — the append cursor and the append-detection baseline. */
|
||||
#items: readonly T[] = [];
|
||||
/** id → index; a stable reference across appends, a fresh one on rebuild. */
|
||||
#index = new Map<string, number>();
|
||||
|
||||
sync(items: readonly T[]): Map<string, number> {
|
||||
if (isAppendExtension(this.#items, items)) {
|
||||
// Append: the prefix is unchanged (existing ids keep their index), so
|
||||
// only the fresh tail needs indexing. A duplicate id in the tail
|
||||
// overwrites to its higher index — identical to the full rebuild's
|
||||
// last-wins. Same Map reference is returned (see the class doc).
|
||||
for (let i = this.#items.length; i < items.length; i++) {
|
||||
this.#index.set(items[i].id, i);
|
||||
}
|
||||
} else {
|
||||
// Reload / deletion / non-append / first run: rebuild into a NEW Map so
|
||||
// the reap-stale effect (which reference-diffs it) re-runs.
|
||||
this.#index = buildItemIndex(items);
|
||||
}
|
||||
this.#items = items;
|
||||
return this.#index;
|
||||
}
|
||||
}
|
||||
@@ -1092,40 +1092,70 @@ impl CalendarEvent {
|
||||
* @param property_name The name of the property to update
|
||||
* @param value The new value for the property
|
||||
*/
|
||||
/// Write `\n{name}:` into `buf` and return it as `&str`, or `None` if the
|
||||
/// name is too long to fit (unreachable for RFC 5545 property names, the
|
||||
/// longest of which — `LAST-MODIFIED`, `RECURRENCE-ID` — are 13 bytes).
|
||||
///
|
||||
/// The bare-LF form is deliberate: `\n{name}:` is a suffix of the CRLF form
|
||||
/// `\r\n{name}:`, so a single search for it matches a property line whether
|
||||
/// the body is LF- or CRLF-terminated and returns the LF offset either way
|
||||
/// — behaviour-identical to the old `find("\n..").or(find("\r\n.."))` (the
|
||||
/// CRLF needle could never match where the LF one didn't). Built on the
|
||||
/// stack: no per-call heap needle.
|
||||
fn line_needle<'a>(buf: &'a mut [u8; 64], name: &str) -> Option<&'a str> {
|
||||
let n = name.len();
|
||||
if n + 2 > buf.len() {
|
||||
return None;
|
||||
}
|
||||
buf[0] = b'\n';
|
||||
buf[1..1 + n].copy_from_slice(name.as_bytes());
|
||||
buf[1 + n] = b':';
|
||||
// `name` is valid UTF-8 and only ASCII bytes were added around it.
|
||||
std::str::from_utf8(&buf[..n + 2]).ok()
|
||||
}
|
||||
|
||||
fn update_ical_property(&mut self, property_name: &str, value: &str) {
|
||||
let search_str = format!("\n{}:", property_name);
|
||||
let search_str_alt = format!("\r\n{}:", property_name);
|
||||
let mut buf = [0u8; 64];
|
||||
let needle_owned;
|
||||
let needle: &str = match Self::line_needle(&mut buf, property_name) {
|
||||
Some(n) => n,
|
||||
None => {
|
||||
needle_owned = format!("\n{property_name}:");
|
||||
&needle_owned
|
||||
}
|
||||
};
|
||||
|
||||
// Check if property exists
|
||||
let pos = self
|
||||
.ical_data
|
||||
.find(&search_str)
|
||||
.or_else(|| self.ical_data.find(&search_str_alt));
|
||||
|
||||
if let Some(pos) = pos {
|
||||
// Find the start of the value
|
||||
let value_start = pos + search_str.len();
|
||||
|
||||
// Find the end of the value (next line or end of string)
|
||||
if let Some(pos) = self.ical_data.find(needle) {
|
||||
// Value spans from just after `\n{NAME}:` to the next LF (or EOF).
|
||||
let value_start = pos + needle.len();
|
||||
let value_end = self.ical_data[value_start..]
|
||||
.find('\n')
|
||||
.map(|p| value_start + p)
|
||||
.unwrap_or_else(|| self.ical_data.len());
|
||||
.map_or(self.ical_data.len(), |p| value_start + p);
|
||||
|
||||
// Replace the value
|
||||
let before = &self.ical_data[..value_start];
|
||||
let after = &self.ical_data[value_end..];
|
||||
self.ical_data = format!("{}{}{}", before, value, after);
|
||||
// In place: reuses the body's own buffer (growing it once only when
|
||||
// the new value is longer) instead of allocating a whole fresh body
|
||||
// String per property, as the old `format!("{}{}{}")` did — on a
|
||||
// multi-field edit that was one full-body (up to ~11 KB) allocation
|
||||
// per changed property.
|
||||
self.ical_data.replace_range(value_start..value_end, value);
|
||||
} else {
|
||||
// Property doesn't exist, add it before END:VEVENT
|
||||
// Property absent: insert `{NAME}:{value}\n` before END:VEVENT.
|
||||
let end_pos = self
|
||||
.ical_data
|
||||
.find("END:VEVENT")
|
||||
.unwrap_or(self.ical_data.len());
|
||||
|
||||
let before = &self.ical_data[..end_pos];
|
||||
let after = &self.ical_data[end_pos..];
|
||||
self.ical_data = format!("{}{}:{}\n{}", before, property_name, value, after);
|
||||
// Insert the four pieces at one point in reverse order so the result
|
||||
// is `{NAME}:{value}\n` before END:VEVENT — byte-identical to the old
|
||||
// `format!("{}{}:{}\n{}")` — without allocating a fresh body String
|
||||
// (nor a value-sized fragment). One `reserve` caps it at a single
|
||||
// grow; the shifted tail is just the trailing END:VEVENT/VCALENDAR.
|
||||
self.ical_data
|
||||
.reserve(property_name.len() + value.len() + 2);
|
||||
self.ical_data.insert(end_pos, '\n');
|
||||
self.ical_data.insert_str(end_pos, value);
|
||||
self.ical_data.insert(end_pos, ':');
|
||||
self.ical_data.insert_str(end_pos, property_name);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1135,26 +1165,25 @@ impl CalendarEvent {
|
||||
* @param property_name The name of the property to remove
|
||||
*/
|
||||
fn remove_ical_property(&mut self, property_name: &str) {
|
||||
let search_str = format!("\n{}:", property_name);
|
||||
let search_str_alt = format!("\r\n{}:", property_name);
|
||||
let mut buf = [0u8; 64];
|
||||
let needle_owned;
|
||||
let needle: &str = match Self::line_needle(&mut buf, property_name) {
|
||||
Some(n) => n,
|
||||
None => {
|
||||
needle_owned = format!("\n{property_name}:");
|
||||
&needle_owned
|
||||
}
|
||||
};
|
||||
|
||||
// Check if property exists
|
||||
let pos = self
|
||||
.ical_data
|
||||
.find(&search_str)
|
||||
.or_else(|| self.ical_data.find(&search_str_alt));
|
||||
|
||||
if let Some(pos) = pos {
|
||||
// Find the end of the value (next line or end of string)
|
||||
if let Some(pos) = self.ical_data.find(needle) {
|
||||
// Delete from the property's leading LF (`pos`) through the end of
|
||||
// its value line (exclusive of the next line's LF) — byte-identical
|
||||
// to the old `format!("{}{}", &data[..pos], &data[value_end..])`,
|
||||
// but in place, with no fresh body String.
|
||||
let value_end = self.ical_data[pos + 1..]
|
||||
.find('\n')
|
||||
.map(|p| pos + 1 + p)
|
||||
.unwrap_or_else(|| self.ical_data.len());
|
||||
|
||||
// Remove the property
|
||||
let before = &self.ical_data[..pos];
|
||||
let after = &self.ical_data[value_end..];
|
||||
self.ical_data = format!("{}{}", before, after);
|
||||
.map_or(self.ical_data.len(), |p| pos + 1 + p);
|
||||
self.ical_data.replace_range(pos..value_end, "");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user