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

feat: make Sender state publicly accessible #69

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

andrewgazelka
Copy link
Contributor

@andrewgazelka andrewgazelka commented May 28, 2024

This is useful for having a way to prepare events in a multi-threaded context before actually sending them in a single-threaded context.

// todo: I'd love to see if we actually need the two lifetimes or not.
// We might be able to just use one for both of them, but I never really figure out when I need one versus two.
struct SenderLocal<'a, 'b, ES: EventSet> {
    state: &'b ES::Indices,
    bump: &'a Bump,
    pending_write: Vec<(NonNull<u8>, GlobalEventIdx)>,
}

impl<'a, 'b, ES: EventSet> SenderLocal<'a, 'b, ES> {
    pub fn new(state: &'b ES::Indices, bump: &'a Bump) -> Self {
        Self {
            state,
            bump,

            // todo: Potentially, this should be under bump allocator, but let's profile before trying to do that.
            pending_write: Vec::new(),
        }
    }

    // todo: what does track caller do?
    #[track_caller]
    pub fn send<E: GlobalEvent + 'static>(&mut self, event: E) {
        // The event type and event set are all compile time known, so the compiler
        // should be able to optimize this away.
        let event_idx = ES::find_index::<E>(self.state).unwrap_or_else(|| {
            panic!(
                "global event `{}` is not in the `EventSet` of this `Sender`",
                any::type_name::<E>()
            )
        });

        // let ptr = self.alloc_layout(Layout::new::<E>());
        let ptr = self.bump.alloc_layout(Layout::new::<E>());

        unsafe { ptr::write::<E>(ptr.as_ptr().cast(), event) };

        // This will be saved until the end of the system, where we will be sending the events in one thread synchronously.
        // unsafe { self.world.queue_global(ptr, GlobalEventIdx(event_idx)) };
        self.pending_write.push((ptr, GlobalEventIdx(event_idx)));
    }
}

@andrewgazelka
Copy link
Contributor Author

todo: docs

Copy link
Owner

@rj00a rj00a left a comment

Choose a reason for hiding this comment

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

LGTM once docs are added.

@andrewgazelka
Copy link
Contributor Author

Okay, I added docs. I don't know exactly what you want because I don't know the specifics.

@andrewgazelka
Copy link
Contributor Author

I also made UnsafeWorldCell obtainable. I don't know if this is something you want, but I definitely need it. I don't know if it can be obtained through a handler as well.

@andrewgazelka
Copy link
Contributor Author

Actually, I didn't end up needing to flush.

Comment on lines +664 to +667
/// Grab the state of the sender; the [`EventSet`] indices.
pub fn state(&self) -> &ES::Indices {
self.state
}
Copy link
Owner

Choose a reason for hiding this comment

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

Actually I would prefer a more specific name for this method.

Suggested change
/// Grab the state of the sender; the [`EventSet`] indices.
pub fn state(&self) -> &ES::Indices {
self.state
}
/// Get a reference to the [`EventSet`] indices of this sender.
pub fn indices(&self) -> &ES::Indices {
self.state
}

Comment on lines +670 to +672
pub fn world(&self) -> &UnsafeWorldCell<'a> {
&self.world
}
Copy link
Owner

@rj00a rj00a May 29, 2024

Choose a reason for hiding this comment

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

This is currently unsound because UnsafeWorldCell::world is safe to call. Mark UnsafeWorldCell::world as unsafe and we should be good. UnsafeWorldCell should also be made into a HandlerParam at this point.

Edit: Actually, all of the methods on UnsafeWorldCell should be unsafe.

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.

None yet

2 participants