optimize: faster router

BREAKING CHANGE: Removed `Prism::Router::Cacher`.

Closes #45

bench/router.cr

@@ -16,17 +16,7 @@ def dynamic_path_request
   HTTP::Server::Context.new(HTTP::Request.new("GET", "/foo/#{rand(DYNAMIC_ROUTES_NUMBER)}"), RESPONSE)
 end
 
-simple_cacher = Prism::Router::SimpleCacher.new(100_000)
-
-simply_cached_router = Prism::Router.new(simple_cacher) do
-  get "/foo/:number" do |env|
-  end
-
-  get "/bar" do |env|
-  end
-end
-
-non_cached_router = Prism::Router.new do
+router = Prism::Router.new do
   get "/foo/:number" do |env|
   end
 
@@ -36,41 +26,16 @@ end
 
 require "benchmark"
 
-puts "\nBegin benchmarking router..."
-puts "Running static paths...\n\n"
-
-Benchmark.ips do |x|
-  x.report("simply cached router with static path") do
-    simply_cached_router.call(static_path_request)
-  end
-
-  x.report("non-cached router with static path") do
-    non_cached_router.call(static_path_request)
-  end
-end
-
-puts "\nRunning dynamic paths (random from #{DYNAMIC_ROUTES_NUMBER} known paths)...\n\n"
-
 Benchmark.ips do |x|
-  x.report("simply cached router with dynamic paths") do
-    simply_cached_router.call(dynamic_path_request)
+  x.report("with static path") do
+    router.call(static_path_request)
   end
 
-  x.report("non-cached router with dynamic paths") do
-    non_cached_router.call(dynamic_path_request)
+  x.report("with dynamic paths") do
+    router.call(dynamic_path_request)
   end
-end
 
-puts "\nRunning absolutely random paths (caching is useless)...\n\n"
-
-Benchmark.ips do |x|
-  x.report("simply cached router with random paths") do
-    simply_cached_router.call(random_path_request)
-  end
-
-  x.report("non-cached router with random paths") do
-    non_cached_router.call(random_path_request)
+  x.report("with random paths") do
+    router.call(random_path_request)
   end
 end
-
-puts "\n✔️ Done benchmarking router"

spec/router_spec.cr

@@ -43,8 +43,8 @@ class Prism::Router
           context.request.action.should be_a(::Proc(HTTP::Server::Context, Nil))
         end
 
-        it "sets empty path params" do
-          context.request.path_params.not_nil!.empty?.should be_true
+        it "does not set path params" do
+          context.request.path_params.should be_nil
         end
       end
 
@@ -84,22 +84,6 @@ class Prism::Router
           context.request.path_params.should be_nil
         end
       end
-
-      context "with Simple cacher" do
-        cacher = Prism::Router::SimpleCacher.new(10_000)
-
-        router = Prism::Router.new(cacher) do
-          get "/" do |env|
-            env.response.print("Hello!")
-          end
-        end
-
-        context = dummy_context(Req.new("GET", "/"))
-
-        it "works" do
-          router.call(context)
-        end
-      end
     end
   end
 end

src/prism/router.cr

@@ -2,7 +2,6 @@ require "radix"
 require "http/web_socket"
 require "./ext/http/request/action"
 require "./ext/http/request/path_params"
-require "./router/*"
 require "./action"
 require "./channel"
 
@@ -11,8 +10,6 @@ module Prism
   #
   # Always calls next handler.
   #
-  # See `Cacher` for known caching implementations.
-  #
   # ```
   # router = Prism::Router.new do
   #   get "/" do |context|
@@ -34,11 +31,9 @@ module Prism
     # :nodoc:
     HTTP_METHODS = %w(get post put patch delete options)
     @tree = Radix::Tree(Node).new
+    @hash = {} of String => Node
 
-    # Cacher used by this router. Can be changed in the runtime.
-    property cacher
-
-    # Initialize a new router with optional *cacher* and yield it. You should then define routes in *&block*.
+    # Initialize a new router and yield it. You should then define routes in *&block*.
     #
     # ```
     # # The simplest router
@@ -47,35 +42,24 @@ module Prism
     #     env.response.print "Hello world!"
     #   end
     # end
-    #
-    # # Add some caching
-    # cacher = Prism::Router::SimpleCacher.new(10_000)
-    # router = Prism::Router.new(cacher) do
-    #   # ditto
-    # end
     # ```
-    def initialize(@cacher : Cacher? = nil)
-    end
-
-    def self.new(cacher = nil)
-      instance = Router.new(cacher)
+    def self.new
+      instance = Router.new
       with instance yield
       instance
     end
 
     def call(context : HTTP::Server::Context)
       if context.request.headers.includes_word?("Upgrade", "Websocket")
         path = "/ws" + context.request.path
-        result = @cacher ? @cacher.not_nil!.find(@tree, path) : @tree.find(path)
+        result = lookup(path)
       else
         path = "/" + context.request.method.downcase + context.request.path
-        result = @cacher ? @cacher.not_nil!.find(@tree, path) : @tree.find(path)
+        result = lookup(path)
       end
 
-      if result.found?
-        context.request.action = result.payload
-        context.request.path_params = result.params
-      end
+      context.request.action = result.payload
+      context.request.path_params = result.params
 
       call_next(context)
     end
@@ -91,11 +75,7 @@ module Prism
     # ```
     def on(path, methods : Array(String), &proc : ContextProc)
       methods.map(&.downcase).each do |method|
-        begin
-          @tree.add("/" + method + path, proc)
-        rescue Radix::Tree::DuplicateError
-          raise DuplicateRouteError.new(method.upcase + " " + path)
-        end
+        add("/" + method + path, proc)
       end
     end
 
@@ -108,11 +88,7 @@ module Prism
     # ```
     def on(path, methods : Array(String), action : Action.class)
       methods.map(&.downcase).each do |method|
-        begin
-          @tree.add("/" + method + path, ContextProc.new { |c| action.call(c) }.as(Node))
-        rescue Radix::Tree::DuplicateError
-          raise DuplicateRouteError.new(method.upcase + " " + path)
-        end
+        add("/" + method + path, ContextProc.new { |c| action.call(c) }.as(Node))
       end
     end
 
@@ -125,11 +101,7 @@ module Prism
     # ```
     def on(path, methods : Array(String))
       methods.map(&.downcase).each do |method|
-        begin
-          @tree.add("/" + method + path, ContextProc.new { })
-        rescue Radix::Tree::DuplicateError
-          raise DuplicateRouteError.new(method.upcase + " " + path)
-        end
+        add("/" + method + path, ContextProc.new { })
       end
     end
 
@@ -182,11 +154,7 @@ module Prism
     # end
     # ```
     def ws(path, &proc : WebSocketProc)
-      begin
-        @tree.add("/ws" + path, HTTP::WebSocketHandler.new(&proc))
-      rescue Radix::Tree::DuplicateError
-        raise DuplicateRouteError.new("WS " + path)
-      end
+      add("/ws" + path, HTTP::WebSocketHandler.new(&proc))
     end
 
     # Draw a WebSocket route for *path* instantiating *channel*.
@@ -199,11 +167,7 @@ module Prism
     # end
     # ```
     def ws(path, channel : Channel.class)
-      begin
-        @tree.add("/ws" + path, WebSocketProc.new { |s, c| MyChannel.call(s, c) }.as(Node))
-      rescue Radix::Tree::DuplicateError
-        raise DuplicateRouteError.new("WS " + path)
-      end
+      add("/ws" + path, WebSocketProc.new { |s, c| MyChannel.call(s, c) }.as(Node))
     end
 
     # Raised if duplicate route found.
@@ -214,5 +178,37 @@ module Prism
         super("Duplicate route found: #{route}")
       end
     end
+
+    protected def add(path, node)
+      if path.includes?(':')
+        @tree.add(path, node)
+      else
+        raise DuplicateRouteError.new(path) if @hash.has_key?(path)
+        @hash[path] = node
+      end
+    rescue Radix::Tree::DuplicateError
+      raise DuplicateRouteError.new(path)
+    end
+
+    private struct Result
+      getter payload : Node?
+      getter params : Hash(String, String)? = nil
+
+      def initialize(@payload : Node?)
+      end
+
+      def initialize(result : Radix::Result)
+        if result.found?
+          @payload = result.payload
+          @params = result.params
+        end
+      end
+    end
+
+    protected def lookup(path)
+      Result.new(@hash.fetch(path) do
+        @tree.find(path)
+      end)
+    end
   end
 end