Skip to content

refactor(runtime): use enumset for frame demand reasons - #505

Merged
eval-exec merged 2 commits into
mainfrom
refactor/demand-reason-enumset
Oct 8, 2026
Merged

eval-exec merged 2 commits into
mainfrom
refactor/demand-reason-enumset

Conversation

@eval-exec

Copy link
Copy Markdown
Owner

DemandReasonSet implemented its own u32 bitset with shifts based on enum indices, coupling set capacity to 32 reasons. Replace it with an alias for enumset::EnumSet<DemandReason> and derive EnumSetType on the existing enum.

Remove handwritten insertion, membership, emptiness, iteration, and FromIterator implementations. Let enumset select storage for the variant count. Keep the existing enum traits, declaration-order iteration, diagnostic reason names, and counter indices. Disable generated supertrait implementations and enum operators to preserve the existing enum interface. Counter arrays and scheduling policy are unchanged.

Add enumset 1.1.14 as a runtime workspace dependency, including its derive dependencies in the lockfile. Existing locked package versions are retained.

Validation: cargo nextest run -p neomacs-display-runtime --lib -E 'test(render_thread::frame_sched::frame_sched_test::)' passes all 45 scheduler tests in parallel. An added regression test covers all 22 reasons, reverse insertion, deduplication, declaration-order iteration, and default emptiness. Existing tests cover frame attribution, counter ordering, deadlines, and independent windows. Formatting and diff checks pass. Tests remain in render_thread/tests/frame_sched_test.rs.

Copilot AI balanced review requested due to automatic review settings October 7, 2026 18:06
@coderabbitai

coderabbitai Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 89feb13d-30ce-4155-b569-07cb48c46382
📥 Commits

Reviewing files that changed from the base of the PR and between fc752c9 and df9b7d6.

📒 Files selected for processing (1)
  • crates/neomacs-display-runtime/src/render_thread/tests/frame_sched_test.rs
 ____________________________________________________
< Veni, Vidi, Verificavi. I came, I saw, I verified. >
 ----------------------------------------------------
  \
   \   \
        \ /\
        ( )
      .( o ).

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 0794e67e-7a03-4416-9c89-54855132960c
📥 Commits

Reviewing files that changed from the base of the PR and between d8bcee8 and fc752c9.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (4)
  • Cargo.toml
  • crates/neomacs-display-runtime/Cargo.toml
  • crates/neomacs-display-runtime/src/render_thread/frame_sched.rs
  • crates/neomacs-display-runtime/src/render_thread/tests/frame_sched_test.rs

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.


📝 Walkthrough

Walkthrough

The display runtime replaces its custom DemandReasonSet bitset with enumset::EnumSet<DemandReason>. Tests cover deduplication, declaration-order iteration, membership, and the empty default.

Changes

Demand reason set

Layer / File(s) Summary
Enumset-backed demand reasons
Cargo.toml, crates/neomacs-display-runtime/Cargo.toml, crates/neomacs-display-runtime/src/render_thread/frame_sched.rs, crates/neomacs-display-runtime/src/render_thread/tests/frame_sched_test.rs
The workspace and display runtime add enumset. DemandReason derives enumset support, and DemandReasonSet becomes an EnumSet alias instead of a custom bitset. Tests cover deduplication, declaration-order iteration, membership, and the empty default.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Refactor

Merge Risk: ⚪ Minimal · up to fc752

The set migration has no identified issue that needs resolution before merge; proceed with normal checks.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: replacing the frame demand reason implementation with enumset.
Description check ✅ Passed The description is directly related to the changeset. It explains the enumset refactor, preserved behavior, dependency changes, and validation results.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. (2 skipped: 2 …
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

2 open findings
What changed in this PR

Refactors the render-thread frame scheduling “demand reason” set implementation from a fixed u32 bitset to enumset::EnumSet, removing the 32-variant coupling while keeping declaration-order iteration and existing scheduling behavior.

Changes:

  • Derive enumset::EnumSetType for DemandReason and replace DemandReasonSet with enumset::EnumSet<DemandReason>.
  • Remove custom set logic (bit operations, iteration, FromIterator) in favor of enumset’s implementations.
  • Add enumset as a workspace dependency and add a regression test covering ordering/dedup/emptiness.
File Description
crates/​neomacs-display-runtime/​src/​render_thread/​frame_sched.rs Switch DemandReasonSet to EnumSet and derive EnumSetType on DemandReason.
crates/​neomacs-display-runtime/​src/​render_thread/​tests/​frame_sched_test.rs Add regression test for deduplication, ordering, and empty-set behavior.
crates/​neomacs-display-runtime/​Cargo.toml Add enumset dependency to the runtime crate via workspace deps.
Cargo.toml Add enumset to workspace dependency versions.

🧠 Review effort: Lite


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +226 to 227
pub(crate) type DemandReasonSet = enumset::EnumSet<DemandReason>;

.rev()
.chain(DemandReason::ALL)
.collect();
assert_eq!(reasons.iter().collect::<Vec<_>>(), DemandReason::ALL);
@eval-exec
eval-exec merged commit 8d0bd5a into main Oct 8, 2026
21 of 30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants