Resolve link topics against a per-run alias index#1902
Merged
Conversation
has_topic() previously called help() for every (topic, package) pair, and find_package() maps it over every dependency plus the base packages, so each unique linked topic cost ~50 help() searches (~1.5ms each). Instead, read each package's help/aliases.rds once per roxygenize() run (or scan man/ for packages loaded from source, like the package being documented) and answer lookups from that index. This also speeds up check_topic() and the Rd inheritance and R6 code paths that use has_topic(). Documenting testthat drops from 13.2s to 8.7s.
pkgload already maintains a cached alias index for packages loaded from source, built with tools::parse_Rd() so it handles Rd macros and multi-line constructs properly. Use it instead of a hand-rolled man/ scan, and restore the dev_topic_index_reset() call so the index is rebuilt each run.
…ckage Installed packages don't change between runs, so there's no need to re-read their alias indexes every run. The only package whose topics change from run to run is the one being documented, so find_package_cache_reset() evicts just that entry (from our cache and pkgload's). Failed lookups are never cached, so installing a missing dependency mid-session is picked up right away.
Two hotspots visible in profiles after the index change: * pkg_deps() re-read and re-parsed DESCRIPTION for every unresolved topic (~5% of a roxygenise() run); cache it per package dir, reset each run. * has_topic() used %in% on the alias vector, which rebuilds a hash table on every call (~8% of a run, mostly via the R6 and Rd inheritance paths, which have no per-topic cache in front of them). pkg_topics() now caches a hashed environment so each lookup is a single env_has().
Member
Author
|
Added the last two low-hanging items the combined-branch profile surfaced (8.2% + 5.0% of samples):
With these, link/topic resolution is essentially free; the remaining profile is the markdown XML walk (~23%, a structural project) and diffuse Rd assembly. |
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.
Profiling
roxygenise()on a link-heavy package (testthat) showed ~50% of the roxygen work inside link/topic resolution:has_topic()answers every query with autils::help()call (~1.5ms), andfind_package_lookup()maps it over all dependencies plus base packages, so each unique linked topic costs ~50 help-index searches. The per-topic cache added in #1724 only deduplicates repeated topics — every miss still pays the full scan.This PR replaces the
help()calls with a cached topic index per package:help/aliases.rdsindex that ships with the package (once per package per session — installed packages don't change between runs).pkgload::load_all()— reusepkgload::dev_topic_index(), the alias index thatdev_help()already maintains. (pkgload cached this before too, but the old path reached it throughhelp()→ shim →dev_topic_parse()/normalizePath()on every call, and the cache never applied to the ~50 installed dependencies, which were the bigger cost.)has_topic()is a singleenv_has();%in%on the alias vector rebuilt a hash table per membership test, which the R6 and Rd-inheritance paths (no per-topic cache in front of them) paid heavily.pkg_deps()is also cached per run — it re-read and re-parsedDESCRIPTIONfor every unresolved topic.find_package_cache_reset()evicts only the documented package's entry (from our cache and pkgload's) — its topics are the only ones that change between runs. Failed lookups are never cached, so installing a missing dependency mid-session is picked up immediately.check_topic()(validation of explicitly-qualified links) and the Rd-inheritance and R6 code paths that usehas_topic()get the same speedup for free. Re-export attribution (find_source()), the ambiguous-topic warning, and all warning messages are unchanged.Benchmark on a quiet machine (medians of 4 adjacent steady-state runs, spreads ±0.04s):
roxygenise()on testthat goes from 3.00s to 2.15s (1.4x), and to 2.05s (1.5x) combined with the markdown tokenizer rewrite in #1901. Note ~0.9s of every run is fixedload_all()+ Rd-writing overhead, so the roxygen-work portion roughly halves. Link resolution falls from ~50% of the profile to under 8%.🤖 Generated with Claude Code