Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Attempt at widening the arities for multimethod calls #149

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bshepherdson
Copy link
Contributor

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!

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.
@alexander-yakushev
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants