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

👌 Improve performance of needs builders #1054

Merged
merged 1 commit into from
Oct 31, 2023

Conversation

chrisjsewell
Copy link
Member

@chrisjsewell chrisjsewell commented Oct 28, 2023

closes #957

This PR refactors the needs and needs_id builders, so that they can completely skip the "write" phase of the Sphinx build.

The main aim of these builders is to output the final needs data, in a particular format.
The needs data is collected during the "read" phase of the Sphinx build, i.e. when all documents are read, then after all needs data has been collected, a number of "post-processing" functions are applied to the needs data.
This post-processing is independant of (a) the individual documents, (b) the output format, and so does not require the Sphinx "write" phase to run.

Here we combine all of the post-processing into a single post_process_needs_data function that can be called directly by these builders.
Note, ideally this would be called in a Sphinx event occuring between the "read" and "write" events, however, this does not currently exist (the closest is env-check-consistency, but this is not triggered on "write-only" rebuilds).

The only corner case for skipping the "write" phase, is when the needs builder is required to also export filters, due to the presence of export_id options on certain directives.
These filters are currently computed during the "write" phase, so here we do run that, but emit a warning, to let the user know of this degradation in performance.

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.

Great work and could catch with the export_id corner case. 👍

@chrisjsewell chrisjsewell force-pushed the needs-json-fast-build branch from 15966bc to fdae517 Compare October 30, 2023 15:31
@chrisjsewell chrisjsewell merged commit 6d0952b into master Oct 31, 2023
@chrisjsewell chrisjsewell deleted the needs-json-fast-build branch October 31, 2023 05:44
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.

needs.json builder performance
2 participants