Skip to content

feat(gcp-sphinx-docfx-yaml): optimize performance - #18588

Merged
daniel-sanche merged 11 commits into
mainfrom
investigate_post_submit_checks_gcp-sphinx-docfx-yaml_01_speed-optimizations
Oct 8, 2026
Merged

daniel-sanche merged 11 commits into
mainfrom
investigate_post_submit_checks_gcp-sphinx-docfx-yaml_01_speed-optimizations

Conversation

@daniel-sanche

@daniel-sanche daniel-sanche commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Previously, docfx would take > 3 hours on large PR. The compute package alone would take > 70 minutes, so this wasn't a problem that could be solved with sharding

image

This PR attempts to speed up the task by adding optimizations to the gcp-sphinx-docfx-yaml package:

  • 2bb74d8: sort known ids once, instead of inside a loop
  • 7a1c741: immediately skip over words without "." characters, since they can't be uuids
  • c54391e: use custom no-op StandaloneHTMLBuilder class, to avoid doing rendering work for unused html files
  • 7b9ab23: use in-process markdown builder, instead of spanning new process and starting from scratch
  • 8dedfb7: cache default settings, instead of re-constructing on each node visit
  • 7b91c2d: cache class line numbers, instead of re-building for each one
  • 9effc20: disable unused sphinx hooks
  • 28478f8: prefer native yaml parser when available. Fall back to pure-python when needed
  • 6d9bb07: added fast-path to skip non-google symbols

I had gemini run a comparison between these outputs, and outputs from the previous version, and it saw identical outputs

A follow-up PR re-adds docfx as a sharded pre-submit, with all jobs finishing < 15 mins

@daniel-sanche daniel-sanche changed the title feat(gcp-sphinx-docfx-yaml): introduce performance optimizations [DRAFT] feat(gcp-sphinx-docfx-yaml): introduce performance optimizations Oct 7, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request optimizes cross-reference conversion by skipping plain words that do not contain dots and hoisting the sorting of known_uids out of a loop. However, the changes introduce a critical NameError because hard_coded_references is referenced in find_uid_to_convert where it is not in scope. The review feedback suggests reverting this reference and instead combining the collections as immutable tuples within convert_cross_references to safely avoid shared state mutation.

Comment thread packages/gcp-sphinx-docfx-yaml/docfx_yaml/extension.py
Comment thread packages/gcp-sphinx-docfx-yaml/docfx_yaml/extension.py
@daniel-sanche daniel-sanche changed the title [DRAFT] feat(gcp-sphinx-docfx-yaml): introduce performance optimizations feat(gcp-sphinx-docfx-yaml): introduce performance optimizations Oct 7, 2026
@daniel-sanche
daniel-sanche marked this pull request as ready for review October 7, 2026 18:54
@daniel-sanche
daniel-sanche requested a review from a team as a code owner October 7, 2026 18:54
@daniel-sanche daniel-sanche changed the title feat(gcp-sphinx-docfx-yaml): introduce performance optimizations feat(gcp-sphinx-docfx-yaml): optimize performance Oct 7, 2026
@daniel-sanche
daniel-sanche added this pull request to stack #18607 October 8, 2026 16:33
@daniel-sanche
daniel-sanche force-pushed the investigate_post_submit_checks_gcp-sphinx-docfx-yaml_01_speed-optimizations branch from 80ed8b3 to 6b656c5 Compare October 8, 2026 16:37
@daniel-sanche
daniel-sanche merged commit cf1a0e5 into main Oct 8, 2026
52 checks passed
@daniel-sanche
daniel-sanche deleted the investigate_post_submit_checks_gcp-sphinx-docfx-yaml_01_speed-optimizations branch October 8, 2026 17:58
@release-please release-please Bot mentioned this pull request Oct 8, 2026
daniel-sanche added a commit that referenced this pull request Oct 8, 2026
Docfx was recently moved to a post-submit test due to its speed. With
sharding, combined with [performance optimizations in
`gcp-sphinx-docfx-yaml`](#18588),
we can run it within 10 minutes, and keep it pre-submit

This PR also installs `gcp-sphinx-docfx-yaml` from HEAD for the docfx
test, rather than from pypi
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.

2 participants