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

Events .send_batch makes a trace! entry even for empty insertions #5026

Closed
CGMossa opened this issue Jun 16, 2022 · 0 comments
Closed

Events .send_batch makes a trace! entry even for empty insertions #5026

CGMossa opened this issue Jun 16, 2022 · 0 comments
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

@CGMossa
Copy link
Contributor

CGMossa commented Jun 16, 2022

Bevy version

main

What you did

When using RUST_LOG=bevy_ecs=trace to figure out what is going on, there is a lot of entries:

2022-06-15T07:20:53.034538Z TRACE bevy_ecs::event: Events::extend() -> ids: (254..254)
2022-06-15T07:20:53.034966Z TRACE bevy_ecs::event: Events::extend() -> ids: (159..159)
2022-06-15T07:20:53.035293Z TRACE bevy_ecs::event: Events::extend() -> ids: (159..159)
2022-06-15T07:20:53.036467Z TRACE bevy_ecs::event: Events::extend() -> ids: (254..254)
2022-06-15T07:20:53.036863Z TRACE bevy_ecs::event: Events::extend() -> ids: (159..159)
2022-06-15T07:20:53.037252Z TRACE bevy_ecs::event: Events::extend() -> ids: (159..159)
2022-06-15T07:20:53.039673Z TRACE bevy_ecs::event: Events::extend() -> ids: (254..254)

These denotes empty insertions of events.

This is a minimal snippet that showcases this issue:

//! Run with
//! ```text
//! RUST_LOG="bevy_ecs=trace" cargo run --bin bevy_ecs_events_extend
//! ```
use bevy_app::prelude::*;
use bevy_ecs::prelude::*;
use blofeld_asf::bevy_app_toolbox::BevyDiagnosticPlugins;
use blofeld_asf::bevy_app_toolbox::DefaultPlugins;
use blofeld_asf::weekly_model::runner::run_until_app_exit;

#[derive(Debug)]
struct AEvent;

fn main() {
    let mut app = App::new();
    app.add_plugins(DefaultPlugins);
    app.add_plugins(BevyDiagnosticPlugins);

    app.add_event::<AEvent>();

    app.add_startup_system(send_one_event);
    // app.add_system(send_one_event);

    app.set_runner(run_until_app_exit);
    app.run();
}

fn send_one_event(mut a_event_wrt: EventWriter<AEvent>) {
    // a_event_wrt.send_batch(std::iter::once(AEvent));
    a_event_wrt.send_batch(std::iter::once(AEvent).skip(1)); // empty iterator of events
}

What went wrong

I'm not sure it is a good idea to log insertions of nothing.
Nor do I want to check for this every time I want to send off events, because this requires an allocation:

    // this causes a call to extend even if `aging_events_tick` is empty.
    // thus we are just going to allocate here:
    let aging_events_tick = aging_events_tick.collect_vec();
    if !aging_events_tick.is_empty() {
        aging_events.send_batch(aging_events_tick.into_iter());
    }

This is not a bug but I don't want to fill logging with empty insertions, and re-allocating every time
I want to send events seems wasteful.
I tried to post this on the discord first, but didn't get any traction there.

Additional information

The responsible chunk is this:

impl<E: Event> std::iter::Extend<E> for Events<E> {
    fn extend<I>(&mut self, iter: I)
    where
        I: IntoIterator<Item = E>,
    {
        let mut event_count = self.event_count;
        let events = iter.into_iter().map(|event| {
            let event_id = EventId {
                id: event_count,
                _marker: PhantomData,
            };
            event_count += 1;
            EventInstance { event_id, event }
        });

        self.events_b.extend(events);

        trace!(
            "Events::extend() -> ids: ({}..{})",
            self.event_count,
            event_count
        );
        self.event_count = event_count;
    }
}
@CGMossa CGMossa added C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled labels Jun 16, 2022
@alice-i-cecile alice-i-cecile added A-ECS Entities, components, systems, and events A-Diagnostics Logging, crash handling, error reporting and performance analysis and removed S-Needs-Triage This issue needs to be labelled labels Jun 16, 2022
@bors bors bot closed this as completed in 4e15d3d Feb 20, 2023
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.

2 participants