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

Use prop_filter to force directory treenode #1227

Merged
merged 1 commit into from
Jan 14, 2025
Merged

Use prop_filter to force directory treenode #1227

merged 1 commit into from
Jan 14, 2025

Conversation

c-hagem
Copy link
Contributor

@c-hagem c-hagem commented Jan 13, 2025 •
edited
Loading

On my machine, proptest generation has slowed down (now ~40 seconds to run 30 proptests, before ~16 seconds) significantly. This change (hopefully) keeps the behaviour identical and restores the speed we roughly had before.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and I agree to the terms of the Developer Certificate of Origin (DCO).

@c-hagem c-hagem had a problem deploying to PR integration tests January 13, 2025 18:50 — with GitHub Actions Failure
@c-hagem c-hagem had a problem deploying to PR integration tests January 13, 2025 18:50 — with GitHub Actions Failure
@c-hagem c-hagem had a problem deploying to PR integration tests January 13, 2025 18:50 — with GitHub Actions Failure
@c-hagem c-hagem had a problem deploying to PR integration tests January 13, 2025 18:50 — with GitHub Actions Failure
@c-hagem c-hagem had a problem deploying to PR integration tests January 13, 2025 18:50 — with GitHub Actions Failure
@c-hagem c-hagem had a problem deploying to PR integration tests January 13, 2025 18:50 — with GitHub Actions Failure
@c-hagem c-hagem had a problem deploying to PR integration tests January 13, 2025 18:50 — with GitHub Actions Failure
@c-hagem c-hagem force-pushed the address-performance-regression branch from ba76f28 to 61d1425 Compare January 13, 2025 18:54
@c-hagem c-hagem had a problem deploying to PR integration tests January 13, 2025 18:54 — with GitHub Actions Failure
@c-hagem c-hagem had a problem deploying to PR integration tests January 13, 2025 18:54 — with GitHub Actions Failure
@c-hagem c-hagem had a problem deploying to PR integration tests January 13, 2025 18:54 — with GitHub Actions Failure
@c-hagem c-hagem had a problem deploying to PR integration tests January 13, 2025 18:54 — with GitHub Actions Failure
@c-hagem c-hagem had a problem deploying to PR integration tests January 13, 2025 18:54 — with GitHub Actions Failure
@c-hagem c-hagem had a problem deploying to PR integration tests January 13, 2025 18:54 — with GitHub Actions Failure
@c-hagem c-hagem had a problem deploying to PR integration tests January 13, 2025 18:54 — with GitHub Actions Failure
@c-hagem c-hagem force-pushed the address-performance-regression branch from 61d1425 to fe1e797 Compare January 13, 2025 18:56
@c-hagem c-hagem had a problem deploying to PR integration tests January 13, 2025 18:56 — with GitHub Actions Failure
@c-hagem c-hagem had a problem deploying to PR integration tests January 13, 2025 18:56 — with GitHub Actions Failure
@c-hagem c-hagem had a problem deploying to PR integration tests January 13, 2025 18:56 — with GitHub Actions Failure
@c-hagem c-hagem had a problem deploying to PR integration tests January 13, 2025 18:56 — with GitHub Actions Failure
@c-hagem c-hagem had a problem deploying to PR integration tests January 13, 2025 18:56 — with GitHub Actions Failure
@c-hagem c-hagem had a problem deploying to PR integration tests January 13, 2025 18:56 — with GitHub Actions Failure
@c-hagem c-hagem had a problem deploying to PR integration tests January 13, 2025 18:56 — with GitHub Actions Failure
@c-hagem c-hagem force-pushed the address-performance-regression branch from fe1e797 to ca312e1 Compare January 13, 2025 19:04
@c-hagem c-hagem had a problem deploying to PR integration tests January 13, 2025 19:04 — with GitHub Actions Failure
@c-hagem c-hagem had a problem deploying to PR integration tests January 13, 2025 19:04 — with GitHub Actions Failure
@c-hagem c-hagem had a problem deploying to PR integration tests January 13, 2025 19:04 — with GitHub Actions Failure
@c-hagem c-hagem had a problem deploying to PR integration tests January 13, 2025 19:04 — with GitHub Actions Failure
@c-hagem c-hagem had a problem deploying to PR integration tests January 13, 2025 19:04 — with GitHub Actions Failure
@c-hagem c-hagem had a problem deploying to PR integration tests January 13, 2025 19:04 — with GitHub Actions Failure
@c-hagem c-hagem had a problem deploying to PR integration tests January 14, 2025 09:03 — with GitHub Actions Failure
@c-hagem c-hagem had a problem deploying to PR integration tests January 14, 2025 09:03 — with GitHub Actions Failure
@c-hagem c-hagem had a problem deploying to PR integration tests January 14, 2025 09:04 — with GitHub Actions Failure
@c-hagem c-hagem had a problem deploying to PR integration tests January 14, 2025 09:04 — with GitHub Actions Failure
@c-hagem c-hagem had a problem deploying to PR integration tests January 14, 2025 09:04 — with GitHub Actions Failure
@c-hagem c-hagem had a problem deploying to PR integration tests January 14, 2025 09:04 — with GitHub Actions Failure
@c-hagem c-hagem had a problem deploying to PR integration tests January 14, 2025 09:04 — with GitHub Actions Failure
@c-hagem c-hagem force-pushed the address-performance-regression branch from 6a5e46b to 1410e86 Compare January 14, 2025 09:10
@c-hagem c-hagem had a problem deploying to PR integration tests January 14, 2025 09:11 — with GitHub Actions Failure
@c-hagem c-hagem had a problem deploying to PR integration tests January 14, 2025 09:11 — with GitHub Actions Failure
@c-hagem c-hagem had a problem deploying to PR integration tests January 14, 2025 09:11 — with GitHub Actions Failure
@c-hagem c-hagem had a problem deploying to PR integration tests January 14, 2025 09:11 — with GitHub Actions Failure
@c-hagem c-hagem had a problem deploying to PR integration tests January 14, 2025 09:11 — with GitHub Actions Failure
@c-hagem c-hagem had a problem deploying to PR integration tests January 14, 2025 09:11 — with GitHub Actions Failure
@c-hagem c-hagem had a problem deploying to PR integration tests January 14, 2025 09:11 — with GitHub Actions Failure
@c-hagem
This changes how we generate the trees by using the recursive strateg…
1a95ef7 
…y once and then mapping file-nodes to empty directories.

This should be more performant than our latest, and have similar performance and distribution as the old implementation. Our new implementation  lead to the inner strategy (which defines # of nodes, etc ... for the whole tree) to be run multiple times.

Signed-off-by: Christian Hagemeier <[email protected]>
@c-hagem c-hagem force-pushed the address-performance-regression branch from 1410e86 to 1a95ef7 Compare January 14, 2025 11:03
@c-hagem c-hagem temporarily deployed to PR integration tests January 14, 2025 11:03 — with GitHub Actions Inactive
@c-hagem c-hagem temporarily deployed to PR integration tests January 14, 2025 11:03 — with GitHub Actions Inactive
@c-hagem c-hagem temporarily deployed to PR integration tests January 14, 2025 11:03 — with GitHub Actions Inactive
@c-hagem c-hagem temporarily deployed to PR integration tests January 14, 2025 11:03 — with GitHub Actions Inactive
@c-hagem c-hagem temporarily deployed to PR integration tests January 14, 2025 11:03 — with GitHub Actions Inactive
@c-hagem c-hagem temporarily deployed to PR integration tests January 14, 2025 11:03 — with GitHub Actions Inactive
@c-hagem c-hagem temporarily deployed to PR integration tests January 14, 2025 11:03 — with GitHub Actions Inactive
dannycjones
dannycjones approved these changes Jan 14, 2025
View reviewed changes
Copy link
Contributor

@dannycjones dannycjones left a comment

Choose a reason for hiding this comment

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

Thanks for catching and improving this!

@dannycjones dannycjones enabled auto-merge January 14, 2025 12:09
@dannycjones dannycjones added this pull request to the merge queue Jan 14, 2025
Merged via the queue into awslabs:main with commit 5807eb1 Jan 14, 2025
24 checks passed
keremnc pushed a commit to keremnc/mountpoint-s3 that referenced this pull request Jan 24, 2025
@c-hagem @keremnc
Use prop_filter to force directory treenode (awslabs#1227)
de354fd 
On my machine, proptest generation has slowed down (now ~40 seconds to
run 30 proptests, before ~16 seconds) significantly. This change
(hopefully) keeps the behaviour identical and restores the speed we
roughly had before.

---

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license and I agree to the terms of
the [Developer Certificate of Origin
(DCO)](https://developercertificate.org/).

Signed-off-by: Christian Hagemeier <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dannycjones dannycjones dannycjones approved these changes

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

Successfully merging this pull request may close these issues.
2 participants
@c-hagem @dannycjones