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.

Sign up for GitHub

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

Jump to bottom

37 integrate query filtering into mvp #49

Merged
merged 15 commits into from
Mar 30, 2023
Merged

37 integrate query filtering into mvp #49

merged 15 commits into from
Mar 30, 2023

Conversation

EdvinNilsson
Copy link
Collaborator

Implements query filters for With, Without, And, Or, Xor, Not and Any.

This pull request will later be rebased on #38 before being merged.

@github-actions
Copy link

Benchmark for b046ee9

Click to view benchmark
Test Base PR %
sleep 159.7±3.43µs 164.1±10.41µs +2.76%
sleep x/1 60.5±1.74µs 16.1 KElem/sec N/A N/A
sleep x/2 62.0±2.92µs 31.5 KElem/sec N/A N/A
sleep x/3 63.7±4.35µs 46.0 KElem/sec N/A N/A
sleep x/4 63.6±1.84µs 61.4 KElem/sec N/A N/A
sleep x/5 64.0±1.67µs 76.3 KElem/sec N/A N/A
sleep x/6 65.0±1.02µs 90.2 KElem/sec N/A N/A
sleep x/7 65.8±0.57µs 103.9 KElem/sec N/A N/A
sleep x/8 66.8±1.09µs 117.0 KElem/sec N/A N/A
sleep x/9 68.0±1.47µs 129.3 KElem/sec N/A N/A

@github-actions
Copy link

Benchmark for ec6a485

Click to view benchmark
Test Base PR %
sleep 159.4±3.26µs 159.6±1.94µs +0.13%
sleep x/1 60.0±1.02µs 16.3 KElem/sec N/A N/A
sleep x/2 61.2±0.67µs 31.9 KElem/sec N/A N/A
sleep x/3 62.5±1.86µs 46.9 KElem/sec N/A N/A
sleep x/4 62.9±1.01µs 62.1 KElem/sec N/A N/A
sleep x/5 64.2±1.77µs 76.0 KElem/sec N/A N/A
sleep x/6 65.2±1.66µs 89.9 KElem/sec N/A N/A
sleep x/7 66.0±1.60µs 103.5 KElem/sec N/A N/A
sleep x/8 67.3±1.43µs 116.0 KElem/sec N/A N/A
sleep x/9 68.1±1.53µs 129.0 KElem/sec N/A N/A

@martinjonsson01 martinjonsson01 linked an issue Mar 20, 2023 that may be closed by this pull request
Integrate query filtering into MVP #37
Closed
8 tasks
martinjonsson01
martinjonsson01 approved these changes Mar 21, 2023
View reviewed changes
Copy link
Owner

@martinjonsson01 martinjonsson01 left a comment

Choose a reason for hiding this comment

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

Looking good!

I've no major comments, so in principle it's ready for merge imo. But I do have quite a few questions regarding implementation choices, and some nitpicks about the way the tests are written.

crates/ecs/src/filter.rs Outdated Show resolved Hide resolved
crates/ecs/src/filter.rs Outdated Show resolved Hide resolved
crates/ecs/src/filter.rs Outdated Show resolved Hide resolved
crates/ecs/src/filter.rs Show resolved Hide resolved
crates/ecs/src/filter.rs Outdated Show resolved Hide resolved
crates/ecs/src/filter.rs Outdated Show resolved Hide resolved
crates/ecs/src/filter.rs Outdated Show resolved Hide resolved
crates/ecs/src/filter.rs Outdated Show resolved Hide resolved
crates/ecs/src/filter.rs Outdated Show resolved Hide resolved
crates/ecs/src/lib.rs Outdated Show resolved Hide resolved
@github-actions
Copy link

Benchmark for dca8b55

Click to view benchmark
Test Base PR %
sleep 158.9±2.70µs 158.5±0.49µs -0.25%
sleep x/1 59.5±1.48µs 16.4 KElem/sec N/A N/A
sleep x/2 60.6±2.25µs 32.2 KElem/sec N/A N/A
sleep x/3 61.5±0.95µs 47.6 KElem/sec N/A N/A
sleep x/4 62.6±1.00µs 62.4 KElem/sec N/A N/A
sleep x/5 63.6±1.61µs 76.7 KElem/sec N/A N/A
sleep x/6 64.6±1.08µs 90.7 KElem/sec N/A N/A
sleep x/7 65.7±0.93µs 104.0 KElem/sec N/A N/A
sleep x/8 66.5±0.59µs 117.4 KElem/sec N/A N/A
sleep x/9 67.7±0.77µs 129.8 KElem/sec N/A N/A

@github-actions
Copy link

Benchmark for a7501e6

Click to view benchmark
Test Base PR %
sleep 159.3±2.95µs 158.8±0.88µs -0.31%
sleep x/1 59.4±1.23µs 16.4 KElem/sec N/A N/A
sleep x/2 60.3±0.75µs 32.4 KElem/sec N/A N/A
sleep x/3 61.9±1.51µs 47.3 KElem/sec N/A N/A
sleep x/4 63.7±3.07µs 61.3 KElem/sec N/A N/A
sleep x/5 63.7±1.40µs 76.7 KElem/sec N/A N/A
sleep x/6 65.2±2.77µs 89.9 KElem/sec N/A N/A
sleep x/7 65.6±1.61µs 104.1 KElem/sec N/A N/A
sleep x/8 67.9±4.74µs 115.0 KElem/sec N/A N/A
sleep x/9 67.4±0.85µs 130.5 KElem/sec N/A N/A

@github-actions
Copy link

Benchmark for b62fbbc

Click to view benchmark
Test Base PR %
sleep 160.4±3.66µs 162.2±4.58µs +1.12%
sleep x/1 60.0±2.02µs 16.3 KElem/sec N/A N/A
sleep x/2 60.5±0.57µs 32.3 KElem/sec N/A N/A
sleep x/3 61.7±0.78µs 47.5 KElem/sec N/A N/A
sleep x/4 62.8±1.33µs 62.2 KElem/sec N/A N/A
sleep x/5 64.4±1.40µs 75.9 KElem/sec N/A N/A
sleep x/6 65.6±1.65µs 89.3 KElem/sec N/A N/A
sleep x/7 67.7±2.70µs 100.9 KElem/sec N/A N/A
sleep x/8 69.5±2.64µs 112.4 KElem/sec N/A N/A
sleep x/9 70.7±3.02µs 124.4 KElem/sec N/A N/A

@github-actions
Copy link

Benchmark for 0d7ef70

Click to view benchmark
Test Base PR %
sleep 167.8±6.10µs 164.7±5.10µs -1.85%
sleep x/1 65.4±3.07µs 14.9 KElem/sec N/A N/A
sleep x/2 66.4±3.60µs 29.4 KElem/sec N/A N/A
sleep x/3 69.2±3.42µs 42.4 KElem/sec N/A N/A
sleep x/4 67.5±4.37µs 57.9 KElem/sec N/A N/A
sleep x/5 65.0±4.01µs 75.1 KElem/sec N/A N/A
sleep x/6 70.7±4.28µs 82.9 KElem/sec N/A N/A
sleep x/7 72.5±5.36µs 94.3 KElem/sec N/A N/A
sleep x/8 73.0±3.67µs 107.0 KElem/sec N/A N/A
sleep x/9 76.3±7.57µs 115.2 KElem/sec N/A N/A

@github-actions
Copy link

Benchmark for 693a754

Click to view benchmark
Test Base PR %
sleep 160.4±3.70µs 162.5±4.02µs +1.31%
sleep x/1 61.8±3.44µs 15.8 KElem/sec N/A N/A
sleep x/2 60.4±0.78µs 32.4 KElem/sec N/A N/A
sleep x/3 63.0±2.72µs 46.5 KElem/sec N/A N/A
sleep x/4 64.7±2.55µs 60.4 KElem/sec N/A N/A
sleep x/5 65.9±3.22µs 74.1 KElem/sec N/A N/A
sleep x/6 67.6±2.91µs 86.7 KElem/sec N/A N/A
sleep x/7 67.5±2.92µs 101.3 KElem/sec N/A N/A
sleep x/8 67.9±3.02µs 115.0 KElem/sec N/A N/A
sleep x/9 68.7±2.62µs 127.9 KElem/sec N/A N/A

@github-actions
Copy link

Benchmark for c82c376

Click to view benchmark
Test Base PR %
sleep 165.0±5.48µs 167.7±11.81µs +1.64%
sleep x/1 65.3±4.65µs 14.9 KElem/sec N/A N/A
sleep x/2 68.5±5.12µs 28.5 KElem/sec N/A N/A
sleep x/3 68.0±5.22µs 43.1 KElem/sec N/A N/A
sleep x/4 67.3±2.86µs 58.1 KElem/sec N/A N/A
sleep x/5 68.3±3.37µs 71.5 KElem/sec N/A N/A
sleep x/6 70.3±5.93µs 83.3 KElem/sec N/A N/A
sleep x/7 73.3±9.43µs 93.3 KElem/sec N/A N/A
sleep x/8 73.4±5.27µs 106.4 KElem/sec N/A N/A
sleep x/9 76.2±25.33µs 115.3 KElem/sec N/A N/A

@EdvinNilsson EdvinNilsson marked this pull request as ready for review March 29, 2023 19:06
martinjonsson01
martinjonsson01 approved these changes Mar 30, 2023
View reviewed changes
Copy link
Owner

@martinjonsson01 martinjonsson01 left a comment

Choose a reason for hiding this comment

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

LGTM!

crates/ecs/src/lib.rs Outdated Show resolved Hide resolved
crates/ecs/src/systems/mod.rs Outdated Show resolved Hide resolved
crates/ecs/src/filter.rs Show resolved Hide resolved
@github-actions
Copy link

Benchmark for af26826

Click to view benchmark
Test Base PR %
sleep 163.2±4.50µs 165.7±12.89µs +1.53%
sleep x/1 63.4±4.95µs 15.4 KElem/sec N/A N/A
sleep x/2 66.5±2.44µs 29.4 KElem/sec N/A N/A
sleep x/3 64.2±2.68µs 45.6 KElem/sec N/A N/A
sleep x/4 66.0±3.19µs 59.1 KElem/sec N/A N/A
sleep x/5 66.4±4.09µs 73.5 KElem/sec N/A N/A
sleep x/6 67.7±4.01µs 86.5 KElem/sec N/A N/A
sleep x/7 72.6±3.38µs 94.1 KElem/sec N/A N/A
sleep x/8 71.3±2.49µs 109.6 KElem/sec N/A N/A
sleep x/9 72.9±3.77µs 120.6 KElem/sec N/A N/A

JacobBredin
JacobBredin approved these changes Mar 30, 2023
View reviewed changes
Copy link
Collaborator

@JacobBredin JacobBredin left a comment

Choose a reason for hiding this comment

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

LGTM! Nicely done with the intersection and difference methods on the HashSet 👍

crates/ecs/src/lib.rs Show resolved Hide resolved
@EdvinNilsson @martinjonsson01
Fix grammar mistake
31922ba 
Co-authored-by: Martin <[email protected]>
@github-actions
Copy link

Benchmark for 640a334

Click to view benchmark
Test Base PR %
sleep 182.0±68.81µs 174.2±79.19µs -4.29%
sleep x/1 65.1±9.17µs 15.0 KElem/sec N/A N/A
sleep x/2 67.8±12.24µs 28.8 KElem/sec N/A N/A
sleep x/3 70.1±20.88µs 41.8 KElem/sec N/A N/A
sleep x/4 79.4±42.65µs 49.2 KElem/sec N/A N/A
sleep x/5 70.9±13.28µs 68.9 KElem/sec N/A N/A
sleep x/6 73.4±27.46µs 79.8 KElem/sec N/A N/A
sleep x/7 71.3±6.50µs 95.8 KElem/sec N/A N/A
sleep x/8 76.6±30.65µs 102.0 KElem/sec N/A N/A
sleep x/9 77.1±22.70µs 114.1 KElem/sec N/A N/A

@EdvinNilsson EdvinNilsson enabled auto-merge March 30, 2023 12:15
@github-actions
Copy link

Benchmark for e605c36

Click to view benchmark
Test Base PR %
sleep 159.4±2.99µs 160.4±5.23µs +0.63%
sleep x/1 60.3±1.81µs 16.2 KElem/sec N/A N/A
sleep x/2 61.4±1.71µs 31.8 KElem/sec N/A N/A
sleep x/3 62.2±1.29µs 47.1 KElem/sec N/A N/A
sleep x/4 62.9±0.93µs 62.1 KElem/sec N/A N/A
sleep x/5 63.8±1.25µs 76.5 KElem/sec N/A N/A
sleep x/6 64.9±0.91µs 90.3 KElem/sec N/A N/A
sleep x/7 66.0±0.60µs 103.6 KElem/sec N/A N/A
sleep x/8 67.3±2.05µs 116.1 KElem/sec N/A N/A
sleep x/9 68.1±1.03µs 129.0 KElem/sec N/A N/A

@EdvinNilsson EdvinNilsson merged commit 425d9f6 into master Mar 30, 2023
@EdvinNilsson EdvinNilsson deleted the 37-integrate-query-filtering-into-mvp branch March 30, 2023 12:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@martinjonsson01 martinjonsson01 martinjonsson01 approved these changes

@JacobBredin JacobBredin JacobBredin approved these changes

@ChristofferPerssons ChristofferPerssons Awaiting requested review from ChristofferPerssons

@linnosterlund linnosterlund Awaiting requested review from linnosterlund

@Matte02 Matte02 Awaiting requested review from Matte02

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Integrate query filtering into MVP
3 participants
@EdvinNilsson @martinjonsson01 @JacobBredin