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

New flagged storage for parallel joins #737

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Imberflur
Copy link
Contributor

@Imberflur Imberflur commented Oct 29, 2021

Checklist

  • I've added tests for all code changes and additions (where applicable)
  • I've added a demonstration of the new feature to one or more examples
  • I've updated the book to reflect my changes
  • Usage of new public items is shown in the API docs

API changes

Breaking: RestrictedStorage removed (unsound without streaming JoinIter)
Breaking: DerefFlaggedStorage removed (unsound without streaming JoinIter)
Breaking: JoinIter::get/JoinIter::get_unchecked removed (unsound see #647) (can probably be brought back with a streaming JoinIter impl using GATs)

UnsplitFlaggedStorage added for deferred mutable access (like DerefFlaggedStorage) that can also be used in parallel joins (unlike any other flagged storage) (locked behind nightly feature).

Breaking: Safety comments on Join/UnprotectedStorage traits improved (downstream crates that implement novel storages should review these).

…::get that should eliminate unsoundness if followed, add extra safety requirement to UnprotectedStorage::get implementation, comment out unsound methods on JoinIter and some Storages that were clearly unsound with the newly documented requirements (not all storages where reviewed)
…access of a component in a non streaming join as well as using par_join with tracked mutable access
…the current removal of restricted storage (and comment out an example that was using the restricted storage)
@Imberflur Imberflur changed the title New flagged storage New flagged storage for parallel joins Oct 29, 2021
@@ -71,8 +71,9 @@ name = "cluster_bomb"
[[example]]
name = "bitset"

[[example]]
name = "track"
# TODO: restricted storage is unsound without streaming iterator or some changes made to it (i.e. to not allow access to component data on other entities in the mutable case)
Copy link
Contributor

Choose a reason for hiding this comment

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

My addition of GAT support of the storage traits should allow for this, I think.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, Join::Type would probably need to be made into a GAT as well. Are there any GAT based iterator crates that could serve as a replacement for the Iterator trait from std? I'm not seeing any 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

It's pretty trivial to create one.

trait StreamingIterator {
    type Item<'a>;

    fn next(&mut self) -> Self::Item<'_>;
}

I've not seen any 'canonical' streaming iterator crates that use GATs yet. Perhaps when std gets such a trait (here's hoping!) with for integration we can use that instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It will need some of the common adapters/methods: map, for_each, filter, filter_map, collect. Which seems fairly doable but is a bit more than the just the minimal version.

@Imberflur Imberflur mentioned this pull request Mar 1, 2023
13 tasks
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.

2 participants