Improve performance by caching regexes?

[asterite]

Hi @soveran! 😄

I noticed that to consume segments a regex is created each and every time the segment tries to be matched:

cuba/lib/cuba.rb

Line 214 in f661096

matchdata = env[Rack::PATH_INFO].match(/\A\/(#{pattern})(\/|\z)/)

So I made this benchmark:

require_relative "./lib/cuba"

Cuba.define do
  on "users" do
    on ":id" do |id|
      on root do
        res.write "User #{id}"
      end

      on "projects" do
        on ":project_id" do |project_id|
          res.write "User #{id}, Project #{project_id}"
        end
      end
    end
  end
end

env1 = { "PATH_INFO" => "/users/1", "SCRIPT_NAME" => "/" }
env2 = { "PATH_INFO" => "/users/1/projects/2", "SCRIPT_NAME" => "/" }

time = Time.now
30_000.times do
  Cuba.call(env1)
  Cuba.call(env2)
end
puts Time.now - time

On my machine it takes about 4 seconds to complete.

Now I cache the regexes with this diff:

diff --git a/lib/cuba.rb b/lib/cuba.rb
index b12e9f3..b4090fe 100644
--- a/lib/cuba.rb
+++ b/lib/cuba.rb
@@ -5,6 +5,7 @@ class Cuba
   EMPTY   = "".freeze
   SEGMENT = "([^\\/]+)".freeze
   DEFAULT = "text/html; charset=utf-8".freeze
+  REGEXES = Hash.new { |h, pattern| h[pattern] = /\A\/(#{pattern})(\/|\z)/ }
 
   class Response
     LOCATION = "Location".freeze
@@ -211,7 +212,7 @@ class Cuba
   private :try
 
   def consume(pattern)
-    matchdata = env[Rack::PATH_INFO].match(/\A\/(#{pattern})(\/|\z)/)
+    matchdata = env[Rack::PATH_INFO].match(REGEXES[pattern])
 
     return false unless matchdata

I run the snippet above and it now takes 2 seconds.

Twice as fast!

Do you think this is a good change? I think it's harmless: the routes of an app are fixed (not dynamic) so that REGEXES hash will reach a reasonable maximum size. Plus it can help with routes like ":id" where they might be used in several places.

What do you think?

[soveran]

I kind of replied with a 👍 and a ❤️
I think this is perfect, let's go for it. Do you want to submit the change as a PR?

[asterite]

Sure! I'll send a PR soon :-)

[asterite]

Closed by #91