Migrate motiongfx off bevy_ecs onto typarena#114
Conversation
`ActionWorld` (renamed `ActionTable`) no longer wraps a bevy_ecs
`World`; it now composes `typarena::id::{GenId, IdGenerator}` for
entity-style ids and `typarena::type_table::TypeTable` for
heterogeneous per-action storage. `IdRegistry`'s bevy hook-based
cleanup is replaced by an explicit `Cleanup` fn stored once per
subject-id type in `Resources`, a hand-rolled type-keyed singleton
map standing in for bevy's `Resource` storage. `pipeline::sample`'s
multi-component query is replaced by a manual column probe driven
from the `SampleMode` column, with `ColumnId`s for hot, non-generic
columns cached on `ActionTable` to skip repeat `TypeId` hashing.
The `action` module is split into `action/id_registry.rs`
(`IdRegistry`, `UId`, `Cleanup`) and `action/table.rs` (`ActionTable`
and its builders), with `action.rs` left holding the plain
component/value types and re-exporting the public surface.
`bevy_platform` is dropped in favor of `hashbrown` directly, and
`MotionGfxManager` (in `bevy_motiongfx`) becomes a non-send resource
instead of requiring `ActionTable` to be `Send + Sync` — `typarena`'s
`TypeTable` makes no such guarantee, and forcing one would preclude
storing non-thread-safe values in it for other consumers.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR replaces ECS-based action storage with ChangesActionWorld to ActionTable migration
Estimated code review effort: 4 (Complex) | ~75 minutes Sequence Diagram(s)sequenceDiagram
participant Timeline
participant ActionTable
participant Pipeline
participant SubjectWorld
Timeline->>ActionTable: add actions and targets
Timeline->>Timeline: queue ActionId and SampleMode
Timeline->>Pipeline: sample queued actions
Pipeline->>ActionTable: read segments, interpolation, keys, and easing
Pipeline->>SubjectWorld: apply computed target
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
nixonyh
changed the title
bevy_ecs onto typarena
|
@Jaghov benchmark report: Benchmark: ECS vs typarena action storageComparison of the two action-storage backends for
MethodA single benchmark ( The subject world is a bare Three phases are measured at
Each phase runs in two scenes:
ResultsTimes are criterion medians for typarena. Δ is typarena relative to the ECS baseline (negative = typarena faster). Single-type (1 pipeline)
Mixed — 4 value types (4 pipelines,
|
| Phase | N | typarena | Δ vs ECS |
|---|---|---|---|
| mixed_build | 100 | 72.18 µs | −26.7% |
| mixed_build | 1 000 | 758.6 µs | −12.2% |
| mixed_build | 10 000 | 8.308 ms | −7.8% |
| mixed_bake | 100 | 13.53 µs | −21.3% |
| mixed_bake | 1 000 | 135.0 µs | −21.8% |
| mixed_bake | 10 000 | 1.597 ms | −16.6% |
| mixed_scrub | 100 | 979.9 µs | −40.2% |
| mixed_scrub | 1 000 | 11.48 ms | −26.3% |
| mixed_scrub | 10 000 | 158.4 ms | −3.6% |
typarena is faster than ECS on all 18 cells.
Reproducing
Run on a quiet, cool machine. A shared CRITERION_HOME enables the automatic change: / p-value diff.
# ECS baseline (main worktree)
cd ../motiongfx-main
CRITERION_HOME=/tmp/mgfx-crit \
cargo bench -p motiongfx --bench action_storage -- --save-baseline ecs
# typarena, auto-diffed against the baseline
cd ../motiongfx # the nixon/typarena worktree
CRITERION_HOME=/tmp/mgfx-crit \
cargo bench -p motiongfx --bench action_storage -- --baseline ecsHTML reports are written to $CRITERION_HOME/report/index.html. To probe the type-diversity axis harder, add more field types to Widget in benches/timeline.rs.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/motiongfx/benches/action_storage.rs (1)
139-178: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThroughput undercounts actual sampled frames.
Throughput::Elementsis set ton * FRAMES(orn * 4 * FRAMESfor mixed), but the timed closure samplesFRAMES + 1stepped frames (0..=FRAMES) plus one extra rewind sample — one moresample_queued_actionscall than accounted for. This slightly understates elements/sec in the report, though it's a consistent bias so relative branch comparisons remain valid.Also applies to: 369-404
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/motiongfx/benches/action_storage.rs` around lines 139 - 178, The benchmark throughput accounting in bench_scrub undercounts the actual sampled work because the loop in bench_scrub performs FRAMES + 1 sampled frames plus an extra rewind sample, while Throughput::Elements only counts n * FRAMES. Update the throughput units to match the real number of sample_queued_actions calls made inside the timed closure, and apply the same correction to the mixed benchmark block referenced in the comment so the reported elements/sec reflects the full sampled workload.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@crates/motiongfx/benches/action_storage.rs`:
- Around line 139-178: The benchmark throughput accounting in bench_scrub
undercounts the actual sampled work because the loop in bench_scrub performs
FRAMES + 1 sampled frames plus an extra rewind sample, while
Throughput::Elements only counts n * FRAMES. Update the throughput units to
match the real number of sample_queued_actions calls made inside the timed
closure, and apply the same correction to the mixed benchmark block referenced
in the comment so the reported elements/sec reflects the full sampled workload.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1006c13e-43d3-4b93-ad5a-be2a0b6903c2
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
crates/motiongfx/Cargo.tomlcrates/motiongfx/benches/action_storage.rscrates/motiongfx/src/action/table.rscrates/motiongfx/src/pipeline.rscrates/motiongfx/src/timeline.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/motiongfx/Cargo.toml
1 participant
https://github.com/voxell-tech/typarena