perf: eliminate O(docs) disk reads in check_all_figures_have_license - #67
Merged
Conversation
Previously the license summary check re-read every doctree pickle from disk (env.get_doctree) for all found_docs, even when the build had only modified a handful of documents. For large projects this caused significant I/O on every incremental build. New approach: - MetadataFigure.run() records each figure's image_uri and license value in env.metadata_figure_license_index[docname] as directives are processed. This is pure in-memory bookkeeping with no extra I/O. - check_all_figures_have_license iterates the index directly; it falls back to env.get_doctree only for documents not present in the index (e.g. figures emitted by third-party extensions or unchanged docs from a previous incremental build whose index entry was cleared). - clear_page_defaults now also clears the index for documents that are about to be rebuilt, preventing stale entries from surviving across incremental builds. - A new merge_figure_license_index handler wired to env-merge-info merges index data from parallel read workers back into the main env so that parallel_read_safe=True builds are handled correctly. https://claude.ai/code/session_01ApsmWPWsKBesMV1rq2VenA
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
check_all_figures_have_licensepreviously calledenv.get_doctree(docname)for every document inenv.found_docs, which reads and unpickles each doctree file from disk on every build — including incremental builds where only a few documents changed. For large projects this produced significant unnecessary I/O proportional to the total number of documents.Changes
MetadataFigure.run()— record each processed figure'simage_uriandlicensevalue intoenv.metadata_figure_license_index[docname]as a plain dict. This is pure in-memory bookkeeping with no extra I/O.check_all_figures_have_license()— iterate the index directly; fall back toenv.get_doctree()only for documents not present in the index (unchanged docs from a previous incremental build, or figures emitted by third-party extensions).clear_page_defaults()— also clearenv.metadata_figure_license_indexentries for documents that are about to be rebuilt, so stale entries from a previous run do not survive into the new build.merge_figure_license_index()— newenv-merge-infoevent handler that merges the index from each parallel read worker back into the main env, keepingparallel_read_safe = Truefully functional.Complexity
In the common incremental case where most docs are unchanged, the number of disk reads drops from O(N) to O(N-k), approaching zero for small changesets.
Test plan
summaries: trueinmetadata_figure_settings.licenseand run a full build; confirm missing/unrecognized license warnings still appear correctly.sphinx-build -j auto) and confirm the summary is complete (index merge working).MetadataFiguredirectives (standard.. figure::not overridden) still appear in the summary via the doctree fallback.https://claude.ai/code/session_01ApsmWPWsKBesMV1rq2VenA