Repository navigation
Conversation
Browser::Base#bot built a second Browser from the downcased user agent, which runs every browser matcher again only to hand an Unknown instance to the bot matchers. It now passes itself. validate_size also receives its limit from the caller instead of interpolating a method name. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.
Hi Nando! We call
Browser.newon every request of our shop to read the name, version and platform, and to skip bots, so I took a look at what it allocates.Most of it came from
Browser::Base#bot:Bot.new(ua)builds a secondBrowserfrom the downcased user agent, which instantiates up to 30 browser matchers again just to hand that instance to the bot matchers. None of the built-in matchers read it (they all take_browser), and since detection runs on a downcased string, custom matchers always received aBrowser::Unknown. This PR passes the browser that is already detected:Bot.new(ua, browser: self).Browser::Bot.new(ua)called on its own keeps working as before.The second change is smaller:
validate_sizebuilt a:"#{subject}_size_limit"symbol on every call, which allocates a string per matcher tried. The callers now pass the limit directly.Allocated objects per call, Ruby 4.0.9 arm64:
Browser.newBrowser.new(ua).bot?name,version,platform.nameandbot?With benchmark-ips,
Browser.new(ua).bot?goes from 18.4k to 24.3k i/s (+32%).One behavior change, noted in the changelog: custom bot matchers now receive the real browser instead of an
Unknownone. That is what the README's->(ua, browser)signature suggests, and a new test covers it. RuboCop reports the same 6 offenses as onmain, none in the changed lines.By the way, the test suite no longer boots on
mainwith Rails 8.1 (delegate_missing_tois undefined). I ran it withRUBYOPT="-rlogger -ractive_support -ractive_support/core_ext/module/delegation", and I'm happy to send a separate fix if that helps.🤖 Generated with Claude Code, but prompted, read, and verified by @czj, a caring human :)