Conversation
✅ Coverage Report📊 View Full HTML Report (download artifact) Overall Coverage
Changed Files in This PR
PR Files Coverage: 35.27% (194/550 lines) Generated by cargo-llvm-cov · Latest main coverage Last updated: 2026-03-11 01:12:48 UTC · Commit: |
There was a problem hiding this comment.
Pull request overview
Optimizes ApplicationRuntime event dispatch by introducing a precomputed per-category listener index, reducing per-event iteration to only components interested in the event’s category.
Changes:
- Added
event_listener_bucketandbuild_event_listener_indexhelpers to map event categories to listener buckets and precompute listeners. - Updated event dispatch to iterate only the listener indices for the current event category.
- Added unit tests covering bucket mapping, empty-mask rejection, and listener index construction.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if event_mask.contains(EventMask::WINDOW) { | ||
| return Ok(0); | ||
| } | ||
| if event_mask.contains(EventMask::KEYBOARD) { | ||
| return Ok(1); | ||
| } | ||
| if event_mask.contains(EventMask::MOUSE) { | ||
| return Ok(2); | ||
| } | ||
| if event_mask.contains(EventMask::RUNTIME) { | ||
| return Ok(3); | ||
| } | ||
| if event_mask.contains(EventMask::COMPONENT) { |
There was a problem hiding this comment.
event_listener_bucket is documented as accepting only a single event-category bit, but the implementation uses contains(...), which will silently accept multi-bit masks (e.g. WINDOW | KEYBOARD would map to bucket 0). This makes invariant violations hard to detect and can misroute events if a composite mask is ever passed. Consider matching on exact equality (or explicitly validating the mask has exactly one bit set) and add a unit test asserting multi-bit masks are rejected.
| if event_mask.contains(EventMask::WINDOW) { | |
| return Ok(0); | |
| } | |
| if event_mask.contains(EventMask::KEYBOARD) { | |
| return Ok(1); | |
| } | |
| if event_mask.contains(EventMask::MOUSE) { | |
| return Ok(2); | |
| } | |
| if event_mask.contains(EventMask::RUNTIME) { | |
| return Ok(3); | |
| } | |
| if event_mask.contains(EventMask::COMPONENT) { | |
| if event_mask == EventMask::WINDOW { | |
| return Ok(0); | |
| } | |
| if event_mask == EventMask::KEYBOARD { | |
| return Ok(1); | |
| } | |
| if event_mask == EventMask::MOUSE { | |
| return Ok(2); | |
| } | |
| if event_mask == EventMask::RUNTIME { | |
| return Ok(3); | |
| } | |
| if event_mask == EventMask::COMPONENT { |
| let mut event_loop = LoopBuilder::new().build(); | ||
| let window = self.window_builder.build(&mut event_loop); | ||
| let mut component_stack = self.component_stack; | ||
| let listener_index = build_event_listener_index(&component_stack); |
There was a problem hiding this comment.
listener_index is built once before the RuntimeEvent::Initialized attach phase runs. If a component’s event_mask() depends on state established during on_attach (or later changes during runtime), it won’t be included in the correct buckets and will stop receiving those events (a behavior change vs the previous per-dispatch mask check). Consider building/rebuilding the index after on_attach completes, or updating the index when masks can change, or documenting/enforcing that event_mask() must be stable before runtime start.
| logging::trace!("Sending event: {:?} to all components", event); | ||
|
|
There was a problem hiding this comment.
The trace log message says the event is sent "to all components", but dispatch now iterates only the components in the current category bucket. Updating the message (e.g., to "to matching components" and/or include listener count) will avoid misleading diagnostics.
Summary
Optimizes
ApplicationRuntimeevent dispatch by precomputing per-categorylistener buckets so each event is delivered only to interested components
instead of scanning the full component stack. This reduces dispatch from
O(C)per event toO(k)per event after a one-timeO(C)startup indexbuild, where
Cis total components andkis listeners for the eventcategory.
Related Issues
Changes
crates/lambda-rs/src/runtimes/application.rsconstruction
Type of Change
Affected Crates
lambda-rslambda-rs-platformlambda-rs-argslambda-rs-loggingChecklist
cargo +nightly fmt --all)cargo clippy --workspace --all-targets -- -D warnings)cargo test --workspace)Testing
Commands run:
cargo test -p lambda-rs runtimes::applicationManual verification steps (if applicable):
Screenshots/Recordings
N/A
Platform Testing
Additional Notes