Speed up brew module package install & upgrade

[UnknownPlatypus]

SUMMARY

I noticed while working with the homebrew module that it was quite slow, even when doing noops so I instrumented the code a bit to understand why.

For the following task:

- name: Install brew packages
  community.general.homebrew:
    name: "{{ thibaut_brew_packages }}"
    state: latest
    update_homebrew: true

I got the following logs:

_run starting ...'
    _update_homebrew starting ...'
    _update_homebrew took 992.34 ms'
    _upgrade_packages starting ...'
        _upgrade_current_package starting ...' (ALREADY INSTALLED CASE)
            _current_package_is_installed starting ...'
            _current_package_is_installed took 780.45 ms'
            _current_package_is_installed starting ...'
            _current_package_is_installed took 738.39 ms'
            _current_package_is_outdated starting ...'
            _current_package_is_outdated took 750.32 ms'
        _upgrade_current_package took 2269.26 ms'
        _upgrade_current_package starting ...'  (NOT INSTALLED CASE)
            _current_package_is_installed starting ...'
            _current_package_is_installed took 752.09 ms'
            _current_package_is_installed starting ...'
            _current_package_is_installed took 733.06 ms'
            _current_package_is_installed starting ...'
            _current_package_is_installed took 759.49 ms'
            _current_package_is_outdated starting ...'
            _current_package_is_outdated took 751.80 ms'
        _upgrade_current_package took 4387.61 ms'
...

Lot's of redundant commands that are quite expensive because of brew.
This PR addresses some quick wins, removing duplicated _current_package_is_installed calls in both example above.

However, this could be improved further. Some ideas for other PRs if interest arise:

• the current_package_is_outdated hook is always used with _current_package_is_installed, requiring 2 brew call while one brew info returns both info.
• _upgrade_current_package is called in a loop, leading to a lot of brew info / brew outdated calls (N+1 issue) when the list of package is large. It would be much more efficient to fetch every packages installed first (brew list --formula -1), every outdated packages (brew outdated --json=v2) and then just check against these lists.

ISSUE TYPE

• Refactoring Pull Request

COMPONENT NAME

homebrew

ADDITIONAL INFORMATION

The code I used to instrument the `Homebrew` class

import time
from functools import wraps

def timing_decorator(cls):
    class MethodCallsTimingWrapper(cls):
        log = []
        stack_level = 0
        def __init__(self, *args, **kwargs):
            super().__init__(*args, **kwargs)

        def __getattribute__(self, name):
            attr = object.__getattribute__(self, name)
            if callable(attr):
                @wraps(attr)
                def wrapper(*args, **kwargs):
                    start_time = time.monotonic()
                    self.log.append(f"{'    '*self.stack_level}{attr.__name__} starting ...")
                    self.stack_level += 1

                    result = attr(*args, **kwargs)

                    end_time = time.monotonic()
                    self.stack_level -= 1
                    self.log.append(f"{'    '*self.stack_level}{attr.__name__} took {(end_time - start_time) * 1000:.2f} ms")
                    return result
                return wrapper
            return attr

    return MethodCallsTimingWrapper

@timing_decorator
class Homebrew(object):

</details>

[ansibullbot]

cc @Akasurde @andrew-d @danieljaouen @indrajitr @kyleabenson @martinm82
click here for bot help

[felixfontein]

Thanks for your contribution! I don't know much about brew, so I cannot comment on the other changes, but I have a tiny note for the changelog fragment :)

[russoz]

LGTM

[felixfontein]

@UnknownPlatypus thanks for your contribution!
@russoz thanks for reviewing!

[UnknownPlatypus]

Thank you both for the quick review! Was a pleasure to contribute!

Would you be open to a follow-up PR addressing the other optimisation ideas I left in the PR description?

Cheers!

[felixfontein]

There is no active module maintainer for this module, so there's nobody to say "no" :) We generally accept all PRs that look good enough and don't fail CI.

[russoz]

Absolutely! Contributions are more than welcome! :-)

[UnknownPlatypus]

Awesome, I'll submit new PR's then and will try to keep them scoped clearly to ease review! Thanks!