Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SystemInformationDiagnosticsPlugin is slow due to sysinfo crate #12848

Closed
jannik4 opened this issue Apr 2, 2024 · 3 comments · Fixed by #13693
Closed

SystemInformationDiagnosticsPlugin is slow due to sysinfo crate #12848

jannik4 opened this issue Apr 2, 2024 · 3 comments · Fixed by #13693
Labels
A-Diagnostics Logging, crash handling, error reporting and performance analysis C-Bug An unexpected or incorrect behavior C-Performance A change motivated by improving speed, memory usage or compile times D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes S-Needs-Investigation This issue requires detective work to figure out what's going wrong

Comments

@jannik4
Copy link
Contributor

jannik4 commented Apr 2, 2024

Bevy version

0.13.1

Relevant system information

  • OS: Windows 11
  • Processor: AMD Ryzen 9 7950X3D 16-Core Processor

What you did

Add the bevy::diagnostic::SystemInformationDiagnosticsPlugin plugin.

What went wrong

The diagnostic_system of the SystemInformationDiagnosticsPlugin takes around 4ms on my system, which makes it unsuitable to use while measuring performance.

Additional information

Noticed this, as I used this with iyes_perf_ui and could not get more than 200FPS.

@jannik4 jannik4 added C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled labels Apr 2, 2024
@alice-i-cecile alice-i-cecile added C-Performance A change motivated by improving speed, memory usage or compile times A-Diagnostics Logging, crash handling, error reporting and performance analysis and removed S-Needs-Triage This issue needs to be labelled labels Apr 2, 2024
@alice-i-cecile
Copy link
Member

Any indication if this is an upstream problem, versus something that Bevy is doing wrong? Is this just latency, or is there real work being done?

@jannik4
Copy link
Contributor Author

jannik4 commented Apr 2, 2024

For the CPU usage there seems to be a MINIMUM_CPU_UPDATE_INTERVAL. This is definitively not respected when running at high frame rates.

Other than that I could not find information on how costly the refresh calls are. It might make sense to do the refreshes/measurements on an AsyncCompute task, so it does not block the frames.

@ChristopherBiscardi
Copy link
Contributor

I ran into this today so here's the tracy graph I was looking at. Enabling the plugin can drop fps to 30 or less, while disabling the plugin lets this particular app run at 200+ fps.

image

2024-05-30T17:46:01.222416Z  INFO bevy_render::renderer: AdapterInfo { name: "NVIDIA GeForce RTX 2080 Ti", vendor: 4318, device: 7687, device_type: DiscreteGpu, driver: "NVIDIA", driver_info: "551.23", backend: Vulkan }
2024-05-30T17:46:01.515482Z  INFO bevy_diagnostic::system_information_diagnostics_plugin::internal: SystemInfo { os: "Windows 11 Pro", kernel: "22631", cpu: "AMD Ryzen 9 7950X 16-Core Processor", core_count: "16", memory: "63.2 GiB" }

@alice-i-cecile alice-i-cecile added S-Needs-Investigation This issue requires detective work to figure out what's going wrong D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes labels May 30, 2024
@alice-i-cecile alice-i-cecile changed the title SystemInformationDiagnosticsPlugin is slow SystemInformationDiagnosticsPlugin is slow due to sysinfo crate Jun 3, 2024
github-merge-queue bot pushed a commit that referenced this issue Jun 10, 2024
# Objective

Reading system information severely slows down the update loop.
Fixes #12848.

## Solution

Read system info in a separate thread.

## Testing

- Open the scene 3d example
- Add `FrameTimeDiagnosticsPlugin`, `SystemInformationDiagnosticsPlugin`
and `LogDiagnosticsPlugin` to the app.
- Add this system to the update schedule to disable Vsync on the main
window
```rust
fn change_window_mode(mut windows: Query<&mut Window, Added<Window>>) {
    for mut window in &mut windows {
        window.present_mode = PresentMode::AutoNoVsync;
    }
}
```
- Read the fps values in the console before and after this PR.

On my PC I went from around 50 fps to around 1150 fps.

---

## Changelog

### Changed

- The `SystemInformationDiagnosticsPlugin` now reads system data
separate of the update cycle.

### Added 

- The `EXPECTED_SYSTEM_INFORMATION_INTERVAL` constant which defines how
often we read system diagnostic data.

---------

Co-authored-by: IceSentry <[email protected]>
mockersf pushed a commit that referenced this issue Jun 10, 2024
Reading system information severely slows down the update loop.
Fixes #12848.

Read system info in a separate thread.

- Open the scene 3d example
- Add `FrameTimeDiagnosticsPlugin`, `SystemInformationDiagnosticsPlugin`
and `LogDiagnosticsPlugin` to the app.
- Add this system to the update schedule to disable Vsync on the main
window
```rust
fn change_window_mode(mut windows: Query<&mut Window, Added<Window>>) {
    for mut window in &mut windows {
        window.present_mode = PresentMode::AutoNoVsync;
    }
}
```
- Read the fps values in the console before and after this PR.

On my PC I went from around 50 fps to around 1150 fps.

---

- The `SystemInformationDiagnosticsPlugin` now reads system data
separate of the update cycle.

- The `EXPECTED_SYSTEM_INFORMATION_INTERVAL` constant which defines how
often we read system diagnostic data.

---------

Co-authored-by: IceSentry <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Diagnostics Logging, crash handling, error reporting and performance analysis C-Bug An unexpected or incorrect behavior C-Performance A change motivated by improving speed, memory usage or compile times D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes S-Needs-Investigation This issue requires detective work to figure out what's going wrong
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants