Skip to content

Optimize Util.valid_variable_name? method#1782

Merged
prathmesh-stripe merged 1 commit intostripe:masterfrom
fatkodima:optimize-valid_variable_name
Feb 19, 2026
Merged

Optimize Util.valid_variable_name? method#1782
prathmesh-stripe merged 1 commit intostripe:masterfrom
fatkodima:optimize-valid_variable_name

Conversation

@fatkodima
Copy link
Contributor

Production profiling of our application revealed that a single Stripe::Util.valid_variable_name? method was using a few % of its runtime. This PR optimizes that method.

@fatkodima fatkodima requested a review from a team as a code owner February 10, 2026 00:15
@fatkodima fatkodima requested review from xavdid-stripe and removed request for a team February 10, 2026 00:15
@cla-assistant
Copy link

cla-assistant bot commented Feb 10, 2026

CLA assistant check
All committers have signed the CLA.

@prathmesh-stripe
Copy link
Contributor

fatkodima Please add more details to the PR. Some metrics, screenshots and a way to reproduce this would be helpful to evaluate this PR.

@fatkodima
Copy link
Contributor Author

fatkodima commented Feb 17, 2026

Production profiles:

CPU

       538   (1.7%)         538   (1.7%)     Regexp#match?
     26545  (83.0%)         419   (1.3%)     Array#each
       405   (1.3%)         405   (1.3%)     String#[]
       389   (1.2%)         389   (1.2%)     String#=~  <========== this line
       345   (1.1%)         345   (1.1%)     String#sub

Allocations

      3250   (3.6%)        3250   (3.6%)     String#split
      3199   (3.6%)        3199   (3.6%)     String#=~  <===============
      2315   (2.6%)        2269   (2.5%)     PG::Result#values
      7629   (8.5%)        1957   (2.2%)     Thread.each_caller_location
      1922   (2.2%)        1922   (2.2%)     Thread::Backtrace::Location#to_s
      1860   (2.1%)        1860   (2.1%)     PG::Result#fields
      1728   (1.9%)        1723   (1.9%)     ActiveRecord::ConnectionAdapters::PostgreSQLAdapter#get_oid_type
      4509   (5.1%)        1597   (1.8%)     Kernel#dup
      1400   (1.6%)        1400   (1.6%)     String#byteslice
      1096   (1.2%)        1072   (1.2%)     ActiveRecord::Associations#init_internals
       846   (0.9%)         846   (0.9%)     String#chars <===============
       838   (0.9%)         838   (0.9%)     Integer#to_s

I don't have a benchmark, but it is easy to see that it allocates a chars array (.chars.) each time and creates regexp captures (because of =~), even though these are not needed.

@fatkodima fatkodima force-pushed the optimize-valid_variable_name branch from 4aa4476 to 1e231f2 Compare February 17, 2026 17:28
Copy link
Contributor

@prathmesh-stripe prathmesh-stripe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for submitting this PR. Tested the new regex against:

Variable Valid/Invalid Comments
foo Starts with letter, contains letters
_private Starts with underscore
foo123 Letters + digits
a Single letter
123foo Starts with digit
my-foo Contains hyphen
my foo Contains space
foo! Contains special character
`` Empty string

@prathmesh-stripe prathmesh-stripe merged commit 707d703 into stripe:master Feb 19, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants