Add Hash#values.compact vs select benchmark for non-nil values - #236
Open
etagwerker wants to merge 1 commit into
Open
Add Hash#values.compact vs select benchmark for non-nil values#236etagwerker wants to merge 1 commit into
etagwerker wants to merge 1 commit into
Conversation
Copy link
Copy Markdown
Member
Sorry, something went wrong.
Builds on the idea from #124 (sFrenkie), comparing three ways to collect the non-nil values of a hash: - Hash#select { |_k, v| v }.values - Hash#values.select { |v| v } - Hash#values.compact Uses data that actually contains nil values so all three return the same result, plus an equivalence guard, making it a fair comparison. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
There was a problem hiding this comment.
Pull request overview
This PR adds a new Hash performance benchmark to fairly compare three equivalent ways of collecting non-nil hash values, and documents the results in the README’s Hash section.
Changes:
- Added a new benchmark script comparing Hash#select.values, Hash#values.select, and Hash#values.compact for filtering nil values.
- Added a new README entry describing the tradeoffs and including sample benchmark output.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| README.md | Adds a Hash section entry documenting the benchmark and explaining why values.compact is fastest. |
| code/hash/select-values-vs-values-select-vs-values-compact.rb | Introduces the benchmark implementation plus an equivalence guard to ensure fair comparison. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Sorry, something went wrong.
Comment on lines
+8
to
+10
| def select_values | ||
| HASH.select { |_k, v| v }.values | ||
| end |
Comment on lines
+12
to
+14
| def values_select | ||
| HASH.values.select { |v| v } | ||
| end |
Comment on lines
+20
to
+22
| # Sanity check: all three must return the same values. | ||
| raise "not equivalent" unless select_values.sort == values_select.sort && | ||
| values_select.sort == values_compact.sort |
Comment thread
README.md
Comment on lines
+898
to
+899
| > To collect the non-nil values of a hash, `Hash#select { |_k, v| v }.values` allocates an intermediate hash before extracting its values; <br> | ||
| > `Hash#values.select { |v| v }` skips the intermediate hash but still runs a block per element; <br> |
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
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.
What
Adds a Hash benchmark comparing three ways to collect the non-nil values of a hash:
Hash#values.compact wins by a wide margin (~16x faster than values.select).
Why a new PR instead of #124
This builds on the idea from #124 by @sFrenkie, but that benchmark's three expressions aren't equivalent: its test data uses boolean values (v < 0.5 → true/false) and contains no nil values. So compact filtered nothing while the two select variants also dropped false. The "fastest" result came from compact doing less work, not from a fair comparison.
This version fixes that:
Output
README entry added in the Hash section.
🤖 Generated with Claude Code