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

q: design of resource-pool #37

Open
MangoIV opened this issue Nov 13, 2024 · 2 comments
Open

q: design of resource-pool #37

MangoIV opened this issue Nov 13, 2024 · 2 comments

Comments

@MangoIV
Copy link

MangoIV commented Nov 13, 2024

I have had a thorough read of the library and some things alienate me

  • the documentation states that it is based on QSem, yet there's not a single use of QSem throughout the library
  • from a quick search of mine I could not figure out what the "striped" design implies. I see how it is implemented but I cannot e.g. find literature on how it is supposed to improve performance etc. It also doesn't seem to be a commonly used term in this context. I could find something in the context of zfs but I do not see how this corresponds to this notion of striping

This also makes it quite hard to use the library - it is not obvious what picking a number of stripes even implies. I understand that this is a work internal library you published so there's no obligation of providing good external documentation but I would still love to see my questions answered.

Thanks in advance!

@arybczak
Copy link
Contributor

arybczak commented Dec 16, 2024

the documentation states that it is based on QSem, yet there's not a single use of QSem throughout the library

It's based on the code from Control.Concurrent.QSem (e.g. this vs this).

I see how it is implemented but I cannot e.g. find literature on how it is supposed to improve performance etc.

Benchmarks are enough ;)

benchmark code
module Main where

import Control.Concurrent
import Control.Monad
import Control.Monad.IO.Class
import Data.Function
import Data.Pool
import GHC.Clock
import Numeric
import System.Environment

timed :: IO a -> IO (a, Double)
timed m = do
  t1 <- liftIO getMonotonicTime
  a <- m
  t2 <- liftIO getMonotonicTime
  pure (a, t2 - t1)

fib :: Integer -> Integer
fib 0 = 1
fib 1 = 1
fib n = fib (n - 1) + fib (n - 2)

worker :: Pool Integer -> MVar Int -> MVar Double -> IO ()
worker pool mctr1 mctr2 = forever $ do
  (_, t) <- timed $ withResource pool $ \n -> do
    pure $! fib n
  liftIO $ do
    modifyMVar_ mctr1 $ \x -> let z = x + 1 in z `seq` pure z
    modifyMVar_ mctr2 $ \x -> let z = max x t in z `seq` pure z

main :: IO ()
main = do
  [threads, stripes] <- map (read @Int) <$> getArgs
  caps <- getNumCapabilities
  mctrs1 <- replicateM threads $ newMVar 0
  mctrs2 <- replicateM threads $ newMVar 0
  let config = defaultPoolConfig (pure 3) (\_ -> pure ()) 10 100 & setNumStripes (Just stripes)
  pool <- newPool config
  putStrLn $ "capabilities: " ++ show caps ++ ", threads: " ++ show threads ++ ", stripes: " ++ show stripes
  void . forM_ (zipWith (,) mctrs1 mctrs2) $ \(mctr1, mctr2) -> do
    forkIO $ do
      --liftIO $ putStrLn . show =<< myThreadId
      worker pool mctr1 mctr2
  _ <- forkIO $ forever $ do
    threadDelay 1000000
    ctr1 <- sum <$> mapM (\v -> modifyMVar v (\z -> pure (0, z))) mctrs1
    ctr2 <- maximum <$> mapM (\v -> modifyMVar v (\z -> pure (0, z))) mctrs2
    putStrLn $ show ctr1 ++ " " ++ showFFloat (Just 6) ctr2 ""
  void getLine
unknown@electronics pool-test $ cabal run pool-test -- 4 4 +RTS -N4
capabilities: 4, threads: 4, stripes: 4
13548967 0.000132
13790616 0.000092
13577645 0.000067
13641423 0.000076
13767243 0.000127
13665174 0.000065
13683066 0.000072
13753343 0.000063
14737538 0.000670
15328146 0.000062
^C
unknown@electronics pool-test $ cabal run pool-test -- 4 1 +RTS -N4
capabilities: 4, threads: 4, stripes: 1
110726 0.000176
126351 0.000377
126472 0.000152
129024 0.000475
129358 0.000670
129941 0.000615
134585 0.000076
128805 0.000419
135323 0.000617
129973 0.000786
^C

Throughput can be over 100x greater when you have a stripe per capability and there is no lock contention. That's why it's the default option if you don't explicitly set the number of stripes.

I understand that this is a work internal library you published

Not quite. We took over maintenance from Brian O'Sullivan, the original author, his implementation was already striped.

I can see why it can appear under-documented, from setNumStripes you can kinda derive what stripes are (sub-pools), but it could be clearer.

@MangoIV
Copy link
Author

MangoIV commented Dec 16, 2024

Lovely! Thank you for your elaborate answer, that really clarifies it!

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

No branches or pull requests

2 participants