Lazy page index in RDoc::Store (42.7% perf improvement)

[skatkov]

RDoc::Store#page previously scanned every known file for each page lookup, including repeated misses.

This adds a lazily built hash index keyed by both page_name and base_name, preserving existing first-match behavior when names collide.

The index is invalidated whenever files are added, removed, or bulk-loaded. The implementation remains private to RDoc::Store and adds no public API or configuration.

Benchmark

For testing I used latest rdoc(master) + rdoc-markdown gem. Generation was done on a rails codebase.

Metric	Before	After	Change
Generation time	74.91s	42.08s	-43.8%
Total time	88.52s	50.76s	-42.7%
Peak RSS	411.1 MiB	422.8 MiB	+2.8%

Total time to generate documentation decreased from 88.52s to 50.76s (42.7%).

[github-actions]

Documentation preview

View the preview

Commit: a3a4d09

[Copilot]

🟢 Approval recommended

The indexed lookup preserves existing semantics and is covered across mutation and loading paths.

Pull request overview

Adds a lazy page lookup index to improve repeated page resolution performance while preserving first-match behavior.

Changes:

• Indexes page and base names on first lookup.
• Invalidates the index when files change or load.
• Adds performance and invalidation tests.

File summaries

File	Description
lib/rdoc/store.rb	Implements and invalidates the lazy page index.
test/rdoc/rdoc_store_test.rb	Tests scaling and cache invalidation.

Review details

• Files reviewed: 2/2 changed files
• Comments generated: 0
• Review effort level: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[tompng]

Thanks, this is a solid win and the first-match test is much appreciated.

One thought, from context that isn't visible in the diff: Store#page is only
reached from CrossReference#resolve, i.e. during generation, which always runs
after Store#complete (rdoc, rdoc --server) or Store#load_all (ri servlet).
@unique_classes already relies on exactly this and is built in complete.

So the index could be built once in complete and at the end of load_all,
with no invalidation on add/remove at all. That also removes the three
@page_index = nil lines, which are easy to miss if another mutation path is
added later. Something like:

def complete(min_visibility)
  ...
  @page_index = build_page_index
end

# (A comment that `complete` is needed to use this method)
def page(name)
  # This will raise error if page_index isn't built yet.
  # It's OK and it should, so that we can detect invalid usage of this method
  @page_index[name]
end

[Copilot]

🟢 Approval recommended

The index preserves existing behavior and is invalidated across all internal file mutation paths.

Review details

• Files reviewed: 3/3 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[skatkov]

@tompng Interesting proposal, but I don't think this will work out.

always runs after Store#complete (rdoc, rdoc --server) or Store#load_all (ri servlet).
@unique_classes already relies on exactly this and is built in complete.

RubyGemsHook does not work with any of these finalization approaches. It delegates parsing to RDoc::RDoc#parse_files, which populates the store without finalizing it.

This is a Codex explanation:

Call Flow

RubyGemsHook#generate
  -> collect spec.source_paths + spec.extra_rdoc_files
  -> RDoc#parse_files
    -> RDoc#parse_file
      -> Store#add_file
      -> Parser.for
      -> parser.scan
  -> RubyGemsHook#document
    -> generator.generate

For the test gem, extra_rdoc_files contains README:

# test/rdoc/rdoc_rubygems_hook_test.rb:15
s.extra_rdoc_files = %w[README]

RubyGemsHook#generate adds those files to the RDoc arguments:

# lib/rdoc/rubygems_hook.rb:168-170
args = @spec.rdoc_options
args.concat @spec.source_paths
args.concat @spec.extra_rdoc_files

After option parsing, it creates a store and parses every selected file:

# lib/rdoc/rubygems_hook.rb:198-200
@rdoc.options = parse_options
@rdoc.store = Store.new(parse_options)
@rdoc.parse_files parse_options.files

RDoc#parse_file inserts each file into the store:

# lib/rdoc/rdoc.rb:337-343
top_level = @store.add_file filename, relative_name: relative_path_for(filename)
parser = Parser.for top_level, content, @options, @stats
parser.scan

For a plain file such as README, Parser::Simple is selected. It includes Parser::Text, so assigning the parser causes TopLevel#parser= to register it as a text page:

# lib/rdoc/code_object/top_level.rb:68-71
def parser=(val)
  @parser = val
  @store&.cache_text_file(relative_name)
  @parser
end

Finalization Difference

Normal RDoc#document explicitly finalizes the store:

# lib/rdoc/rdoc.rb:530-552
file_info = parse_files @options.files
# ...
@store.complete @options.visibility
# ...
generate

RubyGemsHook bypasses RDoc#document. After parse_files, it directly constructs and runs generators:

# lib/rdoc/rubygems_hook.rb:203-207
document 'ri', options, @ri_dir
document 'aliki', options, @rdoc_dir

Therefore, its store contains pages and classes, but @page_index remains unset.

Why Existing Tests May Miss It

Exact filenames such as README can resolve through Store#find_file_named before Store#page is reached. While rendering comments, however, an unresolved page-like reference can reach:

# lib/rdoc/cross_reference.rb:204-205
ref = @store.page name if not ref and name =~ /^[\w.\/]+$/

At that point, the new guard raises because neither complete nor load_all ran.

[tompng]

You're right, and thanks for checking this properly. I missed the RubyGemsHook path.

RubyGemsHook skipping Store#complete looks like its own issue
(no remove_nodoc, no visibility filtering), but that's independent of this PR.

[tompng]

Thank you 👍