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

perf: make ape --help faster #2333

Merged
merged 6 commits into from
Oct 25, 2024
Merged

perf: make ape --help faster #2333

merged 6 commits into from
Oct 25, 2024

Conversation

antazoey
Copy link
Member

@antazoey antazoey commented Oct 22, 2024

Makes ape --help faster by:

  1. Making all imports as lazy as possible, only importing what is needed everywhere
  2. Use module-level __getattr__ for exports (__init__.py files in root of all plugins) so things are loading as requested and not on initial module load
  3. ^ similar thing with _cli.py files - make all the slows imports have locally and lazier

Before:

ape --help  4.04s user 0.62s system 124% cpu 3.747 total

After:

ape --help  1.45s user 0.17s system 99% cpu 1.629 tota

Some of the work was broken out in smaller PRs to keep this draft simpler:

#2334
#2335
#2336
#2337
#2338

and i can probably do more if still needed

NOTE: Any CLI plugin needs to also be updated. Installing ape-tokens with this branch renders the benefits null unless it also follows suite and making its __init__.py and _cli.py files load faster.

Plugin PRs:

ApeWorX/ape-tokens#46
TBD vyper, solidity and whatever else

@antazoey antazoey marked this pull request as draft October 22, 2024 21:21
src/ape/_cli.py Outdated Show resolved Hide resolved
@antazoey
Copy link
Member Author

antazoey commented Oct 24, 2024

  • Fix root methoddocs (is empty now)

@fubuloubu
Copy link
Member

fubuloubu commented Oct 24, 2024

I hate to say this, but with some of the "public" api moving around would it make sense to call this a v0.9 breaking change?

@antazoey
Copy link
Member Author

antazoey commented Oct 24, 2024

I hate to say this, but with some of the "public" api moving around would it make sense to call this a v0.9 breaking change?

I think all of the original imports still work though, but I was going to cross check all the plugins.

@antazoey
Copy link
Member Author

I hate to say this, but with some of the "public" api moving around would it make sense to call this a v0.9 breaking change?

Like for example, if something was in ape.types.__init__ and was moved to a submodule, the import from ape.types should still work for users. That was intent everywhere anyway.

Internally, we are importing from more direct sources but that is just for making Ape faster and not a clear indication of public API usage.

I am still testing all the plugins just to be sure, and can test ApePay as well.

@antazoey
Copy link
Member Author

I did find something with ApePay but technically ApePay was already doing it wrong.
I can still re-expose the values from ape.types just in case but this never should have worked to begin with.
(albeit ape.types may be a better namespace for the base model stuff, maybe a 0.9-ism)

ApeWorX/ApePay#115

@antazoey
Copy link
Member Author

@fubuloubu OK, I have tested a bunch of stuff to ensure nothing is breaking anywhere...
I am still down for punting this to 0.9 if you want, but I feel like it is pretty safe. You can see everything I checked in this comment: #2333 (comment)

Also - down to check more repos and such too.

Thoughts?

Copy link
Member

@fubuloubu fubuloubu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thoughts?

LFG

@antazoey antazoey merged commit 85eddb8 into ApeWorX:main Oct 25, 2024
16 checks passed
@antazoey antazoey deleted the perf/help branch October 25, 2024 17:55
fubuloubu added a commit to ApeWorX/uniswap-sdk that referenced this pull request Oct 26, 2024
fubuloubu added a commit to ApeWorX/uniswap-sdk that referenced this pull request Oct 26, 2024
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.

3 participants