perf: avoid cloning events in InMemoryEventStore::read_stream #363

New issue

Closed

opened 2026-04-12 11:56:15 -07:00 by jwilger-ai-bot · 1 comment

jwilger-ai-bot commented

2026-04-12 11:56:15 -07:00

(Migrated from github.com)

Problem

InMemoryEventStore::read_stream() clones every event during reads (event.clone() at eventcore-memory/src/lib.rs:110). For 1000 events, that's 1000 heap allocations just to read state.

Benchmark: read 1000 events takes 43 µs (22M elem/sec). The cloning overhead is a portion of this.

Proposed Solution

Store events as Arc<dyn Any + Send + Sync> instead of Box<dyn Any + Send + Sync>. Reads would clone the Arc (cheap pointer increment) rather than deep-cloning the event data.

Expected Impact

Minor — estimated 43 µs → ~20 µs for reading 1000 events. The in-memory store is primarily used for testing, so this is low priority.

Caveats

The in-memory store is designed for testing and development, not production workloads. This optimization matters only if the in-memory store is used for benchmarking other components where read overhead should be minimized.

Location

eventcore-memory/src/lib.rs — storage types and read_stream() method.

Benchmark Baseline

Run cargo bench -p eventcore-bench --bench store_operations -- 'store/read_stream/memory' to measure before/after.

## Problem `InMemoryEventStore::read_stream()` clones every event during reads (`event.clone()` at `eventcore-memory/src/lib.rs:110`). For 1000 events, that's 1000 heap allocations just to read state. Benchmark: read 1000 events takes 43 µs (22M elem/sec). The cloning overhead is a portion of this. ## Proposed Solution Store events as `Arc<dyn Any + Send + Sync>` instead of `Box<dyn Any + Send + Sync>`. Reads would clone the `Arc` (cheap pointer increment) rather than deep-cloning the event data. ## Expected Impact Minor — estimated 43 µs → ~20 µs for reading 1000 events. The in-memory store is primarily used for testing, so this is low priority. ## Caveats The in-memory store is designed for testing and development, not production workloads. This optimization matters only if the in-memory store is used for benchmarking other components where read overhead should be minimized. ## Location `eventcore-memory/src/lib.rs` — storage types and `read_stream()` method. ## Benchmark Baseline Run `cargo bench -p eventcore-bench --bench store_operations -- 'store/read_stream/memory'` to measure before/after.

jwilger added this to the 1.0.0 milestone

2026-06-13 05:44:58 -07:00

jwilger commented

2026-06-13 10:51:33 -07:00

Owner

Closing as obsoleted by #364 (streaming reads, ADR-0049).

The proposed optimization was to store events as Arc<dyn Any> instead of Box<dyn Any> so that reads clone a cheap pointer rather than deep-cloning event data. After #364, EventStore::read_stream yields owned events (Item = Result<E, EventStoreError>) one at a time. Producing each owned E from the type-erased stored value still requires a deep clone regardless of whether it is stored behind Box or Arc — Arc would only avoid the clone if the read API handed out shared events (Arc<E>), which we deliberately chose not to do (the streaming API yields owned events, consistent with the other backends that deserialize fresh owned events from JSON).

In addition, InMemoryEventStore is a test/development backend, not a production workload (as the issue itself notes), and #364 already removed the up-front full-Vec materialization that was the larger memory concern.

No code change delivers the intended win without changing the public read API to yield shared events, which is out of scope and undesirable. Resolving as won't-fix / obsolete.

Closing as obsoleted by #364 (streaming reads, ADR-0049). The proposed optimization was to store events as `Arc<dyn Any>` instead of `Box<dyn Any>` so that reads clone a cheap pointer rather than deep-cloning event data. After #364, `EventStore::read_stream` yields **owned** events (`Item = Result<E, EventStoreError>`) one at a time. Producing each owned `E` from the type-erased stored value still requires a deep clone regardless of whether it is stored behind `Box` or `Arc` — `Arc` would only avoid the clone if the read API handed out *shared* events (`Arc<E>`), which we deliberately chose not to do (the streaming API yields owned events, consistent with the other backends that deserialize fresh owned events from JSON). In addition, `InMemoryEventStore` is a test/development backend, not a production workload (as the issue itself notes), and #364 already removed the up-front full-Vec materialization that was the larger memory concern. No code change delivers the intended win without changing the public read API to yield shared events, which is out of scope and undesirable. Resolving as won't-fix / obsolete.

jwilger closed this issue

2026-06-13 10:51:33 -07:00