Rethink `Pool` design

[nickrobinson251]

i noticed that Pool is currently providing two things, which i think it is confusing to combine (at least in the way they're currently combined):

1. "permits": acting as a Semaphore to limit the maximum number of objects in use at a time
2. "store": providing a collection of objects available for re-use

But note that the limit is on the "permits" (the number of objects in use), not on the "store" (the number of object available for re-use), on which there is no limit, e.g.

julia> pool = Pool{Int}(2);

julia> for i in 1:100
           x = acquire(() -> i, pool; forcenew=true)
           release(pool, x)
       end;

julia> length(pool.values)
100

I worry this could lead to a potential memory leak for users who do not realise that the "store" can grow arbitrarily in size and never be cleaned up.

I wonder if the size of the "store" should also be limited (for example, forcenew could force an eviction either always or if the collection is at capacity)

[nickrobinson251]

the isvalid function also has a kind of weird interaction with the stored collection on objects, because it iterated the objects and evicts them until it finds a valid one... which means which objects stay in the pool depends on their insertion order

e.g.

julia> pool = Pool{Int}(2);

julia> for i in 1:100
           x = acquire(() -> i, pool; forcenew=true)
           release(pool, x)
       end;

julia> length(pool.values)
100

julia> acquire(() -> 0, pool; isvalid=x -> x == 1);

julia> length(pool.values)
0

but if they're the other order:

julia> pool = Pool{Int}(2);

julia> for i in 100:-1:1  # <- reversed
           x = acquire(() -> i, pool; forcenew=true)
           release(pool, x)
       end;

julia> length(pool.values)
100

julia> acquire(() -> 0, pool; isvalid=x -> x == 1);


julia> length(pool.values)
99

[nickrobinson251]

i find the "keyed" pool also a bit odd, especially the interaction with release:

• the "permits" are not keyed
• the "store" is keyed

when you acquire an object you must provide a key, so that we know which "store" to check, but this also creates an empty store if there isn't one already

julia> pool = Pool{String, Int}(3);

julia> pool.keyedvalues   # <- no stores
Dict{String, Vector{Int64}}()

julia> acquire(() -> 0, pool, "a");

julia> pool.keyedvalues  # <- now there is an "a" store
Dict{String, Vector{Int64}} with 1 entry:
  "a" => []

when you release an object you must provide a key, but i don't know why, if you just want to release a "permit"; you need to provide a key even though it isn't used... we check the key is of the right type for the pool but not if it's a value for which there is a store:

julia> pool = Pool{String, Int}(3);

julia> x1 = acquire(() -> 1, pool, "a");

julia> pool.cur  # <- 1 permit in use
1

julia> release(pool, x1);  # <- error to release without providing a key
ERROR: ArgumentError: invalid key `nothing` provided for pool key type String
Stacktrace:
 ...

julia> pool.cur  # <- the permit is still in use
1

julia> release(pool, :z, x1);  # <- error to release while providing a key of the wrong _type_
ERROR: ArgumentError: invalid key `z` provided for pool key type String
Stacktrace:
 ...

julia> pool.cur  # <- the permit is still in use
1

julia> release(pool, "z");  # <- fine to release while providing a unknown key 

julia> pool.cur  # <-- the permit was released
0

why do we require a key which we're not going to use, but require it to have the right type but not a known value?

if you want to release the permit and return the object to the store for re-use, then it makes sense that you need to provide a key... but we don't allow returning things to a new store (you can only make stores at acquire time - why?) and providing the wrong key type will error and not release the permit (as above), but providing an unknown key value will error but also still release the permit

julia> pool = Pool{String, Int}(3);

julia> x1 = acquire(() -> 1, pool, "a");

julia> release(pool, :z, x1)   # <- error to release / return object while providing a key of the wrong _type_
ERROR: ArgumentError: invalid key `z` provided for pool key type String
Stacktrace:
 ...

julia> pool.cur   # <- the permit is still in use
1

julia> release(pool, "z", x1)  # <- error to release /return object while providing a unknown key 
ERROR: KeyError: key "z" not found
Stacktrace:
 ...

julia> pool.cur  # <-- but the permit was released (despite the error)
0