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

Cache the CMakeDeps Jinja2 templates #13857

Merged

Conversation

fredizzimo
Copy link
Contributor

Changelog: Fix: Speed up the CMakeDeps generation
Docs: omit

  • Refer to the issue that supports this Pull Request.
  • If the issue has missing info, explain the purpose/use case/pain/need that covers this Pull Request.
  • I've read the Contributing guide.
  • I've followed the PEP8 style guides for Python code.
  • I've opened another PR in the Conan docs repo to the develop branch, documenting this one.

The compilation of the Jinja2 templates is quite slow, and several templates needs to be compiled per dependency, so for larger projects with a lot of dependencies it can be quite slow. This is fixed by caching the templates per class instance. For a medium size project with around 80 packages and dependencies I saw an improvement from 1:10 seconds to 10 seconds with this fix, so it's quite a bit faster.

NOTE: I'm not particularly happy with the implementation itself, but any alternative implementation I can think of need all the template files to be modified. So I decided to go with this non-intrusive implementation. But I can change it if you prefer.

Fixes #13845

The compilation of the Jinja2 templates is quite slow, and several
templates needs to be compiled per dependency, so for larger projects
with a lot of dependencies it can be quite slow. This is fixed by
caching the templates per class instance.
Copy link
Member

@memsharded memsharded left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

I have realized that there is a potentially unnecessary slow textwrap.dedent() for every large string, for every file, and this PR also optimizes it, not only jinja.

undefined=jinja2.StrictUndefined).render(context)

# Cache the template instance as a class attribute to greatly speed up the rendering
# NOTE: this assumes that self.template always returns the same string
Copy link
Member

Choose a reason for hiding this comment

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

Doubled checked, all .template are indeed constant strings, all good.

@memsharded memsharded added this to the 2.0.5 milestone May 9, 2023
@memsharded
Copy link
Member

NOTE: I'm not particularly happy with the implementation itself, but any alternative implementation I can think of need all the template files to be modified. So I decided to go with this non-intrusive implementation. But I can change it if you prefer.

I was also initially thinking something a bit more explicit, without getattr(type(self)..., but the PR is short, only in one single place instead of multiple files, so I think it has merit, and is good enough to move forward. Thanks again!

@memsharded memsharded merged commit 9113454 into conan-io:release/2.0 May 9, 2023
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.

[bug] The CMakeDeps generation is very slow
2 participants