Skip to content

Conversation

@andrewgazelka
Copy link
Member

No description provided.

Copilot AI review requested due to automatic review settings April 22, 2025 20:49
@andrewgazelka andrewgazelka enabled auto-merge April 22, 2025 20:49
@github-actions github-actions bot added the feat label Apr 22, 2025
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR changes the threading model for flecs systems to enforce single-threaded execution. Key changes include the removal of the .multi_threaded() calls from multiple system definitions and updating NUM_THREADS to 1 in the core configuration.

Reviewed Changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated no comments.

Show a summary per file
File Description
events/tag/src/module/vanish.rs Removed .multi_threaded() to run vanish events single-threaded.
events/tag/src/module/stats.rs Removed .multi_threaded() from the stats system.
events/tag/src/module/regeneration.rs Removed .multi_threaded() from the regeneration module.
events/tag/src/module/chat.rs Removed .multi_threaded() from the chat system.
events/tag/src/module/bow.rs Removed .multi_threaded() from both bow system blocks.
events/tag/src/module/block.rs Removed .multi_threaded() from several block handling systems.
events/tag/src/module/attack.rs Removed .multi_threaded() from attack handling systems.
crates/hyperion/src/simulation/metadata/mod.rs Removed .multi_threaded() from metadata sync system.
crates/hyperion/src/simulation/inventory.rs Removed .multi_threaded() from the inventory system.
crates/hyperion/src/lib.rs Changed NUM_THREADS from 8 to 1 to match single-threaded execution.
crates/hyperion/src/ingress/mod.rs Removed .multi_threaded() from the ingress processing.
crates/hyperion/src/egress/sync_entity_state.rs Removed .multi_threaded() from multiple entity state sync systems.
crates/hyperion/src/egress/sync_chunks.rs Removed .multi_threaded() from the full chunk sync system.
crates/hyperion/src/egress/mod.rs Removed .multi_threaded() from the egress broadcast system.
Comments suppressed due to low confidence (5)

events/tag/src/module/vanish.rs:38

  • Removal of '.multi_threaded()' enforces single-threaded execution. Ensure that this change does not affect the intended concurrency behavior for handling vanished events.
        .multi_threaded()

events/tag/src/module/stats.rs:22

  • Removal of '.multi_threaded()' is consistent with a single-threaded approach; please verify that sequential processing here meets performance requirements.
            .multi_threaded()

crates/hyperion/src/ingress/mod.rs:445

  • The removal of '.multi_threaded()' here ensures immediate, single-threaded processing in ingress; confirm that this change does not lead to unintended delays or data race issues.
        .multi_threaded()

crates/hyperion/src/egress/sync_entity_state.rs:55

  • Ensure that removing '.multi_threaded()' does not impact the correct propagation of entity state synchronization, given timing and ordering considerations.
        .multi_threaded()

crates/hyperion/src/lib.rs:28

  • Changing NUM_THREADS to 1 aligns with the single-threaded execution model; ensure that all parts of the system are adjusted accordingly.
pub const NUM_THREADS: usize = 1;

@github-actions
Copy link

Benchmark Results for general

ray_intersection/aabb_size_0.1                     [  33.4 ns ...  33.4 ns ]      -0.08%
ray_intersection/aabb_size_1                       [  33.8 ns ...  33.7 ns ]      -0.55%
ray_intersection/aabb_size_10                      [  24.2 ns ...  24.2 ns ]      -0.02%
ray_intersection/ray_distance_1                    [  13.7 ns ...  13.8 ns ]      +0.18%
ray_intersection/ray_distance_5                    [  13.8 ns ...  13.8 ns ]      -0.16%
ray_intersection/ray_distance_20                   [  13.8 ns ...  13.8 ns ]      -0.01%
overlap/no_overlap                                 [  24.8 ns ...  24.8 ns ]      -0.02%
overlap/partial_overlap                            [  25.1 ns ...  25.1 ns ]      +0.27%
overlap/full_containment                           [  22.5 ns ...  22.5 ns ]      +0.09%
point_containment/inside                           [   4.9 ns ...   4.9 ns ]      -0.07%
point_containment/outside                          [   4.4 ns ...   4.4 ns ]      -0.04%
point_containment/boundary                         [   4.9 ns ...   4.9 ns ]      +0.33%

Comparing to d817614

@andrewgazelka andrewgazelka added this pull request to the merge queue Apr 22, 2025
@codecov
Copy link

codecov bot commented Apr 22, 2025

Codecov Report

Attention: Patch coverage is 2.22222% with 44 lines in your changes missing coverage. Please review.

Project coverage is 20.86%. Comparing base (d817614) to head (2d2d3c4).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
events/tag/src/module/stats.rs 0.00% 38 Missing ⚠️
events/tag/src/module/block.rs 0.00% 3 Missing ⚠️
crates/hyperion/src/simulation/inventory.rs 0.00% 1 Missing ⚠️
events/tag/src/module/attack.rs 0.00% 1 Missing ⚠️
events/tag/src/module/chat.rs 0.00% 1 Missing ⚠️
@@            Coverage Diff             @@
##             main     #883      +/-   ##
==========================================
- Coverage   20.91%   20.86%   -0.05%     
==========================================
  Files         161      161              
  Lines       16876    16857      -19     
  Branches      469      469              
==========================================
- Hits         3529     3518      -11     
+ Misses      13283    13275       -8     
  Partials       64       64              
Files with missing lines Coverage Δ
crates/hyperion/src/egress/mod.rs 51.44% <ø> (-0.35%) ⬇️
crates/hyperion/src/egress/sync_chunks.rs 18.82% <100.00%> (-0.48%) ⬇️
crates/hyperion/src/egress/sync_entity_state.rs 30.44% <ø> (-1.15%) ⬇️
crates/hyperion/src/ingress/mod.rs 22.32% <ø> (-0.36%) ⬇️
crates/hyperion/src/lib.rs 85.06% <ø> (ø)
crates/hyperion/src/simulation/metadata/mod.rs 75.00% <ø> (-0.21%) ⬇️
events/tag/src/module/bow.rs 0.00% <ø> (ø)
events/tag/src/module/regeneration.rs 0.00% <ø> (ø)
events/tag/src/module/vanish.rs 0.00% <ø> (ø)
crates/hyperion/src/simulation/inventory.rs 0.00% <0.00%> (ø)
... and 4 more
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Merged via the queue into main with commit 24fff2e Apr 22, 2025
12 checks passed
@andrewgazelka andrewgazelka deleted the andrew/single-thread branch April 22, 2025 21:00
TestingPlant added a commit to TestingPlant/hyperion that referenced this pull request Jul 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants