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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃憣 Performance: Memoize Inline parser for build_need #968

Merged
merged 4 commits into from Aug 17, 2023

Conversation

chrisjsewell
Copy link
Member

@chrisjsewell chrisjsewell commented Aug 16, 2023

As discussed in #967, build_need (called from print_need_nodes) is currently taking a large amount of build times.

A key reason for this is that the initialisation of the inline parser, which requires compiling numerous regexes, is being run on every call.
This is not required, since the initialisation is invariant.

In this PR therefore, we move the initialisation to a cached function, which will only run once per build.

As shown when using phinx-analysis --project needs --pyinstrument --tree --tree-filter 0.01, compared to #967, this significantly reduces the print_need_nodes component of the build time; from taking ~25% to ~8%

In this example it reduces the total build time from ~1.6 seconds to ~1.2 seconds

image

@danwos
Copy link
Member

danwos commented Aug 16, 2023

Wow cool, learned some interesting new caching features from the code.
The code looks good.

Can you add also a small line to our /docs/cahngelog.rst file?
Feel also free to add yourself to our AUTHORS file as well.

I'm looking forward to merge this and see it in action for bigger projects.

@chrisjsewell
Copy link
Member Author

Can you add also a small line to our /docs/cahngelog.rst file?
Feel also free to add yourself to our AUTHORS file as well.

done @danwos 馃憤

Copy link
Member

@danwos danwos left a comment

Choose a reason for hiding this comment

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

Thanks, looks good 馃憤

@danwos danwos merged commit 11d6b1b into master Aug 17, 2023
11 checks passed
@danwos danwos deleted the print_need_nodes branch August 17, 2023 04:35
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.

None yet

2 participants