Skip to content

Reuse decls in resolve_type_names when no type names change#2977

Closed
tk0miya wants to merge 2 commits into
ruby:masterfrom
tk0miya:claude/immutable-resolve-type-names
Closed

Reuse decls in resolve_type_names when no type names change#2977
tk0miya wants to merge 2 commits into
ruby:masterfrom
tk0miya:claude/immutable-resolve-type-names

Conversation

@tk0miya
Copy link
Copy Markdown
Contributor

@tk0miya tk0miya commented May 26, 2026

resolve_type_names previously rebuilt every declaration, member, and type even when type name resolution did not change anything. The first resolve must produce absolutized type names, so its cost is unavoidable; but the second and later resolves were re-allocating identical structures for no benefit, pressuring GC heavily.

This change makes every map_type_name / map_type / resolve_* helper return its receiver when each child maps back to a value equal? to the original. Combined with the existing flyweight behavior of TypeName and Namespace, declarations whose type names were already absolute are now reused verbatim across resolves.

The first resolve is therefore unchanged in both wall time and allocations. The numbers below compare the second-and-later resolves only, measured on conference-app (kaigionrails/conference-app):

  • allocated per resolve: 20.60 MB / 387,664 objects
    -> 3.52 MB / 64,293 objects (-83%)
  • retained over 10 resolves: 18.36 MB / 346,343 objects
    -> 1.25 MB / 22,973 objects (-93%)
  • resolve wall time, p99: 112.7 ms -> 98.3 ms (-12.8%)
  • GC major / 50 resolves: 4 -> 1 (-75%)

Single-shot CLI usage (rbs list etc.) calls resolve_type_names only once, so it sees no change. Long-running clients such as Steep that re-resolve repeatedly are the primary beneficiaries.

Comment thread lib/rbs.rb
# changed elements substituted in. Callers detect a no-op by comparing
# the return value with the input via `equal?`, which avoids
# allocating a `[mapped, changed]` tuple on every invocation.
def map_if_changed(array, &)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure where the best location for these utility methods is.

@tk0miya
Copy link
Copy Markdown
Contributor Author

tk0miya commented May 26, 2026

This is a benchmark result for this change. Please let me know if you need the benchmark scripts. I'll commit them too.

master:

tkomiya@altair> time bundle exec rbs -Isig list > /dev/null
bundle exec rbs -Isig list > /dev/null  0.54s user 0.17s system 63% cpu 1.126 total
tkomiya@altair> time bundle exec rbs -Isig list > /dev/null
bundle exec rbs -Isig list > /dev/null  0.56s user 0.18s system 64% cpu 1.153 total
tkomiya@altair> time bundle exec rbs -Isig list > /dev/null
bundle exec rbs -Isig list > /dev/null  0.55s user 0.17s system 64% cpu 1.125 total
tkomiya@altair> time bundle exec rbs -Isig list > /dev/null
bundle exec rbs -Isig list > /dev/null  0.55s user 0.18s system 61% cpu 1.191 total
tkomiya@altair> time bundle exec rbs -Isig list > /dev/null
bundle exec rbs -Isig list > /dev/null  0.57s user 0.18s system 63% cpu 1.170 total
tkomiya@altair> ruby -Ilib benchmark/memory_resolve_type_names.rb

[resolve_type_names after unload+add_source (1×)]
  allocated:  20.60 MB / 387664 objects
  retained:   0 B / 0 objects

[resolve_type_names after unload+add_source (10×)]
  allocated:  274.42 MB / 3876640 objects
  per iter:   27.44 MB / 387664 objects (n=10)
tkomiya@altair> ruby -Ilib benchmark/benchmark_resolve_type_names.rb
resolve_type_names after unload+add_source (iterations=50)
  mean      90.277 ms
  median    86.063 ms
  min       79.369 ms
  max       112.695 ms
  stdev     9.978 ms (±11.1%)
  i/s       11.077
  GC minor  13 (0.26/iter)
  GC major  4 (0.08/iter)
  alloc obj 21335967 (426719/iter, incl. unload+add)

proposal:

tkomiya@altair> time bundle exec rbs -Isig list > /dev/null
bundle exec rbs -Isig list > /dev/null  0.57s user 0.18s system 64% cpu 1.161 total
tkomiya@altair> time bundle exec rbs -Isig list > /dev/null
bundle exec rbs -Isig list > /dev/null  0.56s user 0.16s system 65% cpu 1.108 total
tkomiya@altair> time bundle exec rbs -Isig list > /dev/null
bundle exec rbs -Isig list > /dev/null  0.57s user 0.18s system 65% cpu 1.145 total
tkomiya@altair> time bundle exec rbs -Isig list > /dev/null
bundle exec rbs -Isig list > /dev/null  0.56s user 0.17s system 62% cpu 1.162 total
tkomiya@altair> time bundle exec rbs -Isig list > /dev/null
bundle exec rbs -Isig list > /dev/null  0.57s user 0.18s system 62% cpu 1.188 total
tkomiya@altair> ruby -Ilib benchmark/memory_resolve_type_names.rb

[resolve_type_names after unload+add_source (1×)]
  allocated:  3.52 MB / 64293 objects
  retained:   0 B / 0 objects

[resolve_type_names after unload+add_source (10×)]
  allocated:  37.85 MB / 642930 objects
  per iter:   3.78 MB / 64293 objects (n=10)
tkomiya@altair> ruby -Ilib benchmark/benchmark_resolve_type_names.rb
resolve_type_names after unload+add_source (iterations=50)
  mean      85.126 ms
  median    83.005 ms
  min       81.371 ms
  max       98.323 ms
  stdev     4.865 ms (±5.7%)
  i/s       11.747
  GC minor  11 (0.22/iter)
  GC major  1 (0.02/iter)
  alloc obj 8143518 (162870/iter, incl. unload+add)

`resolve_type_names` previously rebuilt every declaration, member, and
type even when type name resolution did not change anything. The first
resolve must produce absolutized type names, so its cost is
unavoidable; but the second and later resolves were re-allocating
identical structures for no benefit, pressuring GC heavily.

This change makes every `map_type_name` / `map_type` / `resolve_*`
helper return its receiver when each child maps back to a value
`equal?` to the original. Combined with the existing flyweight
behavior of `TypeName` and `Namespace`, declarations whose type names
were already absolute are now reused verbatim across resolves.

The first resolve is therefore unchanged in both wall time and
allocations. The numbers below compare the second-and-later resolves
only, measured on conference-app (kaigionrails/conference-app):

  - allocated per resolve:     20.60 MB / 387,664 objects
                            ->  3.52 MB /  64,293 objects   (-83%)
  - retained over 10 resolves: 18.36 MB / 346,343 objects
                            ->  1.25 MB /  22,973 objects   (-93%)
  - resolve wall time, p99:   112.7 ms  ->  98.3 ms         (-12.8%)
  - GC major / 50 resolves:    4        ->  1               (-75%)

Single-shot CLI usage (`rbs list` etc.) calls resolve_type_names only
once, so it sees no change. Long-running clients such as Steep that
re-resolve repeatedly are the primary beneficiaries.
@tk0miya tk0miya force-pushed the claude/immutable-resolve-type-names branch from 56c6d46 to 79ac920 Compare May 26, 2026 18:17
Add benchmark_resolve_rails_env.rb (benchmark/ips) and
memory_resolve_rails_env.rb (memory_profiler) measuring the edit-cycle
re-resolve that long-running clients such as Steep hit: unload one
source, add it back, then resolve_type_names. This is the path that
benefits from reusing declarations whose type names did not change.

The naming follows the existing benchmarks: where *_new_rails_env.rb
measures building a Rails environment (Environment.new + first resolve),
these measure re-resolving an already-built Rails environment. Both use
the existing prepare_collection! / new_rails_env helpers, so they need
no fixture beyond `gem 'rails'`.

Also require 'pathname' in benchmark/utils.rb: requiring "rbs" only loads
the builtin pathname, which lacks Pathname#rmtree on Ruby 4.0, so
prepare_collection!'s at_exit cleanup raised NoMethodError. Requiring
pathname explicitly loads the full implementation.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@tk0miya
Copy link
Copy Markdown
Contributor Author

tk0miya commented May 30, 2026

Now I added benchmark scripts to check the 2nd resolve_type_names to measure time and memory of the RBS.

Additionally, I added a benchmark for this optimization: benchmark/benchmark_resolve_rails_env.rb
(time, via benchmark-ips) and benchmark/memory_resolve_rails_env.rb (allocations, via memory_profiler).

Both build a Rails environment with the existing prepare_collection! /
new_rails_env helpers, then measure the re-resolve path that long-running
clients such as Steep hit on every edit: unload one source, add it back, and call
resolve_type_names again. This is exactly the path this PR optimizes (reusing
declarations whose type names did not change).

Results on conference-app-sized Rails deps (Ruby 4.0.4):

master this PR
resolve_type_names 11.86 i/s 12.57 i/s (+6%)
allocated / resolve 15.86 MB (287k objs) 2.76 MB (50.7k objs) — -83%
retained 14.11 MB (254k objs) 1.00 MB (18.3k objs) — -93%

Single-shot build (new_rails_env) is unaffected: allocations actually drop
slightly (84.1 MB → 76.2 MB) and wall time is within noise, so rbs list and
other one-shot users see no regression.

@soutaro soutaro added this to the RBS 4.1 milestone May 30, 2026
@soutaro
Copy link
Copy Markdown
Member

soutaro commented May 30, 2026

@tk0miya Did you see any actual improvement with Steep? (or some user facing program.)

@tk0miya
Copy link
Copy Markdown
Contributor Author

tk0miya commented May 31, 2026

I have to apologize because the effect of this change is very limited. Sorry for the noise.

This optimization was ported from a part of the Rust resolver (my private project). So it doesn't address a real problem here.
After your comment, I read the Steep code and found that it resolves only modified sources, not the whole project.

So this only matters if the user updates many RBS files at the same time, which would happen only if some generators (ex. rbs_rails, rbs-inline) regenerate the whole project. Even then, the cost of re-resolving is not that high. I have to admit this optimization does not solve anything.

So I'll withdraw this PR. Thank you for reviewing.

@tk0miya tk0miya closed this May 31, 2026
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