Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

73 changes: 73 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ SHELL := /bin/bash

# Default docs port; override with: make docs PORT=5180
PORT ?= 5173
BENCH_PORT ?= 8000

.PHONY: hooks docs docs-build docs-ci echo-total
hooks:
Expand Down Expand Up @@ -43,3 +44,75 @@ docs-ci:
echo-total:
@chmod +x scripts/gen-echo-total.sh
@./scripts/gen-echo-total.sh
# Benchmarks and reports
.PHONY: bench-report vendor-d3 bench-serve bench-open

vendor-d3:
@mkdir -p docs/benchmarks/vendor
@if [ ! -f docs/benchmarks/vendor/d3.v7.min.js ]; then \
echo "Downloading D3 v7 to docs/benchmarks/vendor..."; \
curl -fsSL https://unpkg.com/d3@7/dist/d3.min.js -o docs/benchmarks/vendor/d3.v7.min.js; \
echo "D3 saved to docs/benchmarks/vendor/d3.v7.min.js"; \
else \
echo "D3 already present (docs/benchmarks/vendor/d3.v7.min.js)"; \
fi

bench-serve:
@echo "Serving repo at http://localhost:$(BENCH_PORT) (Ctrl+C to stop)"
@python3 -m http.server $(BENCH_PORT)

bench-open:
@open "http://localhost:$(BENCH_PORT)/docs/benchmarks/"

bench-report: vendor-d3
@echo "Running benches (rmg-benches)..."
cargo bench -p rmg-benches
@echo "Starting local server on :$(BENCH_PORT) and opening dashboard..."
@mkdir -p target
@if [ -f target/bench_http.pid ] && ps -p $$(cat target/bench_http.pid) >/dev/null 2>&1; then \
echo "[bench] Stopping previous server (pid $$(cat target/bench_http.pid))"; \
kill $$(cat target/bench_http.pid) >/dev/null 2>&1 || true; \
rm -f target/bench_http.pid; \
fi
@/bin/sh -c 'nohup python3 -m http.server $(BENCH_PORT) >/dev/null 2>&1 & echo $$! > target/bench_http.pid'
@echo "[bench] Waiting for server to become ready..."
@for i in {1..80}; do \
if curl -sSf "http://localhost:$(BENCH_PORT)/" >/dev/null ; then \
echo "[bench] Server is up at http://localhost:$(BENCH_PORT)/" ; \
break ; \
fi ; \
sleep 0.25 ; \
done
@open "http://localhost:$(BENCH_PORT)/docs/benchmarks/"

.PHONY: bench-status bench-stop

bench-status:
@if [ -f target/bench_http.pid ] && ps -p $$(cat target/bench_http.pid) >/dev/null 2>&1; then \
echo "[bench] Server running (pid $$(cat target/bench_http.pid)) at http://localhost:$(BENCH_PORT)"; \
else \
echo "[bench] Server not running"; \
fi

bench-stop:
@if [ -f target/bench_http.pid ]; then \
kill $$(cat target/bench_http.pid) >/dev/null 2>&1 || true; \
rm -f target/bench_http.pid; \
echo "[bench] Server stopped"; \
else \
echo "[bench] No PID file at target/bench_http.pid"; \
fi

.PHONY: bench-bake bench-open-inline

# Bake a standalone HTML with inline data that works over file://
bench-bake: vendor-d3
@echo "Running benches (rmg-benches)..."
cargo bench -p rmg-benches
@echo "Baking inline report..."
@python3 scripts/bench_bake.py --out docs/benchmarks/report-inline.html
@echo "Opening inline report..."
@open docs/benchmarks/report-inline.html

bench-open-inline:
@open docs/benchmarks/report-inline.html
11 changes: 9 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,15 @@ Echo is fundamentally **built different**.

RMG provides atomic, in-place edits of recursive meta-graphs with deterministic local scheduling and snapshot isolation. It’s the core of the Echo engine: runtime, assets, networking, and tools all operate on the same living graph of graphs.

Echo is a mathematically rigorous game engine that replaces traditional OOP with deterministic graph rewriting, enabling time-travel debugging, perfect
replay, and Git-like branching for game states.
Echo is a mathematically rigorous game engine that replaces traditional OOP with deterministic graph rewriting, enabling time-travel debugging, perfect replay, and Git-like branching for game states.

## Developer: Running Benchmarks

- Command (live dashboard): `make bench-report`
- Runs `cargo bench -p rmg-benches`, starts a local server, and opens the dashboard at `http://localhost:8000/docs/benchmarks/`.
- Command (offline static file): `make bench-bake`
- Runs benches and bakes `docs/benchmarks/report-inline.html` with results injected so it works over `file://` (no server required).
- Docs: see `crates/rmg-benches/benches/README.md` for details, tips, and report paths.

### Core Principles

Expand Down
4 changes: 2 additions & 2 deletions crates/rmg-benches/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ description = "Microbenchmarks for Echo (rmg-core): snapshot hashing and schedul
criterion = { version = "0.5", default-features = false, features = ["html_reports"] }
# Pin version alongside path to satisfy cargo-deny wildcard bans
rmg-core = { version = "0.1.0", path = "../rmg-core" }
# Minor-pin for semver compatibility; benches do not rely on a specific patch.
blake3 = "1.8"
# Exact pin and trimmed features to avoid rayon/parallelism in benches.
blake3 = { version = "=1.8.2", default-features = false, features = ["std"] }

[[bench]]
name = "motion_throughput"
Expand Down
11 changes: 9 additions & 2 deletions crates/rmg-benches/benches/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,12 @@ cargo bench -p rmg-benches --bench scheduler_drain

Criterion HTML reports are written under `target/criterion/<group>/report/index.html`.

### Charts & Reports

- Live server + dashboard: `make bench-report` opens `http://localhost:8000/docs/benchmarks/`.
- Offline static report (no server): `make bench-bake` writes `docs/benchmarks/report-inline.html` with results injected.
- Open the file directly (Finder or `open docs/benchmarks/report-inline.html`).

## Interpreting Results

- Use the throughput value to sanity‑check the scale of work per iteration.
Expand All @@ -48,7 +54,9 @@ Criterion HTML reports are written under `target/criterion/<group>/report/index.
## Environment Notes

- Toolchain: `stable` Rust (see `rust-toolchain.toml`).
- Dependency policy: avoid wildcards; benches use a minor pin for `blake3`.
- Dependency policy: avoid wildcards; benches use an exact patch pin for `blake3`
with trimmed features to avoid incidental parallelism:
`blake3 = { version = "=1.8.2", default-features = false, features = ["std"] }`.
- Repro: keep your machine under minimal background load; prefer `--quiet` and
close other apps.

Expand All @@ -62,4 +70,3 @@ cargo flamegraph -p rmg-benches --bench snapshot_hash -- --sample-size 50
```

These tools are not required for CI and are optional for local analysis.

56 changes: 56 additions & 0 deletions crates/rmg-benches/benches/reserve_scaling.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
#![allow(missing_docs)]
//! Benchmark: reserve() scaling with footprint size and number of reserved rewrites
//!
//! Measures how reserve() performance scales with:
//! 1. Number of previously reserved rewrites (k)
//! 2. Size of footprint being reserved (m)
//!
//! The current GenSet-based implementation should scale as O(m), independent of k.
//! A naive Vec<Footprint> implementation would scale as O(k × m).
use criterion::{criterion_group, criterion_main, BenchmarkId, Criterion, Throughput};
use rmg_core::Hash;
use std::time::Duration;

// Import the scheduler - it's crate-private so we can't access it directly
// Instead we'll use it through the Engine API
// Actually, we need direct access for this micro-benchmark, so we'll create
// a test module inside rmg-core and expose it via a feature flag or just
// write an integration test instead.

// For now, let's write a simpler benchmark that measures reserve through the Engine API

fn make_hash(val: u8) -> Hash {
let mut h = [0u8; 32];
h[0] = val;
h
}
Comment on lines +23 to +27
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Eliminate code duplication: reuse existing hash helper.

The make_hash function duplicates the h function from crates/rmg-core/src/scheduler.rs lines 535-539. This violates DRY and creates maintenance burden.

Solution: Extract the hash helper to a shared test utility module, or expose it through a #[cfg(test)] pub(crate) module in rmg-core that benches can access.

If the h function in scheduler.rs is moved to a test_utils module:

use rmg_core::test_utils::make_test_hash;

// Remove local make_hash, use shared version

Alternatively, define once in a benches/common/mod.rs:

// benches/common/mod.rs
pub fn make_hash(val: u8) -> rmg_core::Hash {
    let mut h = [0u8; 32];
    h[0] = val;
    h
}

Then import: use common::make_hash;

🤖 Prompt for AI Agents
In crates/rmg-benches/benches/reserve_scaling.rs around lines 23 to 27, the
local make_hash duplicates the h helper from crates/rmg-core/src/scheduler.rs;
remove this duplication by reusing a single helper: either move the helper into
a shared test utility (e.g. add a test_utils module in rmg-core and expose a
make_test_hash function via #[cfg(test)] pub(crate) so benches can import
rmg_core::test_utils::make_test_hash) or create a benches/common/mod.rs with a
public make_hash that returns rmg_core::Hash and import it in
reserve_scaling.rs; then delete the local make_hash and update the use/import to
reference the shared helper.


// Note: This benchmark requires access to DeterministicScheduler which is crate-private.
// Moving this to rmg-core/src/scheduler.rs tests module or using a pub(crate) test harness.

fn bench_reserve_scaling(_c: &mut Criterion) {
// This is a placeholder - the actual benchmark needs to be in rmg-core
// where we can access the scheduler directly.

// TODO: Implement this properly by either:
// 1. Adding a test-only public API to DeterministicScheduler
// 2. Moving this benchmark into rmg-core as a test module
// 3. Using Engine API indirectly (less precise)

let _ = (
BenchmarkId::new("placeholder", "reserve_scaling"),
Throughput::Elements(1),
make_hash(0),
);
}
Comment on lines +29 to +46
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Incomplete benchmark provides no value and pollutes the bench suite.

This placeholder benchmark:

  1. Doesn't measure anything (bench_reserve_scaling is empty)
  2. Executes on every cargo bench, wasting CI time
  3. Produces misleading benchmark output (timing of no-op)
  4. Contains only TODO comments

While scaffolding has value during development, merged code should be functional. The extensive comments indicate this work is blocked on API design decisions (exposing DeterministicScheduler internals).

Options:

  1. Recommended: Remove this file entirely and track the work in issue rmg-core: scheduler radix-drain performance tuning #122. Add it back when the implementation path is clear.

  2. Use #[ignore] attribute to prevent execution:

#[ignore = "Waiting for scheduler test API - see issue #122"]
fn bench_reserve_scaling(_c: &mut Criterion) {
    // ...
}
  1. Implement a minimal version using the Engine API (even if less precise), documenting the limitations.

Which approach aligns with your team's practices?

If you'd like, I can draft a minimal implementation using the Engine API approach, or generate an issue template for tracking this work.

🤖 Prompt for AI Agents
crates/rmg-benches/benches/reserve_scaling.rs lines 29-46: the file contains a
no-op placeholder benchmark that runs on every cargo bench and pollutes CI;
remove the file entirely (preferred) or, if you must keep it, mark the benchmark
to not run by adding an ignore attribute with a clear reason referencing issue
#122 (e.g., #[ignore = "Waiting for scheduler test API - see issue #122"]) and
leave a short TODO comment pointing to the issue; ensure CI and bench docs are
updated if you remove the file and add an issue reference in the PR description.


criterion_group! {
name = benches;
config = Criterion::default()
.warm_up_time(Duration::from_secs(1))
.measurement_time(Duration::from_secs(5))
.sample_size(50);
targets = bench_reserve_scaling
}
criterion_main!(benches);
53 changes: 48 additions & 5 deletions crates/rmg-benches/benches/scheduler_drain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use rmg_core::{
make_node_id, make_type_id, ApplyResult, ConflictPolicy, Engine, Footprint, Hash, NodeId,
NodeRecord, PatternGraph, RewriteRule,
};
use std::time::Duration;

// Bench constants to avoid magic strings.
const BENCH_NOOP_RULE_NAME: &str = "bench/noop";
Expand Down Expand Up @@ -70,29 +71,71 @@ fn build_engine_with_entities(n: usize) -> (Engine, Vec<NodeId>) {

fn bench_scheduler_drain(c: &mut Criterion) {
let mut group = c.benchmark_group("scheduler_drain");
for &n in &[10usize, 100, 1_000] {
// Stabilize CI runs: explicit warmup/measurement and sample size.
group
.warm_up_time(Duration::from_secs(3))
.measurement_time(Duration::from_secs(10))
.sample_size(60);
for &n in &[10usize, 100, 1_000, 3_000, 10_000, 30_000] {
// Throughput: number of rule applications in this run (n entities).
group.throughput(Throughput::Elements(n as u64));

// Full apply+commit cycle (original benchmark)
group.bench_with_input(BenchmarkId::from_parameter(n), &n, |b, &n| {
b.iter_batched(
|| build_engine_with_entities(n),
|(mut engine, ids)| {
// Apply the no-op rule to all entities, then commit.
let tx = engine.begin();
for id in &ids {
let res = engine
.apply(tx, BENCH_NOOP_RULE_NAME, id)
.expect("Failed to apply noop bench rule");
let res = engine.apply(tx, BENCH_NOOP_RULE_NAME, id).unwrap();
// Avoid affecting timing; check only in debug builds.
debug_assert!(matches!(res, ApplyResult::Applied));
}
let snap = engine.commit(tx).expect("Failed to commit benchmark tx");
let snap = engine.commit(tx).unwrap();
// Ensure the commit work is not optimized away.
criterion::black_box(snap);
},
BatchSize::PerIteration,
)
});

// Enqueue phase only (apply without commit)
group.bench_function(BenchmarkId::new("enqueue", n), |b| {
b.iter_batched(
|| build_engine_with_entities(n),
|(mut engine, ids)| {
let tx = engine.begin();
for id in &ids {
let res = engine.apply(tx, BENCH_NOOP_RULE_NAME, id).unwrap();
debug_assert!(matches!(res, ApplyResult::Applied));
}
criterion::black_box(tx);
},
BatchSize::PerIteration,
)
});

// Drain phase only (commit with pre-enqueued rewrites)
group.bench_function(BenchmarkId::new("drain", n), |b| {
b.iter_batched(
|| {
let (mut engine, ids) = build_engine_with_entities(n);
let tx = engine.begin();
// Pre-enqueue all rewrites (not timed)
for id in &ids {
let _ = engine.apply(tx, BENCH_NOOP_RULE_NAME, id).unwrap();
}
(engine, tx)
},
|(mut engine, tx)| {
// Only measure the commit (drain + execute)
let snap = engine.commit(tx).unwrap();
criterion::black_box(snap);
},
BatchSize::PerIteration,
)
});
}
group.finish();
}
Expand Down
10 changes: 8 additions & 2 deletions crates/rmg-benches/benches/snapshot_hash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use criterion::{criterion_group, criterion_main, BatchSize, BenchmarkId, Criteri
use rmg_core::{
make_edge_id, make_node_id, make_type_id, EdgeRecord, Engine, GraphStore, NodeRecord,
};
use std::time::Duration;

// String constants to avoid magic literals drifting silently.
const ROOT_ID_STR: &str = "root";
Expand Down Expand Up @@ -71,7 +72,12 @@ fn build_chain_engine(n: usize) -> Engine {

fn bench_snapshot_hash(c: &mut Criterion) {
let mut group = c.benchmark_group("snapshot_hash");
for &n in &[10usize, 100, 1_000] {
// Stabilize CI runs across environments.
group
.warm_up_time(Duration::from_secs(3))
.measurement_time(Duration::from_secs(10))
.sample_size(80);
for &n in &[10usize, 100, 1_000, 3_000, 10_000, 30_000] {
// Throughput: total nodes in reachable set (n entities + 1 root).
group.throughput(Throughput::Elements(n as u64 + 1));
group.bench_with_input(BenchmarkId::from_parameter(n), &n, |b, &n| {
Expand All @@ -82,7 +88,7 @@ fn bench_snapshot_hash(c: &mut Criterion) {
let snap = engine.snapshot();
criterion::black_box(snap.hash);
},
BatchSize::SmallInput,
BatchSize::PerIteration,
)
});
}
Expand Down
1 change: 1 addition & 0 deletions crates/rmg-core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ hex = { version = "0.4", optional = true }
serde = { version = "1.0", features = ["derive"], optional = true }
serde_json = { version = "1.0", optional = true }
once_cell = "1.19"
rustc-hash = "2.0"

[dev-dependencies]
serde = { version = "1.0", features = ["derive"] }
Expand Down
4 changes: 2 additions & 2 deletions crates/rmg-core/src/engine_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,8 +157,8 @@ impl Engine {
"missing compact rule id for a registered rule",
));
};
self.scheduler.pending.entry(tx).or_default().insert(
(scope_fp, rule.id),
self.scheduler.enqueue(
tx,
PendingRewrite {
rule_id: rule.id,
compact_rule,
Expand Down
8 changes: 8 additions & 0 deletions crates/rmg-core/src/footprint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ impl IdSet {
pub fn insert_edge(&mut self, id: &EdgeId) {
self.0.insert(id.0);
}
/// Returns an iterator over the identifiers in the set.
pub fn iter(&self) -> impl Iterator<Item = &Hash> {
self.0.iter()
}
Comment on lines +40 to +43
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Nice: deterministic iteration exposure for IdSet.

Consider also implementing IntoIterator for &IdSet so for h in &id_set works idiomatically.

 #[derive(Debug, Clone, Default)]
-pub struct IdSet(BTreeSet<Hash>);
+pub struct IdSet(BTreeSet<Hash>);
+
+impl<'a> IntoIterator for &'a IdSet {
+    type Item = &'a Hash;
+    type IntoIter = std::collections::btree_set::Iter<'a, Hash>;
+    fn into_iter(self) -> Self::IntoIter { self.0.iter() }
+}

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In crates/rmg-core/src/footprint.rs around lines 40–43, add an implementation of
IntoIterator for &IdSet so users can write `for h in &id_set`; implement
`impl<'a> IntoIterator for &'a IdSet` with Item = &Hash and IntoIter = the
iterator type returned by the internal set (use the concrete iter type, e.g. the
BTreeSet::Iter or equivalent) and have into_iter simply call the existing iter()
method, ensuring lifetimes and types match the file's imports.

/// Returns true if any element is shared with `other`.
pub fn intersects(&self, other: &Self) -> bool {
// Early‑exit by zipping ordered sets.
Expand Down Expand Up @@ -64,6 +68,10 @@ impl PortSet {
pub fn insert(&mut self, key: PortKey) {
let _ = self.0.insert(key);
}
/// Returns an iterator over the port keys in the set.
pub fn iter(&self) -> impl Iterator<Item = &PortKey> {
self.0.iter()
}
Comment on lines +71 to +74
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Likewise for PortSet: add IntoIterator for &PortSet.

This improves ergonomics without exposing internals.

 #[derive(Debug, Clone, Default)]
 pub struct PortSet(BTreeSet<PortKey>);
 
 impl PortSet {
@@
     pub fn iter(&self) -> impl Iterator<Item = &PortKey> {
         self.0.iter()
     }
 }
+
+impl<'a> IntoIterator for &'a PortSet {
+    type Item = &'a PortKey;
+    type IntoIter = std::collections::btree_set::Iter<'a, PortKey>;
+    fn into_iter(self) -> Self::IntoIter { self.0.iter() }
+}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/// Returns an iterator over the port keys in the set.
pub fn iter(&self) -> impl Iterator<Item = &PortKey> {
self.0.iter()
}
/// Returns an iterator over the port keys in the set.
pub fn iter(&self) -> impl Iterator<Item = &PortKey> {
self.0.iter()
}
}
impl<'a> IntoIterator for &'a PortSet {
type Item = &'a PortKey;
type IntoIter = std::collections::btree_set::Iter<'a, PortKey>;
fn into_iter(self) -> Self::IntoIter { self.0.iter() }
}
🤖 Prompt for AI Agents
In crates/rmg-core/src/footprint.rs around lines 71 to 74, add an implementation
of IntoIterator for &PortSet so taking a reference can be used in for-loops and
iterator contexts; implement IntoIterator for &'_ PortSet with type Item = &'_
PortKey and IntoIter = std::slice::Iter<'_, PortKey> (or the appropriate
iterator produced by self.0.iter()), and have into_iter(self) return self.iter()
to forward to the existing iter() method without exposing internals.

/// Returns true if any element is shared with `other`.
pub fn intersects(&self, other: &Self) -> bool {
let mut a = self.0.iter();
Expand Down
Loading