Conversation
After running a task with `next_worker`, this task is run on the currently indicated index in the `next` field, `1` in this test, and then this field is atomically incremented (modulo the size), to `2` in this test. So when asking for the new index, this _should_ be the incremented value, two, so this test seems to have been covering an inconsistency on how the storage is recovered.
Codecov Report
@@ Coverage Diff @@
## main #183 +/- ##
==========================================
- Coverage 92.85% 92.79% -0.07%
==========================================
Files 10 10
Lines 448 430 -18
==========================================
- Hits 416 399 -17
+ Misses 32 31 -1
Continue to review full report at Codecov.
|
| expires(infinity) -> | ||
| infinity; | ||
| expires(Timeout) -> | ||
| case Timeout of | ||
| infinity -> | ||
| infinity; | ||
| Timeout -> | ||
| now_in_microseconds() + Timeout * 1000 | ||
| end. | ||
| now_in_microseconds() + Timeout * 1000. |
|
I don't understand what CodeCov is complaining about… So, I'll just ignore that thing and merge anyway… |
I analyzed the situation manually and I figured out the issue is just that the module has now fewer lines of code. 🤦🏻♂️ |
It's not terrible and that is a slightly utopic feature anyway… |
2 participants
Couldn't resist the temptation 😱
TL;DR;
The PT implementation spends A LOT less time finding the pool details. At some point perfs seems to complain we now spend more time in getting the process info than anything else (grabbing the main_lock for the process and seeing the message queue length and so on, can't get any faster than that by now).
How it is all done: the wpool record contains now one more field with all the worker names as a tuple, so that not only the names are pre-generated, but also getting them is a O(1)
element(N #wpool.workers)op. Also,#wpool.nextis now anatomics_ref(), so that the rotation for the next worker doesn't take any pt modifications – the idea is from https://www.erlang.org/blog/persistent_term/ and already successfully applied by me in https://github.com/esl/segmented_cache 😄Then all strategies just get the
#wpoolrecord from the persistent_term storage, and operate on fully shared and immutable data, so many other helper functions are removed in favour of justfind_wpool(Name).Code is quite opinionated, and we'd need to take very serious the fact that this would make dynamically adding or removing workers to a pool very very hard, as it would require modifying a persistent_term record.
Time for opinions 😄
reports
fprof
Note a general improvement of 25% over finding the worker and applying all the strategy, and also, some 25% less garbage collections.
In main:
{[{{wpool,call,4}, 100, 204.591, 0.317}], { {wpool_pool,best_worker,1}, 100, 204.591, 0.317}, % [{{wpool_pool,min_message_queue,1}, 100, 203.504, 0.419}, {{wpool_pool,find_wpool,1}, 100, 0.770, 0.439}]}. {[{{wpool_pool,best_worker,1}, 100, 203.504, 0.419}], { {wpool_pool,min_message_queue,1}, 100, 203.504, 0.419}, % [{{wpool_pool,min_message_queue,3}, 100, 201.402, 0.675}, {{rand,uniform,1}, 100, 1.551, 0.565}, {{erlang,setelement,3}, 100, 0.132, 0.132}]}. {[{{wpool_pool,min_message_queue,1}, 100, 201.402, 0.675}, {{wpool_pool,min_message_queue,3}, 10000, 0.000, 62.036}], { {wpool_pool,min_message_queue,3}, 10100, 201.402, 62.711}, % [{{wpool_pool,worker_name,2}, 10000, 38.456, 25.041}, {{wpool_pool,queue_length,1}, 10000, 37.420, 24.245}, {{wpool_pool,next_wpool,1}, 10000, 36.806, 24.847}, {{lists,min,1}, 100, 13.245, 0.152}, {{erlang,whereis,1}, 10000, 12.573, 12.311}, {garbage_collect, 33, 0.191, 0.191}, {{wpool_pool,min_message_queue,3}, 10000, 0.000, 62.036}]}. {[{{wpool_pool,min_message_queue,3}, 10000, 38.456, 25.041}], { {wpool_pool,worker_name,2}, 10000, 38.456, 25.041}, % [{{ets,lookup,2}, 10000, 13.344, 13.158}, {garbage_collect, 21, 0.071, 0.071}]}. {[{{wpool_pool,min_message_queue,3}, 10000, 37.420, 24.245}], { {wpool_pool,queue_length,1}, 10000, 37.420, 24.245}, % [{{erlang,process_info,2}, 10000, 13.175, 12.575}]}. {[{{wpool_pool,min_message_queue,3}, 10000, 36.806, 24.847}], { {wpool_pool,next_wpool,1}, 10000, 36.806, 24.847}, % [{{erlang,setelement,3}, 10000, 11.923, 11.692}, {garbage_collect, 9, 0.036, 0.036}]}. {[{{wpool_pool,best_worker,1}, 100, 0.770, 0.439}], { {wpool_pool,find_wpool,1}, 100, 0.770, 0.439}, % [{{ets,lookup,2}, 100, 0.183, 0.183}, {{erlang,whereis,1}, 100, 0.148, 0.141}]}. {[{{erlang,whereis,1}, 67, 0.269, 0.269}, {{erlang,process_info,2}, 46, 0.248, 0.248}, {{erlang,setelement,3}, 48, 0.231, 0.231}, {{wpool_pool,min_message_queue,3}, 33, 0.191, 0.191}, {{ets,lookup,2}, 49, 0.186, 0.186}, {{gen_server,call,3}, 14, 0.073, 0.073}, {{wpool_pool,worker_name,2}, 21, 0.071, 0.071}, {{wpool_pool,next_wpool,1}, 9, 0.036, 0.036}, {{erlang,integer_to_list,1}, 4, 0.021, 0.021}, {{io_lib_format,general_case_10,5}, 4, 0.018, 0.018}, {{io_lib_pretty,pp_tail,10}, 2, 0.010, 0.010}, {{io_lib_format_ryu_table,value,1}, 1, 0.009, 0.009}, {{erlang,'++',2}, 2, 0.007, 0.007}, {{string,uppercase_list,2}, 1, 0.006, 0.006}, {{lists,reverse,2}, 1, 0.006, 0.006}, {{io_lib_format,compute_shortest,6}, 1, 0.006, 0.006}, {{io_lib_format,convert_to_decimal,3}, 1, 0.004, 0.004}, {{io_lib_format,collect_cc,2}, 1, 0.004, 0.004}, {{gen,do_call,4}, 1, 0.004, 0.004}, {{io_lib_format,mulShiftAll,4}, 1, 0.003, 0.003}, {{io_lib_format,mulShift64,3}, 1, 0.003, 0.003}, {{gen,call,4}, 1, 0.003, 0.003}, {{lists,do_flatten,2}, 1, 0.001, 0.001}, {{io_lib_format,count_small,2}, 1, 0.001, 0.001}], { garbage_collect, 311, 1.411, 1.411}, % [ ]}.in the persistent_term branch:
{[{{wpool,call,4}, 100, 138.649, 0.243}], { {wpool_pool,best_worker,1}, 100, 138.649, 0.243}, % [{{wpool_pool,min_message_queue,1}, 100, 137.763, 0.257}, {{wpool_pool,find_wpool,1}, 100, 0.643, 0.364}]}. {[{{wpool_pool,best_worker,1}, 100, 137.763, 0.257}], { {wpool_pool,min_message_queue,1}, 100, 137.763, 0.257}, % [{{wpool_pool,min_message_queue,5}, 100, 136.132, 0.899}, {{rand,uniform,1}, 100, 1.374, 0.495}]}. {[{{wpool_pool,min_message_queue,1}, 100, 136.132, 0.899}, {{wpool_pool,min_message_queue,5}, 10000, 0.000, 55.225}], { {wpool_pool,min_message_queue,5}, 10100, 136.132, 56.124}, % [{{wpool_pool,queue_length,1}, 10000, 35.359, 23.258}, {{lists,min,1}, 100, 12.112, 0.115}, {{erlang,whereis,1}, 10000, 11.238, 10.997}, {{wpool_pool,next_to_check,2}, 10000, 10.575, 10.571}, {{wpool_pool,worker_name,2}, 10000, 10.532, 10.468}, {garbage_collect, 42, 0.192, 0.192}, {{wpool_pool,min_message_queue,5}, 10000, 0.000, 55.225}]}. {[{{wpool_pool,min_message_queue,5}, 10000, 35.359, 23.258}], { {wpool_pool,queue_length,1}, 10000, 35.359, 23.258}, % [{{erlang,process_info,2}, 10000, 12.101, 11.584}]}. {[{{wpool_pool,min_message_queue,5}, 100, 12.112, 0.115}], { {lists,min,1}, 100, 12.112, 0.115}, % [{{lists,min,2}, 100, 11.997, 0.118}]}. {[{{wpool_pool,best_worker,1}, 100, 0.643, 0.364}], { {wpool_pool,find_wpool,1}, 100, 0.643, 0.364}, % [{{erlang,whereis,1}, 100, 0.148, 0.144}, {{persistent_term,get,1}, 100, 0.131, 0.131}]}. {[{{erlang,process_info,2}, 40, 0.370, 0.370}, {{erlang,whereis,1}, 75, 0.245, 0.245}, {{wpool_pool,min_message_queue,5}, 42, 0.192, 0.192}, {{gen_server,call,3}, 14, 0.091, 0.091}, {{wpool_pool,worker_name,2}, 17, 0.064, 0.064}, {{erlang,monotonic_time,0}, 2, 0.014, 0.014}, {{io_lib_format,general_case_10,5}, 2, 0.011, 0.011}, {{erlang,integer_to_list,1}, 4, 0.011, 0.011}, {{wpool_bench,run_task,3}, 2, 0.008, 0.008}, {{erlang,'++',2}, 2, 0.008, 0.008}, {{io_lib_format,collect_cc,2}, 1, 0.007, 0.007}, {{lists,foldl,3}, 1, 0.006, 0.006}, {{lists,duplicate,3}, 1, 0.006, 0.006}, {{wpool_pool,next_to_check,2}, 2, 0.004, 0.004}, {{lists,reverse,2}, 1, 0.004, 0.004}, {{io_lib_format,bounds,6}, 1, 0.004, 0.004}, {{erlang,iolist_size,1}, 1, 0.004, 0.004}, {{erts_internal,time_unit,0}, 2, 0.002, 0.002}, {{io_lib_pretty,pp_tail,10}, 1, 0.001, 0.001}, {{io_lib_pretty,cind_tail,8}, 1, 0.001, 0.001}], { garbage_collect, 212, 1.053, 1.053}, % [ ]}.perf
Note how there's way more time now on timer:tc or in io:format, proportionally.
main
persistent_term