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

Read/Write iterators #12

Merged
merged 5 commits into from May 5, 2017

Conversation

@kvark
Copy link
Owner

commented May 2, 2017

Replaces #11
Fixes #8

The only (and important) downside of the PR is the introduction of unsafe code for WriteItem.

@kvark kvark referenced this pull request May 2, 2017

Closed

[WIP] Iterators #11

4 of 7 tasks complete
@vitvakatu

This comment has been minimized.

Copy link
Collaborator

commented May 2, 2017

Well, I like it more than my attempt :)
If it's final decision, you can merge it and I'll fix demo.

README.md Outdated
[![Crates.io](https://img.shields.io/crates/v/froggy.svg?maxAge=2592000)](https://crates.io/crates/froggy)
[![Gitter](https://badges.gitter.im/kvark/froggy.svg)](https://gitter.im/almost-ecs?utm_source=badge&utm_medium=badge&utm_campaign=pr-badge)

This comment has been minimized.

Copy link
@torkleyy

torkleyy May 2, 2017

Contributor

A link/badge for docs might also be nice.

This comment has been minimized.

Copy link
@kvark

kvark May 2, 2017

Author Owner

agreed, fixed now

@kvark kvark force-pushed the iter branch from 807733e to 2560977 May 2, 2017

@kvark kvark force-pushed the iter branch from 2560977 to 12487c3 May 2, 2017

return None
}
self.index += 1;
if !self.skip_lost || self.lock.guard.meta[id] != 0 {

This comment has been minimized.

Copy link
@torkleyy

torkleyy May 3, 2017

Contributor

Do you think the compiler can move the branch out of the loop or should we have ReadIterAlive?

This comment has been minimized.

Copy link
@kvark

kvark May 3, 2017

Author Owner

I don't think this is a big performance concern at the moment. If not anything else, the hardware CPU branch predictor will remember to always go the right path

@kvark

This comment has been minimized.

Copy link
Owner Author

commented May 5, 2017

Ok, I believe that everyone had a chance to look at this:

  • the replacement of slice derefs with new iterators is a confirmed decision, so even if we decide to change it, we can do this incrementally
  • pointer advancement is new and experimental, but it doesn't introduce new concepts, so not concerning much
  • cloning during the scene graph traversal is unfortunate, we'll look into fixing it without stalling the rest of the changes

@kvark kvark merged commit 92a10e1 into master May 5, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@kvark kvark deleted the iter branch May 5, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.