Fix memory leak in mock #68
Merged
Conversation
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
This avoids needing has_key? which iterates on hash keys This also clarifies code a lot, since replaced and hash_key? had a "sym" argument which wasn't a regular method name, unlike every other instance of "sym" in the file. That was confusing.
…ow expected a key This decouples the key, which must be unique, from the replacement method name, which we would prefer not to be unique (See ruby#67)
…method name This solves ruby#67
eregon approved these changes Feb 29, 2024
Member
eregon left a comment
eregon left a comment There was a problem hiding this comment.
Looks great, thank you.
The second commit description was quite to understand, basically it already talks about the 3rd commit's diff. Probably best to squash those two together.
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.
As discussed in #67.
The commits are incremental and coherent changes, so going commit by commit may be the easiest way to review.
rspecon this repo, and themspecon ruby-spec are both passing with these changes.To see the impact easily, edit
ruby-spec/spec/core/hash/compare_by_identity_spec.rbto only leave the specuses #equal? semantics, but doesn't actually call #equal? to determine identity. (line 50)Then run
../mspec/bin/mspec --repeat 1000000 core/hash/compare_by_identity_spec.rb.It should be easy to see the memory that just keeps going up quickly (before this change) using
topor similar monitoring tool.