Skip to content

Commit 7980069

Browse files
author
David Leach
committed
Convert worker list to queue for consistent lifo|fifo performance
1 parent 0fe289b commit 7980069

2 files changed

Lines changed: 65 additions & 60 deletions

File tree

‎src/poolboy.erl‎

Lines changed: 30 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@
3838

3939
-record(state, {
4040
supervisor :: undefined | pid(),
41-
workers = [] :: [pid()],
41+
workers :: pid_queue(),
4242
waiting :: pid_queue(),
4343
monitors :: ets:tid(),
4444
size = 5 :: non_neg_integer(),
@@ -205,19 +205,20 @@ handle_call({checkout, CRef, Block}, {FromPid, _} = From, State) ->
205205
workers = Workers,
206206
monitors = Monitors,
207207
overflow = Overflow,
208-
max_overflow = MaxOverflow} = State,
209-
case Workers of
210-
[Pid | Left] ->
208+
max_overflow = MaxOverflow,
209+
strategy = Strategy} = State,
210+
case get_worker_with_strategy(Workers, Strategy) of
211+
{{value, Pid}, Left} ->
211212
MRef = erlang:monitor(process, FromPid),
212213
true = ets:insert(Monitors, {Pid, CRef, MRef}),
213214
{reply, Pid, State#state{workers = Left}};
214-
[] when MaxOverflow > 0, Overflow < MaxOverflow ->
215+
{empty, _Left} when MaxOverflow > 0, Overflow < MaxOverflow ->
215216
{Pid, MRef} = new_worker(Sup, FromPid),
216217
true = ets:insert(Monitors, {Pid, CRef, MRef}),
217218
{reply, Pid, State#state{overflow = Overflow + 1}};
218-
[] when Block =:= false ->
219+
{empty, _Left} when Block =:= false ->
219220
{reply, full, State};
220-
[] ->
221+
{empty, _Left} ->
221222
MRef = erlang:monitor(process, FromPid),
222223
Waiting = queue:in({From, CRef, MRef}, State#state.waiting),
223224
{noreply, State#state{waiting = Waiting}}
@@ -228,7 +229,7 @@ handle_call(status, _From, State) ->
228229
monitors = Monitors,
229230
overflow = Overflow} = State,
230231
StateName = state_name(State),
231-
{reply, {StateName, length(Workers), Overflow, ets:info(Monitors, size)}, State};
232+
{reply, {StateName, queue:len(Workers), Overflow, ets:info(Monitors, size)}, State};
232233
handle_call(get_avail_workers, _From, State) ->
233234
Workers = State#state.workers,
234235
{reply, Workers, State};
@@ -266,10 +267,10 @@ handle_info({'EXIT', Pid, _Reason}, State) ->
266267
NewState = handle_worker_exit(Pid, State),
267268
{noreply, NewState};
268269
[] ->
269-
case lists:member(Pid, State#state.workers) of
270+
case queue:member(Pid, State#state.workers) of
270271
true ->
271-
W = lists:filter(fun (P) -> P =/= Pid end, State#state.workers),
272-
{noreply, State#state{workers = [new_worker(Sup) | W]}};
272+
W = filter_worker_by_pid(Pid, State#state.workers),
273+
{noreply, State#state{workers = queue:in(new_worker(Sup), W)}};
273274
false ->
274275
{noreply, State}
275276
end
@@ -279,7 +280,8 @@ handle_info(_Info, State) ->
279280
{noreply, State}.
280281

281282
terminate(_Reason, State) ->
282-
ok = lists:foreach(fun (W) -> unlink(W) end, State#state.workers),
283+
Workers = queue:to_list(State#state.workers),
284+
ok = lists:foreach(fun (W) -> unlink(W) end, Workers),
283285
true = exit(State#state.supervisor, shutdown),
284286
ok.
285287

@@ -304,26 +306,33 @@ new_worker(Sup, FromPid) ->
304306
Ref = erlang:monitor(process, FromPid),
305307
{Pid, Ref}.
306308

309+
get_worker_with_strategy(Workers, fifo) ->
310+
queue:out(Workers);
311+
get_worker_with_strategy(Workers, lifo) ->
312+
queue:out_r(Workers).
313+
307314
dismiss_worker(Sup, Pid) ->
308315
true = unlink(Pid),
309316
supervisor:terminate_child(Sup, Pid).
310317

318+
filter_worker_by_pid(Pid, Workers) ->
319+
queue:filter(fun (WPid) -> WPid =/= Pid end, Workers).
320+
311321
prepopulate(N, _Sup) when N < 1 ->
312-
[];
322+
queue:new();
313323
prepopulate(N, Sup) ->
314-
prepopulate(N, Sup, []).
324+
prepopulate(N, Sup, queue:new()).
315325

316326
prepopulate(0, _Sup, Workers) ->
317327
Workers;
318328
prepopulate(N, Sup, Workers) ->
319-
prepopulate(N-1, Sup, [new_worker(Sup) | Workers]).
329+
prepopulate(N-1, Sup, queue:in(new_worker(Sup), Workers)).
320330

321331
handle_checkin(Pid, State) ->
322332
#state{supervisor = Sup,
323333
waiting = Waiting,
324334
monitors = Monitors,
325-
overflow = Overflow,
326-
strategy = Strategy} = State,
335+
overflow = Overflow} = State,
327336
case queue:out(Waiting) of
328337
{{value, {From, CRef, MRef}}, Left} ->
329338
true = ets:insert(Monitors, {Pid, CRef, MRef}),
@@ -333,10 +342,7 @@ handle_checkin(Pid, State) ->
333342
ok = dismiss_worker(Sup, Pid),
334343
State#state{waiting = Empty, overflow = Overflow - 1};
335344
{empty, Empty} ->
336-
Workers = case Strategy of
337-
lifo -> [Pid | State#state.workers];
338-
fifo -> State#state.workers ++ [Pid]
339-
end,
345+
Workers = queue:in(Pid, State#state.workers),
340346
State#state{workers = Workers, waiting = Empty, overflow = 0}
341347
end.
342348

@@ -353,15 +359,14 @@ handle_worker_exit(Pid, State) ->
353359
{empty, Empty} when Overflow > 0 ->
354360
State#state{overflow = Overflow - 1, waiting = Empty};
355361
{empty, Empty} ->
356-
Workers =
357-
[new_worker(Sup)
358-
| lists:filter(fun (P) -> P =/= Pid end, State#state.workers)],
362+
W = filter_worker_by_pid(Pid, State#state.workers),
363+
Workers = queue:in(new_worker(Sup), W),
359364
State#state{workers = Workers, waiting = Empty}
360365
end.
361366

362367
state_name(State = #state{overflow = Overflow}) when Overflow < 1 ->
363368
#state{max_overflow = MaxOverflow, workers = Workers} = State,
364-
case length(Workers) == 0 of
369+
case queue:len(Workers) == 0 of
365370
true when MaxOverflow < 1 -> full;
366371
true -> overflow;
367372
false -> ready

‎test/poolboy_tests.erl‎

Lines changed: 35 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -127,37 +127,37 @@ transaction_timeout() ->
127127
pool_startup() ->
128128
%% Check basic pool operation.
129129
{ok, Pid} = new_pool(10, 5),
130-
?assertEqual(10, length(pool_call(Pid, get_avail_workers))),
130+
?assertEqual(10, queue:len(pool_call(Pid, get_avail_workers))),
131131
poolboy:checkout(Pid),
132-
?assertEqual(9, length(pool_call(Pid, get_avail_workers))),
132+
?assertEqual(9, queue:len(pool_call(Pid, get_avail_workers))),
133133
Worker = poolboy:checkout(Pid),
134-
?assertEqual(8, length(pool_call(Pid, get_avail_workers))),
134+
?assertEqual(8, queue:len(pool_call(Pid, get_avail_workers))),
135135
checkin_worker(Pid, Worker),
136-
?assertEqual(9, length(pool_call(Pid, get_avail_workers))),
136+
?assertEqual(9, queue:len(pool_call(Pid, get_avail_workers))),
137137
?assertEqual(1, length(pool_call(Pid, get_all_monitors))),
138138
ok = pool_call(Pid, stop).
139139

140140
pool_overflow() ->
141141
%% Check that the pool overflows properly.
142142
{ok, Pid} = new_pool(5, 5),
143143
Workers = [poolboy:checkout(Pid) || _ <- lists:seq(0, 6)],
144-
?assertEqual(0, length(pool_call(Pid, get_avail_workers))),
144+
?assertEqual(0, queue:len(pool_call(Pid, get_avail_workers))),
145145
?assertEqual(7, length(pool_call(Pid, get_all_workers))),
146146
[A, B, C, D, E, F, G] = Workers,
147147
checkin_worker(Pid, A),
148148
checkin_worker(Pid, B),
149-
?assertEqual(0, length(pool_call(Pid, get_avail_workers))),
149+
?assertEqual(0, queue:len(pool_call(Pid, get_avail_workers))),
150150
?assertEqual(5, length(pool_call(Pid, get_all_workers))),
151151
checkin_worker(Pid, C),
152152
checkin_worker(Pid, D),
153-
?assertEqual(2, length(pool_call(Pid, get_avail_workers))),
153+
?assertEqual(2, queue:len(pool_call(Pid, get_avail_workers))),
154154
?assertEqual(5, length(pool_call(Pid, get_all_workers))),
155155
checkin_worker(Pid, E),
156156
checkin_worker(Pid, F),
157-
?assertEqual(4, length(pool_call(Pid, get_avail_workers))),
157+
?assertEqual(4, queue:len(pool_call(Pid, get_avail_workers))),
158158
?assertEqual(5, length(pool_call(Pid, get_all_workers))),
159159
checkin_worker(Pid, G),
160-
?assertEqual(5, length(pool_call(Pid, get_avail_workers))),
160+
?assertEqual(5, queue:len(pool_call(Pid, get_avail_workers))),
161161
?assertEqual(5, length(pool_call(Pid, get_all_workers))),
162162
?assertEqual(0, length(pool_call(Pid, get_all_monitors))),
163163
ok = pool_call(Pid, stop).
@@ -167,7 +167,7 @@ pool_empty() ->
167167
%% overflow is enabled.
168168
{ok, Pid} = new_pool(5, 2),
169169
Workers = [poolboy:checkout(Pid) || _ <- lists:seq(0, 6)],
170-
?assertEqual(0, length(pool_call(Pid, get_avail_workers))),
170+
?assertEqual(0, queue:len(pool_call(Pid, get_avail_workers))),
171171
?assertEqual(7, length(pool_call(Pid, get_all_workers))),
172172
[A, B, C, D, E, F, G] = Workers,
173173
Self = self(),
@@ -192,18 +192,18 @@ pool_empty() ->
192192
after
193193
500 -> ?assert(false)
194194
end,
195-
?assertEqual(0, length(pool_call(Pid, get_avail_workers))),
195+
?assertEqual(0, queue:len(pool_call(Pid, get_avail_workers))),
196196
?assertEqual(5, length(pool_call(Pid, get_all_workers))),
197197
checkin_worker(Pid, C),
198198
checkin_worker(Pid, D),
199-
?assertEqual(2, length(pool_call(Pid, get_avail_workers))),
199+
?assertEqual(2, queue:len(pool_call(Pid, get_avail_workers))),
200200
?assertEqual(5, length(pool_call(Pid, get_all_workers))),
201201
checkin_worker(Pid, E),
202202
checkin_worker(Pid, F),
203-
?assertEqual(4, length(pool_call(Pid, get_avail_workers))),
203+
?assertEqual(4, queue:len(pool_call(Pid, get_avail_workers))),
204204
?assertEqual(5, length(pool_call(Pid, get_all_workers))),
205205
checkin_worker(Pid, G),
206-
?assertEqual(5, length(pool_call(Pid, get_avail_workers))),
206+
?assertEqual(5, queue:len(pool_call(Pid, get_avail_workers))),
207207
?assertEqual(5, length(pool_call(Pid, get_all_workers))),
208208
?assertEqual(0, length(pool_call(Pid, get_all_monitors))),
209209
ok = pool_call(Pid, stop).
@@ -213,7 +213,7 @@ pool_empty_no_overflow() ->
213213
%% disabled.
214214
{ok, Pid} = new_pool(5, 0),
215215
Workers = [poolboy:checkout(Pid) || _ <- lists:seq(0, 4)],
216-
?assertEqual(0, length(pool_call(Pid, get_avail_workers))),
216+
?assertEqual(0, queue:len(pool_call(Pid, get_avail_workers))),
217217
?assertEqual(5, length(pool_call(Pid, get_all_workers))),
218218
[A, B, C, D, E] = Workers,
219219
Self = self(),
@@ -238,14 +238,14 @@ pool_empty_no_overflow() ->
238238
after
239239
500 -> ?assert(false)
240240
end,
241-
?assertEqual(2, length(pool_call(Pid, get_avail_workers))),
241+
?assertEqual(2, queue:len(pool_call(Pid, get_avail_workers))),
242242
?assertEqual(5, length(pool_call(Pid, get_all_workers))),
243243
checkin_worker(Pid, C),
244244
checkin_worker(Pid, D),
245-
?assertEqual(4, length(pool_call(Pid, get_avail_workers))),
245+
?assertEqual(4, queue:len(pool_call(Pid, get_avail_workers))),
246246
?assertEqual(5, length(pool_call(Pid, get_all_workers))),
247247
checkin_worker(Pid, E),
248-
?assertEqual(5, length(pool_call(Pid, get_avail_workers))),
248+
?assertEqual(5, queue:len(pool_call(Pid, get_avail_workers))),
249249
?assertEqual(5, length(pool_call(Pid, get_all_workers))),
250250
?assertEqual(0, length(pool_call(Pid, get_all_monitors))),
251251
ok = pool_call(Pid, stop).
@@ -256,16 +256,16 @@ worker_death() ->
256256
{ok, Pid} = new_pool(5, 2),
257257
Worker = poolboy:checkout(Pid),
258258
kill_worker(Worker),
259-
?assertEqual(5, length(pool_call(Pid, get_avail_workers))),
259+
?assertEqual(5, queue:len(pool_call(Pid, get_avail_workers))),
260260
[A, B, C|_Workers] = [poolboy:checkout(Pid) || _ <- lists:seq(0, 6)],
261-
?assertEqual(0, length(pool_call(Pid, get_avail_workers))),
261+
?assertEqual(0, queue:len(pool_call(Pid, get_avail_workers))),
262262
?assertEqual(7, length(pool_call(Pid, get_all_workers))),
263263
kill_worker(A),
264-
?assertEqual(0, length(pool_call(Pid, get_avail_workers))),
264+
?assertEqual(0, queue:len(pool_call(Pid, get_avail_workers))),
265265
?assertEqual(6, length(pool_call(Pid, get_all_workers))),
266266
kill_worker(B),
267267
kill_worker(C),
268-
?assertEqual(1, length(pool_call(Pid, get_avail_workers))),
268+
?assertEqual(1, queue:len(pool_call(Pid, get_avail_workers))),
269269
?assertEqual(5, length(pool_call(Pid, get_all_workers))),
270270
?assertEqual(4, length(pool_call(Pid, get_all_monitors))),
271271
ok = pool_call(Pid, stop).
@@ -277,9 +277,9 @@ worker_death_while_full() ->
277277
{ok, Pid} = new_pool(5, 2),
278278
Worker = poolboy:checkout(Pid),
279279
kill_worker(Worker),
280-
?assertEqual(5, length(pool_call(Pid, get_avail_workers))),
280+
?assertEqual(5, queue:len(pool_call(Pid, get_avail_workers))),
281281
[A, B|_Workers] = [poolboy:checkout(Pid) || _ <- lists:seq(0, 6)],
282-
?assertEqual(0, length(pool_call(Pid, get_avail_workers))),
282+
?assertEqual(0, queue:len(pool_call(Pid, get_avail_workers))),
283283
?assertEqual(7, length(pool_call(Pid, get_all_workers))),
284284
Self = self(),
285285
spawn(fun() ->
@@ -306,7 +306,7 @@ worker_death_while_full() ->
306306
1000 -> ?assert(false)
307307
end,
308308
kill_worker(B),
309-
?assertEqual(0, length(pool_call(Pid, get_avail_workers))),
309+
?assertEqual(0, queue:len(pool_call(Pid, get_avail_workers))),
310310
?assertEqual(6, length(pool_call(Pid, get_all_workers))),
311311
?assertEqual(6, length(pool_call(Pid, get_all_monitors))),
312312
ok = pool_call(Pid, stop).
@@ -318,9 +318,9 @@ worker_death_while_full_no_overflow() ->
318318
{ok, Pid} = new_pool(5, 0),
319319
Worker = poolboy:checkout(Pid),
320320
kill_worker(Worker),
321-
?assertEqual(5, length(pool_call(Pid, get_avail_workers))),
321+
?assertEqual(5, queue:len(pool_call(Pid, get_avail_workers))),
322322
[A, B, C|_Workers] = [poolboy:checkout(Pid) || _ <- lists:seq(0, 4)],
323-
?assertEqual(0, length(pool_call(Pid, get_avail_workers))),
323+
?assertEqual(0, queue:len(pool_call(Pid, get_avail_workers))),
324324
?assertEqual(5, length(pool_call(Pid, get_all_workers))),
325325
Self = self(),
326326
spawn(fun() ->
@@ -346,10 +346,10 @@ worker_death_while_full_no_overflow() ->
346346
1000 -> ?assert(false)
347347
end,
348348
kill_worker(B),
349-
?assertEqual(1, length(pool_call(Pid, get_avail_workers))),
349+
?assertEqual(1, queue:len(pool_call(Pid, get_avail_workers))),
350350
?assertEqual(5, length(pool_call(Pid, get_all_workers))),
351351
kill_worker(C),
352-
?assertEqual(2, length(pool_call(Pid, get_avail_workers))),
352+
?assertEqual(2, queue:len(pool_call(Pid, get_avail_workers))),
353353
?assertEqual(5, length(pool_call(Pid, get_all_workers))),
354354
?assertEqual(3, length(pool_call(Pid, get_all_monitors))),
355355
ok = pool_call(Pid, stop).
@@ -359,7 +359,7 @@ pool_full_nonblocking_no_overflow() ->
359359
%% option to use non-blocking checkouts is used.
360360
{ok, Pid} = new_pool(5, 0),
361361
Workers = [poolboy:checkout(Pid) || _ <- lists:seq(0, 4)],
362-
?assertEqual(0, length(pool_call(Pid, get_avail_workers))),
362+
?assertEqual(0, queue:len(pool_call(Pid, get_avail_workers))),
363363
?assertEqual(5, length(pool_call(Pid, get_all_workers))),
364364
?assertEqual(full, poolboy:checkout(Pid, false)),
365365
?assertEqual(full, poolboy:checkout(Pid, false)),
@@ -374,7 +374,7 @@ pool_full_nonblocking() ->
374374
%% option to use non-blocking checkouts is used.
375375
{ok, Pid} = new_pool(5, 5),
376376
Workers = [poolboy:checkout(Pid) || _ <- lists:seq(0, 9)],
377-
?assertEqual(0, length(pool_call(Pid, get_avail_workers))),
377+
?assertEqual(0, queue:len(pool_call(Pid, get_avail_workers))),
378378
?assertEqual(10, length(pool_call(Pid, get_all_workers))),
379379
?assertEqual(full, poolboy:checkout(Pid, false)),
380380
A = hd(Workers),
@@ -395,17 +395,17 @@ owner_death() ->
395395
receive after 500 -> exit(normal) end
396396
end),
397397
timer:sleep(1000),
398-
?assertEqual(5, length(pool_call(Pid, get_avail_workers))),
398+
?assertEqual(5, queue:len(pool_call(Pid, get_avail_workers))),
399399
?assertEqual(5, length(pool_call(Pid, get_all_workers))),
400400
?assertEqual(0, length(pool_call(Pid, get_all_monitors))),
401401
ok = pool_call(Pid, stop).
402402

403403
checkin_after_exception_in_transaction() ->
404404
{ok, Pool} = new_pool(2, 0),
405-
?assertEqual(2, length(pool_call(Pool, get_avail_workers))),
405+
?assertEqual(2, queue:len(pool_call(Pool, get_avail_workers))),
406406
Tx = fun(Worker) ->
407407
?assert(is_pid(Worker)),
408-
?assertEqual(1, length(pool_call(Pool, get_avail_workers))),
408+
?assertEqual(1, queue:len(pool_call(Pool, get_avail_workers))),
409409
throw(it_on_the_ground),
410410
?assert(false)
411411
end,
@@ -414,7 +414,7 @@ checkin_after_exception_in_transaction() ->
414414
catch
415415
throw:it_on_the_ground -> ok
416416
end,
417-
?assertEqual(2, length(pool_call(Pool, get_avail_workers))),
417+
?assertEqual(2, queue:len(pool_call(Pool, get_avail_workers))),
418418
ok = pool_call(Pool, stop).
419419

420420
pool_returns_status() ->

0 commit comments

Comments
 (0)