Skip to content

Commit f7cd0e9

Browse files
skatkovCopilot
andauthored
Lazy code colorization (#1792)
Please see the prior PR #1788 In that PR it was correctly established that eager generation of `RDoc::Parser::RubyColorizer::ColoredToken` objects creates a heavy performance/system burden for all generators, but not all generators need these objects. My suggested approach in that PR was to establish a generator property that would stop this eager code colorization process. This approach is not functional, because rdoc itself can switch generators, but use the same @store object. In this PR I want to suggest an alternative solution: - Instead of doing this eagerly, do code colorization lazily (only if stream_token was actually called). - This would also ensure, that no new properties or methods will be introduced. And this will help us keep backward compatibility, while improving performance. ## Solution This change stores a copied method-source slice in a deferred token stream and materializes the existing mutable token array only when token_stream is accessed. It also switches the initial file scan from Prism.parse_lex to Prism.parse, preserves indentation and heredoc boundaries, and keeps the existing public token-stream behavior for HTML and third-party generators. ## Benchmark I have done a benchmark on google-client-api gem (one of the biggest and popular gems in the ecosystem). This could be found here: https://gist.github.com/skatkov/7e509742651586c1b993a8b15b10a8da ### RI #### After Parse | Variant | RSS | Peak RSS | Colored | Deferred | Raw strings | Raw source | Prism nodes | Prism tokens | Heap slots | | --- | ---: | ---: | ---: | ---: | ---: | ---: | ---: | ---: | ---: | | released | 958.8 MiB | 965.6 MiB | 3900930 | 0 | 0 | 0.0 MiB | 0 | 0 | 9998568 | | current | 478.2 MiB | 478.2 MiB | 0 | 46571 | 617 | 57.0 MiB | 0 | 0 | 2433708 | #### After Generate | Variant | RSS | Peak RSS | Colored | Deferred | Raw strings | Raw source | Prism nodes | Prism tokens | Heap slots | | --- | ---: | ---: | ---: | ---: | ---: | ---: | ---: | ---: | ---: | | released | 1356.7 MiB | 1391.4 MiB | 3900930 | 0 | 0 | 0.0 MiB | 0 | 0 | 11901244 | | current | 719.7 MiB | 755.3 MiB | 0 | 46571 | 617 | 57.0 MiB | 0 | 0 | 4336429 | | Variant | Parse | Generate | Total | GNU peak RSS | | --- | ---: | ---: | ---: | ---: | | released | 24.31s | 30.40s | 58.18s | 1391.4 MiB | | current | 14.62s | 28.59s | 45.43s | 755.3 MiB | Parse RSS change: -50.1% Generated RSS change: -47.0% Peak RSS change: -45.7% Parse time change: -39.8% Generate time change: -5.9% Total time change: -21.9% Outputs identical: yes ### Aliki, 100 files ```sh ./benchmark.rb --format aliki --limit 100 ``` #### After Parse | Variant | RSS | Peak RSS | Colored | Deferred | Raw strings | Raw source | Prism nodes | Prism tokens | Heap slots | | --- | ---: | ---: | ---: | ---: | ---: | ---: | ---: | ---: | ---: | | released | 96.3 MiB | 96.9 MiB | 253392 | 0 | 0 | 0.0 MiB | 0 | 0 | 708268 | | current | 65.4 MiB | 65.4 MiB | 0 | 2701 | 2701 | 0.9 MiB | 0 | 0 | 206955 | #### After Generate | Variant | RSS | Peak RSS | Colored | Deferred | Raw strings | Raw source | Prism nodes | Prism tokens | Heap slots | | --- | ---: | ---: | ---: | ---: | ---: | ---: | ---: | ---: | ---: | | released | 219.1 MiB | 220.4 MiB | 253392 | 0 | 0 | 0.0 MiB | 0 | 0 | 1037598 | | current | 210.3 MiB | 210.3 MiB | 253392 | 0 | 0 | 0.0 MiB | 0 | 0 | 1038957 | | Variant | Parse | Generate | Total | GNU peak RSS | | --- | ---: | ---: | ---: | ---: | | released | 1.40s | 44.09s | 46.10s | 220.4 MiB | | current | 0.90s | 44.56s | 46.04s | 209.5 MiB | Parse RSS changed by -32.1%, generated RSS by -4.0%, peak RSS by -4.9%, parse time by -35.7%, generation time by +1.1%, and total time by -0.1%. Outputs were identical. ## Results According to the benchmark, we can see that the lazy approach brings significant memory improvements even for generators that actually output highlighted code. In cases of 'RI', generator that has no use for highlighted code, we can see significant memory improvements (~50%), but also ~20% speed improvements. --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
1 parent d33c5a7 commit f7cd0e9

6 files changed

Lines changed: 194 additions & 38 deletions

File tree

‎lib/rdoc/generator/markup.rb‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -138,9 +138,9 @@ def add_location_comment(src)
138138
# Prepends line numbers if +options.line_numbers+ is true.
139139

140140
def markup_code
141-
return '' if !@token_stream
141+
return '' unless token_stream
142142

143-
src = RDoc::TokenStream.to_html @token_stream
143+
src = RDoc::TokenStream.to_html token_stream
144144

145145
# dedent the source
146146
common_indent = src.length

‎lib/rdoc/parser/ruby.rb‎

Lines changed: 13 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,7 @@ def initialize(top_level, content, options, stats)
143143
@token_listeners = nil
144144
content = RDoc::Encoding.remove_magic_comment content
145145
@content = content
146+
@colorizer_context = RDoc::Parser::RubyColorizer::DeferredContext.new(content)
146147
@markup = @options.markup
147148
@track_visibility = :nodoc != @options.visibility
148149
@encoding = @options.encoding
@@ -251,11 +252,8 @@ def record_location(container) # :nodoc:
251252

252253
def scan
253254
@lines = @content.lines
254-
result = Prism.parse_lex(@content)
255-
@program_node, unordered_tokens = result.value
256-
# Heredoc tokens are not in start_offset order.
257-
# Need to sort them to use bsearch for finding tokens from location.
258-
@prism_tokens = unordered_tokens.map(&:first).sort_by { |t| t.location.start_offset }
255+
result = Prism.parse(@content)
256+
@program_node = result.value
259257
@line_nodes = {}
260258
prepare_line_nodes(@program_node)
261259
prepare_comments(result.comments)
@@ -367,10 +365,9 @@ def parse_comment_tomdoc(container, comment, line_no, start_line)
367365
meth.call_seq = signature
368366
return unless meth.name
369367

370-
meth.start_collecting_tokens(:ruby)
371368
node = @line_nodes[line_no]
372-
tokens = node ? syntax_highlighted_tokens(node) : []
373-
tokens.each { |token| meth.token_stream << token }
369+
token_stream_loader = @colorizer_context.token_stream_loader(node.node_id) if node
370+
meth.start_collecting_tokens(:ruby, loader: token_stream_loader)
374371

375372
container.add_method meth
376373
meth.comment = comment
@@ -440,12 +437,7 @@ def handle_meta_method_comment(comment, directives, node)
440437
end
441438
elsif line_no || node
442439
method_name ||= call_node_name_arguments(node).first if is_call_node
443-
if node
444-
tokens = syntax_highlighted_tokens(node)
445-
line_no = node.location.start_line
446-
else
447-
tokens = []
448-
end
440+
line_no = node.location.start_line if node
449441
internal_add_method(
450442
method_name,
451443
@container,
@@ -457,7 +449,7 @@ def handle_meta_method_comment(comment, directives, node)
457449
params: nil,
458450
calls_super: false,
459451
block_params: nil,
460-
tokens: tokens,
452+
node_id: node&.node_id,
461453
)
462454
end
463455
end
@@ -552,12 +544,6 @@ def extract_section_comment(comment_text, prefix_line_count) # :nodoc:
552544
comment_text
553545
end
554546

555-
# Returns syntax highlighted tokens of the given node
556-
557-
def syntax_highlighted_tokens(node)
558-
RDoc::Parser::RubyColorizer.partial_colorize(@content, node, @prism_tokens)
559-
end
560-
561547
# Handles `public :foo, :bar` `private :foo, :bar` and `protected :foo, :bar`
562548

563549
def change_method_visibility(names, visibility, singleton: @singleton)
@@ -696,7 +682,7 @@ def add_extends(names, line_no) # :nodoc:
696682

697683
# Adds a method defined by `def` syntax
698684

699-
def add_method(method_name, receiver_name:, receiver_fallback_type:, visibility:, singleton:, params:, calls_super:, block_params:, tokens:, start_line:, args_end_line:, end_line:)
685+
def add_method(method_name, receiver_name:, receiver_fallback_type:, visibility:, singleton:, params:, calls_super:, block_params:, node_id:, start_line:, args_end_line:, end_line:)
700686
comment, directives, type_signature_lines = consecutive_comment(start_line)
701687
apply_document_control_directive(directives) if directives
702688
handle_code_object_directives(@container, directives) if directives
@@ -716,12 +702,12 @@ def add_method(method_name, receiver_name:, receiver_fallback_type:, visibility:
716702
params: params,
717703
calls_super: calls_super,
718704
block_params: block_params,
719-
tokens: tokens,
705+
node_id: node_id,
720706
type_signature_lines: type_signature_lines
721707
)
722708
end
723709

724-
private def internal_add_method(method_name, container, comment:, directives:, modifier_comment_lines: nil, line_no:, visibility:, singleton:, params:, calls_super:, block_params:, tokens:, type_signature_lines: nil) # :nodoc:
710+
private def internal_add_method(method_name, container, comment:, directives:, modifier_comment_lines: nil, line_no:, visibility:, singleton:, params:, calls_super:, block_params:, node_id:, type_signature_lines: nil) # :nodoc:
725711
meth = RDoc::AnyMethod.new(method_name, singleton: singleton)
726712
meth.comment = comment
727713
handle_code_object_directives(meth, directives) if directives
@@ -745,7 +731,6 @@ def add_method(method_name, receiver_name:, receiver_fallback_type:, visibility:
745731
meth.calls_super = calls_super
746732
meth.block_params ||= block_params if block_params
747733
meth.type_signature_lines = type_signature_lines
748-
749734
# An instance method `initialize` is documented as `::new` unless the
750735
# :notnew: directive is given
751736
if method_name == 'initialize' && !singleton
@@ -760,10 +745,8 @@ def add_method(method_name, receiver_name:, receiver_fallback_type:, visibility:
760745

761746
record_location(meth)
762747
container.add_method(meth)
763-
meth.start_collecting_tokens(:ruby)
764-
tokens.each do |token|
765-
meth.token_stream << token
766-
end
748+
token_stream_loader = @colorizer_context.token_stream_loader(node_id) if node_id
749+
meth.start_collecting_tokens(:ruby, loader: token_stream_loader)
767750
end
768751

769752
# Find or create module or class from a given module name using Ruby lexical
@@ -1173,8 +1156,6 @@ def visit_def_node(node)
11731156
end
11741157
name = node.name.to_s
11751158
params, block_params, calls_super = MethodSignatureVisitor.scan_signature(node)
1176-
tokens = @scanner.syntax_highlighted_tokens(node)
1177-
11781159
@scanner.add_method(
11791160
name,
11801161
receiver_name: receiver_name,
@@ -1184,7 +1165,7 @@ def visit_def_node(node)
11841165
params: params,
11851166
block_params: block_params,
11861167
calls_super: calls_super,
1187-
tokens: tokens,
1168+
node_id: node.node_id,
11881169
start_line: start_line,
11891170
args_end_line: args_end_line,
11901171
end_line: end_line

‎lib/rdoc/parser/ruby_colorizer.rb‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,52 @@ module RDoc::Parser::RubyColorizer
1010

1111
ColoredToken = Struct.new(:kind, :text)
1212

13+
# Defers colorization for all nodes in one source file until first access.
14+
class DeferredContext # :nodoc:
15+
#: (String) -> void
16+
def initialize(source)
17+
@source = source
18+
@token_streams = {}
19+
end
20+
21+
#: (Integer) -> ^() -> Array[ColoredToken]
22+
def token_stream_loader(node_id)
23+
@token_streams[node_id] = nil
24+
-> { token_stream_for(node_id) }
25+
end
26+
27+
#: (Integer) -> Array[ColoredToken]
28+
def token_stream_for(node_id)
29+
materialize unless materialized?
30+
@token_streams.fetch(node_id)
31+
end
32+
33+
private
34+
35+
#: () -> Boolean
36+
def materialized?
37+
!@source
38+
end
39+
40+
#: () -> void
41+
def materialize
42+
program_node, unordered_tokens = Prism.parse_lex(@source).value
43+
prism_tokens = unordered_tokens.map(&:first).sort_by! { |token| token.location.start_offset }
44+
staged_tokens = {}
45+
nodes = [program_node]
46+
until nodes.empty?
47+
node = nodes.pop
48+
if @token_streams.key?(node.node_id)
49+
staged_tokens[node.node_id] = RDoc::Parser::RubyColorizer.partial_colorize(@source, node, prism_tokens)
50+
end
51+
nodes.concat(node.compact_child_nodes)
52+
end
53+
54+
@token_streams = staged_tokens
55+
@source = nil
56+
end
57+
end
58+
1359
# Prism operator token types except assignment '='
1460
OP_TOKENS = %i[
1561
AMPERSAND AMPERSAND_AMPERSAND

‎lib/rdoc/token_stream.rb‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,22 +50,24 @@ def self.to_html(token_stream)
5050
# Adds +tokens+ to the collected tokens
5151

5252
def add_tokens(tokens)
53-
@token_stream.concat(tokens)
53+
token_stream.concat(tokens)
5454
end
5555

5656
##
5757
# Adds one +token+ to the collected tokens
5858

5959
def add_token(token)
60-
@token_stream.push(token)
60+
token_stream.push(token)
6161
end
6262

6363
##
6464
# Starts collecting tokens
6565
#
66+
# The optional +loader+ is called once on first access and its result is reused.
6667

67-
def collect_tokens(language)
68+
def collect_tokens(language, loader: nil)
6869
@token_stream = []
70+
@token_stream_loader = loader
6971
@token_stream_language = language
7072
end
7173

@@ -75,13 +77,17 @@ def collect_tokens(language)
7577
# Remove the last token from the collected tokens
7678

7779
def pop_token
78-
@token_stream.pop
80+
token_stream.pop
7981
end
8082

8183
##
8284
# Current token stream
8385

8486
def token_stream
87+
if @token_stream_loader
88+
@token_stream = @token_stream_loader.call
89+
@token_stream_loader = nil
90+
end
8591
@token_stream
8692
end
8793

‎test/rdoc/parser/ruby_colorizer_test.rb‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,102 @@ def token(kind, text)
77
RDoc::Parser::RubyColorizer::ColoredToken.new(kind, text)
88
end
99

10+
def test_deferred_token_stream
11+
code = <<~'RUBY'
12+
first(<<~ONE); second(<<~TWO) && sibling
13+
one
14+
ONE
15+
two
16+
TWO
17+
RUBY
18+
program_node, unordered_tokens = Prism.parse_lex(code).value
19+
node = program_node.statements.body.last.left
20+
prism_tokens = unordered_tokens.map(&:first).sort_by! { |token| token.location.start_offset }
21+
expected = RDoc::Parser::RubyColorizer.partial_colorize(code, node, prism_tokens)
22+
context = RDoc::Parser::RubyColorizer::DeferredContext.new(code)
23+
loader = context.token_stream_loader(node.node_id)
24+
25+
assert_equal expected, loader.call
26+
assert_same loader.call, loader.call
27+
end
28+
29+
def test_deferred_token_stream_materializes_registered_nodes_once
30+
code = "hidden\nfirst\nsecond\n"
31+
nodes = Prism.parse(code).value.statements.body
32+
context = RDoc::Parser::RubyColorizer::DeferredContext.new(code)
33+
loaders = nodes.drop(1).map { |node| context.token_stream_loader(node.node_id) }
34+
parse_lex_calls = partial_colorize_calls = 0
35+
parse_lex = Prism.method(:parse_lex)
36+
colorizer = RDoc::Parser::RubyColorizer
37+
partial_colorize = colorizer.method(:partial_colorize)
38+
39+
Prism.define_singleton_method(:parse_lex) do |*arguments|
40+
parse_lex_calls += 1
41+
parse_lex.call(*arguments)
42+
end
43+
colorizer.define_singleton_method(:partial_colorize) do |*arguments|
44+
partial_colorize_calls += 1
45+
partial_colorize.call(*arguments)
46+
end
47+
48+
begin
49+
assert_equal %w[first second], loaders.map { |loader| loader.call.map(&:text).join }
50+
ensure
51+
Prism.define_singleton_method(:parse_lex, parse_lex)
52+
colorizer.define_singleton_method(:partial_colorize, partial_colorize)
53+
end
54+
assert_equal 1, parse_lex_calls
55+
assert_equal 2, partial_colorize_calls
56+
end
57+
58+
def test_deferred_token_stream_preserves_lexical_scope
59+
code = "x = 1\ny = 2\ni = 3\nx /y/i\n"
60+
program_node, unordered_tokens = Prism.parse_lex(code).value
61+
node = program_node.statements.body.last
62+
prism_tokens = unordered_tokens.map(&:first).sort_by! { |token| token.location.start_offset }
63+
expected = RDoc::Parser::RubyColorizer.partial_colorize(code, node, prism_tokens)
64+
65+
context = RDoc::Parser::RubyColorizer::DeferredContext.new(code)
66+
loader = context.token_stream_loader(node.node_id)
67+
68+
assert_equal expected, loader.call
69+
end
70+
71+
def test_deferred_token_stream_retries_atomically
72+
code = "first\nsecond\n"
73+
nodes = Prism.parse(code).value.statements.body
74+
context = RDoc::Parser::RubyColorizer::DeferredContext.new(code)
75+
loaders = nodes.map { |node| context.token_stream_loader(node.node_id) }
76+
colorizer = RDoc::Parser::RubyColorizer
77+
partial_colorize = colorizer.method(:partial_colorize)
78+
calls = 0
79+
80+
colorizer.define_singleton_method(:partial_colorize) do |*arguments|
81+
calls += 1
82+
raise 'colorization failed' if calls == 2
83+
84+
partial_colorize.call(*arguments)
85+
end
86+
begin
87+
assert_raise(RuntimeError) { loaders.first.call }
88+
ensure
89+
colorizer.define_singleton_method(:partial_colorize, partial_colorize)
90+
end
91+
92+
assert_equal %w[first second], loaders.map { |loader| loader.call.map(&:text).join }
93+
end
94+
95+
def test_deferred_token_stream_rejects_unmatched_node_id
96+
code = "first\n"
97+
node = Prism.parse(code).value.statements.body.first
98+
context = RDoc::Parser::RubyColorizer::DeferredContext.new(code)
99+
loader = context.token_stream_loader(node.node_id)
100+
missing_loader = context.token_stream_loader(node.node_id + 1_000_000)
101+
102+
assert_equal "first", loader.call.map(&:text).join
103+
2.times { assert_raise(KeyError) { missing_loader.call } }
104+
end
105+
10106
def test_partial_colorize
11107
code = <<~RUBY
12108
class A

‎test/rdoc/rdoc_token_stream_test.rb‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,19 @@ def test_collect_tokens
8585
assert_equal [], foo.token_stream
8686
end
8787

88+
def test_collect_tokens_with_loader
89+
foo = Class.new do
90+
include RDoc::TokenStream
91+
end.new
92+
loads = 0
93+
foo.collect_tokens(:ruby, loader: -> { loads += 1; [:token] })
94+
95+
tokens = foo.token_stream
96+
assert_equal [:token], tokens
97+
assert_same tokens, foo.token_stream
98+
assert_equal 1, loads
99+
end
100+
88101
def test_pop_token
89102
foo = Class.new do
90103
include RDoc::TokenStream
@@ -95,6 +108,20 @@ def test_pop_token
95108
assert_equal [], foo.token_stream
96109
end
97110

111+
def test_mutating_deferred_tokens
112+
foo = Class.new do
113+
include RDoc::TokenStream
114+
end.new
115+
tokens = [:first]
116+
foo.collect_tokens(:ruby, loader: -> { tokens })
117+
118+
foo.add_token(:second)
119+
foo.add_tokens([:third])
120+
121+
assert_equal :third, foo.pop_token
122+
assert_equal [:first, :second], foo.token_stream
123+
end
124+
98125
def test_token_stream
99126
foo = Class.new do
100127
include RDoc::TokenStream

0 commit comments

Comments
 (0)