Reduce memory allocations for RI and POT generators - #1788
Conversation
GitHub Actions
Waiting
55caf23 to
e89f08c
Compare
GitHub Actions
Waiting
RDoc currently syntax-highlights and retains every Ruby method body during parsing, regardless of whether the selected generator renders method source. This PR adds a generator capability for controlling method-source collection. RI and POT opt out, while Darkfish, Aliki, and unknown third-party generators retain the existing behavior. This reduces roughly 50% of allocations for generators that opt-out.
e89f08c to
afe4edb
Compare
GitHub Actions
Waiting
GitHub Actions
Waiting
| def store_method_source? | ||
| return false if @coverage_report | ||
|
|
||
| !@generator.respond_to?(:store_method_source?) || @generator.store_method_source? |
There was a problem hiding this comment.
We need this respond_to? for custom generators 👍
But can we define store_method_source? = true for all existing generators in this repository?
| # Returns whether syntax-highlighted method source should be stored. | ||
|
|
||
| #: () -> bool | ||
| def store_method_source? |
There was a problem hiding this comment.
RDoc::RubyGemsHook#generate parses files once and then generates both formats from the same store, swapping the generator afterwards (simplified):
class RDoc::RubyGemsHook
def generate
@rdoc.store = RDoc::Store.new(parse_options)
@rdoc.parse_files parse_options.files
document 'ri', options, @ri_dir if @generate_ri and (@force or not File.exist? @ri_dir)
document 'aliki', options, @rdoc_dir if @generate_rdoc and (@force or not File.exist? @rdoc_dir)
end
endparse_options takes its generator from the gem's spec.rdoc_options. So if a gem specifies --format=ri there, parsing skips method source tokens, and the subsequent aliki generation raises:
method source for Foo#bar was not stored; set store_method_source? to trueReproduction: https://gist.github.com/tompng/5fe25eaeb6cb53164a3c49374d330914
Since the generator can be swapped after parsing like this, deriving the decision from options.generator at parse time is not reliable. I think store_method_source should be an explicit option that RubyGemsHook sets before parsing, e.g.:
parse_options.store_method_source = Generator::Aliki.store_method_source? || Generator::RI.store_method_source?
@rdoc.parse_files parse_options.files(strictly, only the generators that will actually run need to be OR-ed)
def store_method_source?
# return true or false if it's explicitly set
return @store_method_source unless store_method_source.nil?
# fallback path (@coveragage_report, @generator.store_method_source?)`
endThere was a problem hiding this comment.
Good catch!
I didn't know this was even possible. I'll take a closer look
Each documented method currently retains an array of
RDoc::Parser::RubyColorizer::ColoredTokenobjects and their fragmented source strings.This data is unnecessary for:
Large generated SDKs can contain millions of tokens. Avoiding those allocations substantially reduces parsing time and memory without changing generated output.
Implementation
Generators can declare whether they require method source:
The Ruby parser checks that capability before syntax highlighting a method body.
Generators without the capability default to true, preserving compatibility with existing third-party generators.
This also allows third-party generators (like rdoc-markdown) to skip this costly process if they don't have need for
method.token_streamormethod.markup_code.If
store_method_source?was disabled, buttoken_streamwas still accessed, an error will be raised.Benchmark
Here is a code that was used for benchmark this fix:
https://github.com/skatkov/rdoc-store-method-source-benchmark
RDoc 8.0.0 and
google-api-client0.53.0 on Ruby 4.0.6: