Share method entries between shapes via RBS Method identity - #2248
Open
soutaro wants to merge 1 commit into
Open
Share method entries between shapes via RBS Method identity#2248soutaro wants to merge 1 commit into
soutaro wants to merge 1 commit into
Conversation
RBS::DefinitionBuilder returns the identical Definition::Method object for methods inherited without any change (no overriding, no type application). Steep was rebuilding Shape::Entry/MethodOverload wrappers for every one of them per type: across the 765 classes of Steep's own environment, the definitions contain 118,475 (type, method) entries of which only 21,399 (18%) are distinct Method objects -- 82% of shape construction work was duplicated. Interface::Builder#method_entry now caches entries in an identity-keyed Hash, so a method entry is built once and shared by the shapes of every type that inherits it. The only type-dependent entry is the (Kernel) 'class' method, whose return type is replaced with the singleton type of the owner, so it bypasses the cache. Checking Steep's app target (single process, interleaved A/B): wall 32.9s -> 30.1s, GC 6.9s -> 5.6s, allocations 30.4M -> 27.0M, peak RSS 408MB -> 348MB. Building the shapes of all 1,646 classes in the environment eagerly retains 161MB instead of 283MB, which also helps a future warmup-then-fork worker design. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011WWMEZWwe2481yi4BZNKLo
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
2 participants
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.
Stacked on #2247. (Retarget to
masterafter #2247 merges.)Problem
RBS::DefinitionBuilderreturns the identicalDefinition::Methodobject for methods inherited without any change (no overriding, no type application) —Object#tapinbuild_instance(::String)is the same object as inbuild_instance(::Object). Steep, however, rebuiltShape::Entry/MethodOverloadwrappers for every method of every type.Measured on Steep's own environment (765 classes): the definitions contain 118,475 (type, method) entries, of which only 21,399 (18%) are distinct
Methodobjects — 82% of shape construction work was duplicated, mostly for methods inherited fromObject/Kernel/Enumerable.Change
Interface::Builder#method_entrycaches entries in an identity-keyed Hash (compare_by_identity), so an entry is built once and shared by the shapes of every type that inherits the sameMethodobject.The only type-dependent entry is the
classmethod (defined inKernel), whose return type is replaced with the singleton type of the owner (replace_kernel_class), so it bypasses the cache. The other rewrites (method_name_for,replace_primitive_method,add_implicitly_returns_nil) depend only on the type definition itself and are safe to share.Results
Checking Steep's app target (single process, interleaved A/B on the same machine):
Eagerly building the shapes of all 1,646 classes in the environment retains 161MB instead of 283MB (−43%), which also helps a future warmup-then-fork worker design.
Verification
test/type_check_test.rb(133 runs),test/subtyping_test.rb(50 runs),test/interface_test.rball passsteep checkself-type-check reports the same 23 problems as the base branch (unchanged)test/interface_builder_test.rbhas 1 pre-existing failure on the base branch as well (rbs master changedHash#[]to take_Key), unrelated to this change🤖 Generated with Claude Code
https://claude.ai/code/session_011WWMEZWwe2481yi4BZNKLo
Generated by Claude Code