methodical icon indicating copy to clipboard operation
methodical copied to clipboard

Attempt at widening the arities for multimethod calls

Open bshepherdson opened this issue 1 year ago • 1 comments

There's a lot of apply/RestFn/invoke etc. dynamic call machinery in Methodical's stack traces. This is an attempt to remove some of it by going up to 7 direct args for multimethod calls. (And dispatch functions.)

This hasn't removed much of the apply overhead in practice because with-meta on a function wraps it with a naive function subclass that always does a dynamic call. There are probably still some places that more dynamic calls are creeping in, but I ran out of time to dig deeper.

This may not go anywhere until I get back, but I wanted to publish this just in case.

Thanks for contributing to Methodical. Before open a pull request, please take a moment to:

  • [ ] Ensure the PR follows the Clojure Style Guide.

  • [ ] Tests and linters pass. You can run all of the tests and linters locally with

    ./scripts/lint-and-test.sh
    

    GitHub Actions will also run these same tests against your PR.

  • [ ] Make sure you've included new tests for any new features or bugfixes.

  • [ ] New features are documented, or documentation is updated appropriately for any changed features.

  • [ ] Carefully review your own changes and revert any superfluous ones. (A good example would be moving words in the Markdown documentation to different lines in a way that wouldn't change how the rendered page itself would appear. These sorts of changes make a PR bigger than it needs to be, and, thus, harder to review.)

    Of course, indentation and typo fixes are not covered by this rule and are always appreciated.
    
  • [ ] Include a detailed explanation of what changes you're making and why you've made them. This will help me understand what's going on while we review it.

Once you've done all that, open a PR! Thanks for your contribution!

bshepherdson avatar Jul 31 '24 16:07 bshepherdson

This can be closed now as the same changes were merged in #150.

alexander-yakushev avatar Sep 13 '24 21:09 alexander-yakushev