I won't have time until sometime later, but note that I had 100 actual debug tasks, not a loop of 100 items.
On Tue, Mar 23, 2021 at 4:50 PM Alex Willmer <[email protected]> wrote: > Matt, > Could you share your benchmark. I'm seeing barely any difference > > With lock: > > ± hyperfine -- "ansible-playbook --forks 25 -i 100_hosts.yml > 100_debugs.yml"*Benchmark #1*: ansible-playbook --forks 25 -i 100_hosts.yml > 100_debugs.yml > Time (*mean* ± σ): * 6.744 s* ± 0.107 s [User: 27.194 s, System: > 1.963 s] > Range (min … max): 6.537 s … 6.829 s 10 runs > > Without lock > > $ hyperfine -- "ansible-playbook --forks 25 -i 100_hosts.yml > 100_debugs.yml"*Benchmark #1*: ansible-playbook --forks 25 -i 100_hosts.yml > 100_debugs.yml > Time (*mean* ± σ): * 6.675 s* ± 0.090 s [User: 27.160 s, System: > 1.936 s] > Range (min … max): 6.499 s … 6.748 s 10 runs > > $ git rev-parse HEAD > c4e211a4291162c17054f44ad5381856ffff802f > $ git diff | cat > diff --git a/lib/ansible/plugins/strategy/__init__.py > b/lib/ansible/plugins/strategy/__init__.py > index 70dea1ea6f..31e0f58eb5 100644 > --- a/lib/ansible/plugins/strategy/__init__.py > +++ b/lib/ansible/plugins/strategy/__init__.py > @@ -101,14 +101,13 @@ def results_thread_main(strategy): > strategy._tqm.send_callback(result.method_name, > *result.args, **result.kwargs) > elif isinstance(result, TaskResult): > strategy.normalize_task_result(result) > - with strategy._results_lock: > - # only handlers have the listen attr, so this must be > a handler > - # we split up the results into two queues here to > make sure > - # handler and regular result processing don't cross > wires > - if 'listen' in result._task_fields: > - strategy._handler_results.append(result) > - else: > - strategy._results.append(result) > + # only handlers have the listen attr, so this must be a > handler > + # we split up the results into two queues here to make > sure > + # handler and regular result processing don't cross wires > + if 'listen' in result._task_fields: > + strategy._handler_results.append(result) > + else: > + strategy._results.append(result) > else: > display.warning('Received an invalid object (%s) in the > result queue: %r' % (type(result), result)) > except (IOError, EOFError): > @@ -223,7 +222,6 @@ class StrategyBase: > > self._results = deque() > self._handler_results = deque() > - self._results_lock = threading.Condition(threading.Lock()) > > # create the result processing thread for reading results in the > background > self._results_thread = > threading.Thread(target=results_thread_main, args=(self,)) > @@ -524,15 +522,12 @@ class StrategyBase: > cur_pass = 0 > while True: > try: > - self._results_lock.acquire() > if do_handlers: > task_result = self._handler_results.popleft() > else: > task_result = self._results.popleft() > except IndexError: > break > - finally: > - self._results_lock.release() > > original_host = task_result._host > original_task = task_result._task > > $ cat 100_hosts.yml > all: > hosts: > local[001:100]: > vars: > ansible_connection: local > > $ cat 100_debugs.yml > - hosts: all > gather_facts: false > tasks: > - debug: > loop: "{{ range(100) }}" > > > On Mon, 22 Mar 2021 at 19:43, Matt Martz <[email protected]> wrote: > >> I'll honestly say I was surprised too. I expected the opposite. These are >> questions that chew away at my days; Running endless performance profiling. >> >> On Mon, Mar 22, 2021 at 2:41 PM Alex Willmer <[email protected]> wrote: >> >>> Huh, I would not have guessed that. Thanks, I'll try that next time I >>> can't explain the presence of some code. >>> >>> On Mon, 22 Mar 2021 at 15:39, Matt Martz <[email protected]> wrote: >>> >>>> I did some basic performance testing here, and the lock as currently >>>> implemented provides a decent performance improvement. >>>> >>>> On my machine with 100 hosts, 100 debug tasks, and 25 forks, I found >>>> that removing the lock increased execution time by close to 60 seconds. >>>> >>>> Based on that, I'd be hesitant to remove it. >>>> >>>> On Fri, Mar 19, 2021 at 5:41 PM [email protected] < >>>> [email protected]> wrote: >>>> >>>>> >>>>> ansible.plugins.strategy.StrategyBase._results_lock is acquired to >>>>> control access to StrategyBase._results and StrategyBase._handler_results. >>>>> Specifically >>>>> >>>>> - in ansible.plugins.strategy.results_thread_main() before append() >>>>> ing a result >>>>> - in StrategyBase_process_pending_results() before popleft() ing >>>>> >>>>> However StrategyBase._results & _handler_results are both >>>>> collections.deque objects. From the Python docs >>>>> >>>>> > Deques support thread-safe, memory efficient appends and pops from >>>>> either side of the deque. >>>>> >>>>> Thus AFAICT the lock isn't needed for the deque operations, and could >>>>> possibly be removed. Have I missed something? Is the lock providing >>>>> another >>>>> protection I've not noticed? Is it there as a belt and braces measure? >>>>> >>>>> If there's no reason to have it, and interest in removing it, I'll >>>>> submit a PR. Alternatively maybe there's benefit in keeping the lock and >>>>> using wait()/notify() instead of some of the polling. >>>>> >>>>> Regards, Alex >>>>> >>>>> -- >>>>> You received this message because you are subscribed to the Google >>>>> Groups "Ansible Development" group. >>>>> To unsubscribe from this group and stop receiving emails from it, send >>>>> an email to [email protected]. >>>>> To view this discussion on the web visit >>>>> https://groups.google.com/d/msgid/ansible-devel/3a309938-189d-4b8b-b9c4-539afea81984n%40googlegroups.com >>>>> <https://groups.google.com/d/msgid/ansible-devel/3a309938-189d-4b8b-b9c4-539afea81984n%40googlegroups.com?utm_medium=email&utm_source=footer> >>>>> . >>>>> >>>> >>>> >>>> -- >>>> Matt Martz >>>> @sivel >>>> sivel.net >>>> >>> >>> >>> -- >>> Alex Willmer <[email protected]> >>> >> >> >> -- >> Matt Martz >> @sivel >> sivel.net >> > > > -- > Alex Willmer <[email protected]> > -- Matt Martz @sivel sivel.net -- You received this message because you are subscribed to the Google Groups "Ansible Development" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/ansible-devel/CAD8N0v8%2BLZ-_cMuVJ5kU9J7%3D0H1Wat-AFTcU1cK%3DFZnGcYaoxw%40mail.gmail.com.
