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

Useless warnings about apply_system_buffers ticks #8410

Closed
NiseVoid opened this issue Apr 17, 2023 · 5 comments · Fixed by #8760
Closed

Useless warnings about apply_system_buffers ticks #8410

NiseVoid opened this issue Apr 17, 2023 · 5 comments · Fixed by #8760
Labels
A-Diagnostics Logging, crash handling, error reporting and performance analysis A-ECS Entities, components, systems, and events C-Bug An unexpected or incorrect behavior

Comments

@NiseVoid
Copy link
Contributor

Bevy version

0.10 and 0.10.1

What you did

Keep my game server running for a long time

What went wrong

I get 9 of these messages every 24.5 hours:

System 'bevy_ecs::schedule::executor::apply_system_buffers' has not run for 3776567306 ticks. Changes older than 3258167295 ticks will not be detected.

Additional information

A quick look at the code made it seem like this warning is thrown for apply_system_buffers because it never updates last_tick, since apply_system_buffers is an empty function and is never actually executed as a system. Once the tick gets high enough it prints this warning for every apply_systems_buffers in the schedule.

The game server has a fairly minimal set of plugins:

        .add_plugin(bevy::core::TaskPoolPlugin::default())
        .add_plugin(bevy::core::TypeRegistrationPlugin::default())
        .add_plugin(bevy::core::FrameCountPlugin::default())
        .add_plugin(bevy::time::TimePlugin::default())
        .add_plugin(bevy::log::LogPlugin::default())

The only other things of note are a custom runner (to achieve a more precise tickrate) and bevy_rapier3d

@NiseVoid NiseVoid added C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled labels Apr 17, 2023
@james7132 james7132 added A-ECS Entities, components, systems, and events and removed S-Needs-Triage This issue needs to be labelled labels Apr 17, 2023
@james7132
Copy link
Member

Trying to understand the problem here. Is it that changes don't last long enough? Is it that you're getting the messages?

If the problem the messages, you can always use RUST_LOG environment variable to suppress warning level logs
from bevy_ecs.

@alice-i-cecile alice-i-cecile added the A-Diagnostics Logging, crash handling, error reporting and performance analysis label Apr 17, 2023
@alice-i-cecile
Copy link
Member

I wonder if we should be producing these at all for apply_system_buffers. I suppose that you can actually use the provided &mut World but that won't give you access to that system's change ticks at all so the error is always spurious.

@SkiFire13
Copy link
Contributor

My guess is that the problem is the executor special-casing apply_system_buffer, which results in applying the system buffers but not running the system itself, which results in its last change ticks never being updated.

In particular this block of code (and the equivalent in the single-threaded executor) should be the one responsible for this:

if is_apply_system_buffers(system) {
// TODO: avoid allocation
let unapplied_systems = self.unapplied_systems.clone();
self.unapplied_systems.clear();
let task = async move {
#[cfg(feature = "trace")]
let system_guard = system_span.enter();
let res = apply_system_buffers(&unapplied_systems, systems, world);
#[cfg(feature = "trace")]
drop(system_guard);
// tell the executor that the system finished
sender
.try_send(SystemResult {
system_index,
success: res.is_ok(),
})
.unwrap_or_else(|error| unreachable!("{}", error));
if let Err(payload) = res {
// set the payload to propagate the error
let mut panic_payload = panic_payload.lock().unwrap();
*panic_payload = Some(payload);
}
};
#[cfg(feature = "trace")]
let task = task.instrument(task_span);
scope.spawn_on_scope(task);
} else {

@NiseVoid
Copy link
Contributor Author

Trying to understand the problem here. Is it that changes don't last long enough? Is it that you're getting the messages?

If the problem the messages, you can always use RUST_LOG environment variable to suppress warning level logs from bevy_ecs.

The issue would be that bevy incorrectly produces this warning. A message saying apply_system_buffers did not run for some arbitrary long amount of time sounds pretty concerning, but if that was the case your application probably wouldn't function.

I wonder if we should be producing these at all for apply_system_buffers. I suppose that you can actually use the provided &mut World but that won't give you access to that system's change ticks at all so the error is always spurious.

Maybe the ideal fix for this issue wouldn't be apply_system_buffers specific but making it so that it only throws these warnings where they can actually matter

My guess is that the problem is the executor special-casing apply_system_buffer, which results in applying the system buffers but not running the system itself, which results in its last change ticks never being updated.

Yea, that's what I found with my quick look trough the code too. last_tick is updated in run_unsafe, which is called by run, which is only called in the else block

@alice-i-cecile
Copy link
Member

Maybe the ideal fix for this issue wouldn't be apply_system_buffers specific but making it so that it only throws these warnings where they can actually matter

This error could be useful in any system that accesses any system param that impls either DetectChanges or DetectChangesMut. Which includes resources, non-send resources and queries. Because you can call .is_changed in the body of the function we can't simply check for the presence of Changed query filters.

james7132 pushed a commit that referenced this issue Jun 6, 2023
# Objective

- Fixes #8410

## Solution

- Skip the check that produces the warning for apply_buffers systems.

---

## Changelog

- skip check_change_ticks for apply_buffers systems.
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 A-ECS Entities, components, systems, and events C-Bug An unexpected or incorrect behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants