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

[NO-ISSUE] feat(import): use poolboy to have bounded concurrency #360

Merged
merged 1 commit into from
Apr 3, 2023

Conversation

blackheaven
Copy link
Contributor

Proposed changes

Contributor checklist

  • My PR is related to <insert ticket number>
  • I have read and understood the CONTRIBUTING guide
  • I have inserted my change and a link to this PR in the CHANGELOG

Change

I finally found the time to have a shot at a simple bounded concurrency library.

# Before (Streamly)
real    17m31.962s
user    97m15.617s
sys     26m13.162s

# After (Poolboy)
real    10m40.314s
user    92m33.556s
sys     22m42.311s

On my Intel(R) Core(TM) i9-10850K CPU @ 3.60GHz (10 Cores / 20 Threads (HT)).

Regarding max metrics:

  • load avg: 32 / 19 / 9
  • RAM: 3 GiB

@blackheaven blackheaven force-pushed the feat/poolboy branch 2 times, most recently from 17f10fe to ff8cce7 Compare March 26, 2023 14:52
@tchoutri
Copy link
Contributor

Ah great I can now ask the questions I had about poolboy! :D

  1. What is the difference with resource-pool?
  2. You say it is "Simple". This means you've decided to cut some corners or that you consciously don't support some things. What are they?

@blackheaven
Copy link
Contributor Author

Ah great I can now ask the questions I had about poolboy! :D

  1. What is the difference with resource-pool?

resource-pool is "passive" in the sense, you have a function withResource, which checks the Pool, if there's an element available in it, it takes it, otherwise it creates in (going beyond the limit). Once done, when they are with the item, it tries to put it back in the pool, if they is already as many items, a it is configured in the pool, it frees it. In a sense, it's an unbounded cache pool.

poolboy is quite simple, it spawns N threads polling a queue of computations (it's a work queue).

  1. You say it is "Simple". This means you've decided to cut some corners or that you consciously don't support some things. What are they?

It's simple by design, no fancy stuff in, I may have miss some things inherent to work queues (not willingly though).

@tchoutri
Copy link
Contributor

no fancy stuff in

Could you expand a bit on that? I'm not a pool expert in any way. :)

@blackheaven
Copy link
Contributor Author

no fancy stuff in

Could you expand a bit on that? I'm not a pool expert in any way. :)

We could imagine an option which changes the number of thread depending on the CPU load, it's actually possible to dynamically change the number of thread, but you'll have to do it yourself.

@tchoutri
Copy link
Contributor

Ah perfect, thank you!

@blackheaven
Copy link
Contributor Author

I think hackage is not up-to-date on the CI

@tchoutri
Copy link
Contributor

Yes you should update the timestamp in the freeze file I believe

@blackheaven
Copy link
Contributor Author

Weirdly cabal freeze didn't update the snapshot :/

I did some tests regarding Pools' size, it seems that smaller pools are faster:

# Before (Streamly)
real    17m31.962s
user    97m15.617s
sys     26m13.162s

# Poolboy (100)
real    10m40.314s
user    92m33.556s
sys     22m42.311s

# Poolboy (80)
real    8m36.634s
user    93m2.330s
sys     23m12.678s

# Poolboy (60)
real    7m44.719s
user    91m14.997s
sys     22m57.244s

# Poolboy (50)
real    7m46.905s
user    91m17.109s
sys     22m56.540s

# Poolboy (40)
real    7m45.533s
user    90m50.408s
sys     22m51.399s

# Poolboy (30)
real    7m41.438s
user    90m35.791s
sys     22m53.644s

# Poolboy (20)
real    8m8.061s
user    90m22.348s
sys     22m25.652s

@blackheaven blackheaven marked this pull request as ready for review March 26, 2023 18:10
@blackheaven
Copy link
Contributor Author

Seems all right, should I squash?

@tchoutri
Copy link
Contributor

tchoutri commented Apr 2, 2023 via email

@tchoutri
Copy link
Contributor

tchoutri commented Apr 2, 2023

@blackheaven I'll let you correct the commit name / PR title and then MergifyBot will merge :)

@blackheaven blackheaven changed the title feat(import): use poolboy to have bounded concurrency [NO-ISSUE] feat(import): use poolboy to have bounded concurrency Apr 3, 2023
@tchoutri tchoutri added the merge me Tell Mergify bot to merge the PR label Apr 3, 2023
@mergify mergify bot merged commit d38f65f into flora-pm:development Apr 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge me Tell Mergify bot to merge the PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants