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

Add parallelism to searching for builds #970

Merged
merged 6 commits into from
Jun 3, 2024
Merged

Conversation

jrray
Copy link
Collaborator

@jrray jrray commented Feb 5, 2024

I'll say up front that I have doubts this change is actually going to improve performance since the overhead of #966 is mainly file IO.

I had originally tried to implement #966 in a more parallel way but ran into lifetime issues with the async tasks, as would be expected. This PR explores what was required to overcome those lifetime issues.

Some of the changes here might be worth cherry-picking and keeping.

@jrray jrray added the pr-chain This PR doesn't target the main branch, don't merge! label Feb 5, 2024
@jrray jrray self-assigned this Feb 5, 2024
Base automatically changed from jrray/issue966 to main February 6, 2024 19:15
@jrray jrray removed the pr-chain This PR doesn't target the main branch, don't merge! label Feb 6, 2024
@rydrman rydrman self-requested a review February 14, 2024 23:00
Copy link
Collaborator

@rydrman rydrman left a comment

Choose a reason for hiding this comment

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

I'm okay with these overall, actually - it seems like the update with arcswap is a little nicer than the leaky box but I can imagin how this might not have a huge impact

I was reading a little on io_uring the other day and I think it will be interesting to test once it settles into kernel versions that are not bleeding edge. There's some opinions about whether using async for i/o bound workloads can really improve any throughput. Not that I regret our use of async, but it could be true that it's not currently a viable path to much better performance

@jrray
Copy link
Collaborator Author

jrray commented May 17, 2024

Okay, I'll freshen this up and be willing to merge it.

jrray added 5 commits May 20, 2024 12:06
These methods don't need `self`; remove it to make things easier for
async code.

Signed-off-by: J Robert Ray <jrray@jrray.org>
For better ergonomics compared to AtomicPtr, which required unsafe code.

Signed-off-by: J Robert Ray <jrray@jrray.org>
The code does not require it; this change allows for the removal of
DerefMut for SpfsRepository, which is needed for the next change.

Signed-off-by: J Robert Ray <jrray@jrray.org>
Put the non-clone members behind an Arc. This change facilitates code
that would want to move an SpfsRepository instance into an async task.

Signed-off-by: J Robert Ray <jrray@jrray.org>
When checking for the existence of tags called 1, 1.0, 1.0.0, etc.,
spawn tasks for each form.

Signed-off-by: J Robert Ray <jrray@jrray.org>
JoinSet is a better choice in a situation where tasks are getting
spawned.

Signed-off-by: J Robert Ray <jrray@jrray.org>
@jrray
Copy link
Collaborator Author

jrray commented May 20, 2024

@rydrman I got this cleaned up, it can wait until after the holiday.

@jrray jrray merged commit 080f5de into main Jun 3, 2024
7 checks passed
@jrray jrray deleted the jrray/issue966-parallel branch June 3, 2024 17:14
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