diff --git a/CHANGELOG.md b/CHANGELOG.md index beb78e3..b6e8f27 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ # resource-pool-0.4.0.0 (????-??-??) * Require `poolMaxResources` to be not smaller than the number of stripes. * Add support for setting the number of stripes. +* The pool will not not exceed maximum resources. [#16](https://github.com/scrive/pool/pull/16) # resource-pool-0.3.1.0 (2022-06-15) * Add `tryWithResource` and `tryTakeResource`. diff --git a/resource-pool.cabal b/resource-pool.cabal index 3b92292..c16d7f5 100644 --- a/resource-pool.cabal +++ b/resource-pool.cabal @@ -46,3 +46,27 @@ library , LambdaCase , RankNTypes , TypeApplications + +test-suite tests + type: exitcode-stdio-1.0 + hs-source-dirs: test + main-is: Main.hs + build-depends: + base + , hedgehog + , hspec + , hspec-hedgehog + , resource-pool + ghc-options: -Wall -Wcompat + build-tool-depends: hspec-discover:hspec-discover + + default-language: Haskell2010 + + default-extensions: DeriveGeneric + , LambdaCase + , RankNTypes + , TypeApplications + + other-modules: + Data.PoolSpec + Data.Pool.InternalSpec diff --git a/src/Data/Pool/Internal.hs b/src/Data/Pool/Internal.hs index 5454949..6e9b3d9 100644 --- a/src/Data/Pool/Internal.hs +++ b/src/Data/Pool/Internal.hs @@ -3,6 +3,7 @@ -- This module is intended for internal use only, and may change without warning -- in subsequent releases. {-# OPTIONS_HADDOCK not-home #-} + module Data.Pool.Internal where import Control.Concurrent @@ -64,15 +65,18 @@ data PoolConfig a = PoolConfig -- ^ The maximum number of resources to keep open across all stripes. The -- smallest acceptable value is @1@. -- - -- /Note:/ for each stripe the number of resources is divided by the number of - -- stripes and rounded up, hence the pool might end up creating up to @N - 1@ - -- resources more in total than specified, where @N@ is the number of stripes. + -- /Note/: Each stripe will try to have an equal amount of resources per + -- stripe. If the number of resources does not evenly divide into the + -- stripes, then some stripes will have one fewer resource than others. , poolNumStripes :: !(Maybe Int) - -- ^ The number of stripes in the pool. + -- ^ The number of stripes in the pool. If 'Nothing' is provided, then + -- this will use 'getNumCapabilities'. This choice is most efficient, as + -- it will prevent contention from different capability threads for the + -- resources. -- - -- /Note:/ if set to 'Nothing', it defaults to the number of capabilities, - -- which ensures that threads never compete over access to the same stripe and - -- results in a very good performance in a multi-threaded environment. + -- The resource count in 'poolMaxResources' will be split evenly among + -- the available stripes. If there are fewer stripes than resources, then + -- this will only create as many stripes as resources. } -- | Create a new striped resource pool. @@ -87,22 +91,34 @@ newPool pc = do error "poolCacheTTL must be at least 0.5" when (poolMaxResources pc < 1) $ do error "poolMaxResources must be at least 1" - numStripes <- maybe getNumCapabilities pure (poolNumStripes pc) - when (numStripes < 1) $ do + numStripesRequested <- maybe getNumCapabilities pure (poolNumStripes pc) + when (numStripesRequested < 1) $ do error "numStripes must be at least 1" + let stripeResourceAllocation = + howManyStripes Input + { inputMaxResources = poolMaxResources pc + , inputStripes = numStripesRequested + } + stripeAllocations = + robin stripeResourceAllocation + indexedAllocations = + zip [1..] stripeAllocations + numStripes = + allowedStripes stripeResourceAllocation + when (poolMaxResources pc < numStripes) $ do error "poolMaxResources must not be smaller than numStripes" - pools <- fmap (smallArrayFromListN numStripes) . forM [1..numStripes] $ \n -> do + pools <- fmap (smallArrayFromListN numStripes) . forM indexedAllocations $ \(index, allocation) -> do ref <- newIORef () stripe <- newMVar Stripe - { available = poolMaxResources pc `quotCeil` numStripes + { available = allocation , cache = [] , queue = Empty , queueR = Empty } -- When the local pool goes out of scope, free its resources. void . mkWeakIORef ref $ cleanStripe (const True) (freeResource pc) stripe - pure LocalPool { stripeId = n + pure LocalPool { stripeId = index , stripeVar = stripe , cleanerRef = ref } @@ -118,11 +134,6 @@ newPool pc = do , reaperRef = ref } where - quotCeil :: Int -> Int -> Int - quotCeil x y = - -- Basically ceiling (x / y) without going through Double. - let (z, r) = x `quotRem` y in if r == 0 then z else z + 1 - -- Collect stale resources from the pool once per second. collector pools = forever $ do threadDelay 1000000 @@ -130,6 +141,54 @@ newPool pc = do let isStale e = now - lastUsed e > poolCacheTTL pc mapM_ (cleanStripe isStale (freeResource pc) . stripeVar) pools +-- | A datatype representing the requested maximum resources and count of +-- stripes. We don't use these figures directly, but instead calculate +-- a 'StripeResourceAllocation' using 'howManyStripes'. +data Input = Input + { inputMaxResources :: !Int + -- ^ How many resources the user requested as an upper limit. + , inputStripes :: !Int + -- ^ How many stripes the user requested. + } + deriving Show + +-- | How many stripes to create, respecting the 'inputMaxResources' on the +-- 'poolInput' field. To create one, use 'howManyStripes'. +data StripeResourceAllocation = StripeResourceAllocation + { poolInput :: !Input + -- ^ The original input for the calculation. + , allowedStripes :: !Int + -- ^ The amount of stripes to actually create. + } + deriving Show + +-- | Determine how many resources should be allocated to each stripe. +-- +-- The output list contains a single `Int` per stripe, with the 'Int' +-- representing the amount of resources available to that stripe. +robin :: StripeResourceAllocation -> [Int] +robin stripeResourceAllocation = + let + numStripes = + allowedStripes stripeResourceAllocation + (baseCount, remainder) = + inputMaxResources (poolInput stripeResourceAllocation) + `divMod` numStripes + in + replicate remainder (baseCount + 1) ++ replicate (numStripes - remainder) baseCount + +-- | A stripe must have at least one resource. If the user requested more +-- stripes than total resources, then we cannot create that many stripes +-- without exceeding the maximum resource limit. +howManyStripes :: Input -> StripeResourceAllocation +howManyStripes inp = StripeResourceAllocation + { allowedStripes = + if inputStripes inp > inputMaxResources inp + then inputMaxResources inp + else inputStripes inp + , poolInput = inp + } + -- | Destroy a resource. -- -- Note that this will ignore any exceptions in the destroy function. diff --git a/test/Data/Pool/InternalSpec.hs b/test/Data/Pool/InternalSpec.hs new file mode 100644 index 0000000..f1b08dd --- /dev/null +++ b/test/Data/Pool/InternalSpec.hs @@ -0,0 +1,65 @@ +{-# language RecordWildCards #-} +{-# language OverloadedStrings #-} + +module Data.Pool.InternalSpec where + +import Control.Monad +import Test.Hspec +import Test.Hspec.Hedgehog +import qualified Hedgehog.Gen as Gen +import qualified Hedgehog.Range as Range +import Data.Pool.Internal + +spec :: Spec +spec = do + describe "howManyStripes" $ do + it "has one stripe per resource" $ hedgehog $ do + input@Input {..} <- forAll inputs + let allocation = howManyStripes input + -- actual stripes may be at most requested stripes + if inputStripes >= inputMaxResources + then do + label "More stripes than resources" + else do + label "Fewer stripes than resources" + inputStripes === allowedStripes allocation + diff (allowedStripes allocation) (<=) inputStripes + + describe "robin" $ do + it "sum of resources is always equal to input" $ hedgehog $ do + input <- forAll inputs + let resourceAllocations = robin (howManyStripes input) + sum resourceAllocations === inputMaxResources input + it "the difference between smallest and largest stripe is at most 1" $ hedgehog $ do + input <- forAll inputs + let resourceAllocations = robin (howManyStripes input) + diff (minimum resourceAllocations) (\a b -> abs (a - b) <= 1) (maximum resourceAllocations) + it "there is a resource allocation for each stripe" $ hedgehog $ do + input <- forAll inputs + let stripeAndResource = howManyStripes input + let resourceAllocations = robin stripeAndResource + length resourceAllocations === allowedStripes stripeAndResource + + describe "newPool" $ do + it "does not throw an error if max resources is less than stripes" $ hedgehog $ do + mnumStripes <- forAll $ Gen.maybe $ Gen.integral (Range.linear 1 100) + maxResources <- + forAll $ + case mnumStripes of + Just numStripes -> + Gen.integral (Range.linear 1 numStripes) + Nothing -> + Gen.integral (Range.linear 1 100) + void $ evalIO $ newPool PoolConfig + { createResource = pure () + , freeResource = \_ -> pure () + , poolCacheTTL = 60.0 + , poolMaxResources = maxResources + , poolNumStripes = mnumStripes + } + +inputs :: Gen Input +inputs = do + resources <- Gen.int (Range.exponentialFrom 8 1 1000) + stripes <- Gen.int (Range.exponentialFrom 20 1 100) + pure $ Input resources stripes diff --git a/test/Data/PoolSpec.hs b/test/Data/PoolSpec.hs new file mode 100644 index 0000000..f03f18c --- /dev/null +++ b/test/Data/PoolSpec.hs @@ -0,0 +1,17 @@ +module Data.PoolSpec where + +import Control.Monad +import Test.Hspec +import Test.Hspec.Hedgehog +import qualified Hedgehog.Gen as Gen +import qualified Hedgehog.Range as Range +import Data.Pool + +spec :: Spec +spec = do + describe "createPool" $ do + it "does not error for any legal set of inputs" $ hedgehog $ do + numStripes <- forAll $ Gen.integral $ Range.linear 1 100 + idleTime <- forAll $ Gen.realFrac_ $ Range.linearFrac 0.5 100 + maxResources <- forAll $ Gen.integral $ Range.linear 1 100 + void $ evalIO $ createPool (pure ()) (\_ -> pure ()) numStripes idleTime maxResources diff --git a/test/Main.hs b/test/Main.hs new file mode 100644 index 0000000..a824f8c --- /dev/null +++ b/test/Main.hs @@ -0,0 +1 @@ +{-# OPTIONS_GHC -F -pgmF hspec-discover #-}