Skip to content

Pre-load Jinja macros #13502

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

Merged
merged 9 commits into from
May 29, 2025
Merged

Conversation

jan-cerny
Copy link
Collaborator

Description:

Currently, the scripts building our content spend most of the time by loading Jinja macro definitions. Before processing each file, all Jinja macro definitions need to be reloaded, which means they are loaded more than thousand times.

The build could be optimized by loading the macros only once at the beginning of processing and then reusing them during the build.

Unfortunately, this can't be done with our current Jinja macros. Some of our macros use global variables like rule_id, rule_title. The problem is once a macro is loaded by Jinja, the global variables are permanently bound to values at the time of the loading and the binding can't be changed later. Only variables that are macro arguments can be changed dynamically. This means that to be able to reuse already loaded macros, all variables that are not constant need to be converted to macros parameters and their values always need to be passed from the Jinja template.

Therefore, as a part of this PR, we will replace all occurences of non-constant global variables by macro parameters and we will pass these parameters to macros in all our files where the changed macros are used. This is a massive change but not complex - the affected macros simply got new parameters rule_id or rule_title or both.

The changes in macro definitions and macro invocations have been done automatically using a script. This script is change_jinja_signatures.py and is added and then removed by this PR.

After the change of macros, this PR changes the build system logic to pre-load the Jinja macros at the beginning of the Python build scripts that perform the Jinja rendering so that the macros are loaded just once in each script.

Rationale:

This PR brings significant speed improvements of the build process. Specifically, ./build_product -d rhel9 takes 18 s instead of 33 s on a laptop. The speed up enhances developer experience and saves resources.

Review Hints:

  1. Review the one-off script change_jinja_signatures.py. Run unit tests in this script using python3 -m pytest change_jinja_signatures.py.
  2. Build data streams with current master and with this PR and compare them by diff.

jan-cerny added 8 commits May 27, 2025 09:29
The "products" variable is set by build system to the value of
"product" variable, which is constant during the build of content
for a product.
Macros defined in `harden_ssh_client_crypto_policy/oval/shared.xml`
will be renamed to prevent conflicts with macros of the same
name defined in `shared/macros/10-oval.jinja`.
The macro `oval_metadata` might be already loaded from the time when
there was a different `product` value in scope. To ensure that this
old value doesn't affect the generated OVAL we will specify affected
platforms explicitely.
@jan-cerny jan-cerny requested a review from matusmarhefka as a code owner May 27, 2025 14:23
@jan-cerny jan-cerny added the Infrastructure Our content build system label May 27, 2025
@Mab879 Mab879 self-assigned this May 27, 2025
@Mab879 Mab879 added this to the 0.1.78 milestone May 27, 2025
@Mab879
Copy link
Member

Mab879 commented May 27, 2025

/packit retest-failed

1 similar comment
@Mab879
Copy link
Member

Mab879 commented May 27, 2025

/packit retest-failed

Reduce cognitive complexity by extracting a function and inverting
a condition.
Copy link

codeclimate bot commented May 29, 2025

Code Climate has analyzed commit 9b1dae9 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 88.8% (50% is the threshold).

This pull request will bring the total coverage in the repository to 61.8% (-0.1% change).

View more on Code Climate.

@Mab879 Mab879 merged commit d24ea01 into ComplianceAsCode:master May 29, 2025
124 of 132 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Infrastructure Our content build system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants