Optimise L33t matcher performance and improve test coverage#64
Merged
Conversation
- Add early bailout when no l33t characters are present in password - Replace string concatenation with Set-based dedup for better performance - Use hash comparison instead of string labels for duplicate detection - Add benchmark gem to development dependencies Performance improvement: 5.6% faster (0.143ms -> 0.135ms per password)
Replace split/map/join with each_char string building for better performance. This avoids creating intermediate array objects.
Add comprehensive tests covering: - Early bailout when no l33t characters present - Multiple l33t substitution types - Ambiguous l33t characters (e.g., '1' for 'i' or 'l') - Mixed case password handling - Edge cases (empty passwords, repeated characters) - Detailed translate method testing - l33t flag and sub_display field verification Test count increased from 9 to 27 examples.
Extract match processing logic into private process_match method to satisfy RuboCop Metrics/PerceivedComplexity requirement whilst maintaining readability.
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.
Context
The L33t matcher is a performance-critical component, accounting for approximately 57% of password matching time in benchmarks. The matcher identifies passwords using "l33t speak" substitutions (e.g.,
@fora,0foro) by testing various substitution combinations against dictionary matchers.Changes
Performance optimisations:
translatemethod to useeach_charwith direct string building instead ofsplit/map/join, eliminating intermediate array allocationsTest improvements:
translatemethod testingDependencies:
benchmarkgem to development dependencies for performance measurementPerformance
Benchmark results (1000 iterations across 10 diverse passwords):
All 262 tests pass, confirming correctness is maintained whilst improving performance.