rmosolgo / rmosolgo/graphql-ruby
Optimize some predicate methods with `attr_reader`
Nobody has claimed this yet.
- Dominant language
- Ruby
- Stars
- 5.4k
- Forks
- 1.4k
- Avg merge
- 23h 19m
- Merged PRs (30d)
- 28
Description
When I was tinkering around and profiling GraphQL Ruby around a year ago, I found some small+easy perf wins from optimizing some commonly queried "predicate methods", by defining them via attr_reader. I've lost the benchmarks, and don't have time to recreate them, but I still figured it's worth sharing the idea here.
Unfortunately, Ruby makes these unnecessarily ugly/difficult in the case of boolean attributes, but still possible with this method swizzling dance:
attr_reader :foo # Define the fast method with a temporary name
alias_method :foo?, :foo # Give it a nice name
remove_method :foo # Delete the temporary name
The main cuplrits were in type_kind.rb. I've opened a PoC PR here: #5082
Candidates for this kind of replacement can be found with these 2 regexes:
def ([a-z0-9_]+)\??[;\n]\s*@\1[;\n]\s*end
def ([a-z0-9_]+)\??\s*=\s*@\1
(They can be merged into one, but I opted not to, just to keep them somewhat legible)
Would you be able to take a quick stab at benchmarking these? And in general, do you think those perf wins are worth this ugliness?
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by reviewing the predicate methods in type_kind.rb and use the two regexes from the issue to identify other candidates. Recreate benchmarks for the current methods and attr_reader-based alternatives, then document whether the performance gains justify the method-swizzling complexity.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- ruby
- Domain
- performance
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 30/100