Merge pull request #658 from flightphp/router-performance

n0nag0n · web-flow · commit f0dc1b1adf8e · 2025-08-19T06:54:58.000-06:00
Router performance enhancements
diff --git a/flight/net/Router.php b/flight/net/Router.php
@@ -29,6 +29,13 @@ class Router
      */
     protected array $routes = [];
 
+    /**
+     * Routes grouped by HTTP method for faster lookups
+     *
+     * @var array<string, array<int, Route>>
+     */
+    protected array $routesByMethod = [];
+
     /**
      * The current route that is has been found and executed.
      */
@@ -82,6 +89,7 @@ public function getRoutes(): array
     public function clear(): void
     {
         $this->routes = [];
+        $this->routesByMethod = [];
     }
 
     /**
@@ -134,6 +142,14 @@ public function map(string $pattern, $callback, bool $pass_route = false, string
 
         $this->routes[] = $route;
 
+        // Group routes by HTTP method for faster lookups
+        foreach ($methods as $method) {
+            if (!isset($this->routesByMethod[$method])) {
+                $this->routesByMethod[$method] = [];
+            }
+            $this->routesByMethod[$method][] = $route;
+        }
+
         return $route;
     }
 
@@ -228,17 +244,48 @@ public function group(string $groupPrefix, callable $callback, array $groupMiddl
      */
     public function route(Request $request)
     {
-        while ($route = $this->current()) {
-            $urlMatches = $route->matchUrl($request->url, $this->caseSensitive);
-            $methodMatches = $route->matchMethod($request->method);
-            if ($urlMatches === true && $methodMatches === true) {
-                $this->executedRoute = $route;
-                return $route;
-                // capture the route but don't execute it. We'll use this in Engine->start() to throw a 405
-            } elseif ($urlMatches === true && $methodMatches === false) {
-                $this->executedRoute = $route;
+        $requestMethod = $request->method;
+        $requestUrl = $request->url;
+
+        // If we're in the middle of iterating (index > 0), continue with the original iterator logic
+        // This handles cases where the Engine calls next() and continues routing (e.g., when routes return true)
+        if ($this->index > 0) {
+            while ($route = $this->current()) {
+                $urlMatches = $route->matchUrl($requestUrl, $this->caseSensitive);
+                $methodMatches = $route->matchMethod($requestMethod);
+                if ($urlMatches === true && $methodMatches === true) {
+                    $this->executedRoute = $route;
+                    return $route;
+                } elseif ($urlMatches === true && $methodMatches === false) {
+                    $this->executedRoute = $route;
+                }
+                $this->next();
+            }
+            return false;
+        }
+
+        // Fast path: check method-specific routes first, then wildcard routes (only on first routing attempt)
+        $methodsToCheck = [$requestMethod, '*'];
+        foreach ($methodsToCheck as $method) {
+            if (isset($this->routesByMethod[$method])) {
+                foreach ($this->routesByMethod[$method] as $route) {
+                    if ($route->matchUrl($requestUrl, $this->caseSensitive)) {
+                        $this->executedRoute = $route;
+                        // Set iterator position to this route for potential next() calls
+                        $this->index = array_search($route, $this->routes, true);
+                        return $route;
+                    }
+                }
+            }
+        }
+
+        // If no exact match found, check all routes for 405 (method not allowed) cases
+        // This maintains the original behavior where we capture routes that match URL but not method
+        foreach ($this->routes as $route) {
+            if ($route->matchUrl($requestUrl, $this->caseSensitive) && !$route->matchMethod($requestMethod)) {
+                $this->executedRoute = $route; // Capture for 405 error in Engine
+                // Don't return false yet, continue checking for other potential matches
             }
-            $this->next();
         }
 
         return false;
diff --git a/tests/EngineTest.php b/tests/EngineTest.php
@@ -174,6 +174,108 @@ public function getInitializedVar()
         $engine->start();
     }
 
+    public function testDoubleReturnTrueRoutesContinueIteration(): void
+    {
+        $_SERVER['REQUEST_METHOD'] = 'GET';
+        $_SERVER['REQUEST_URI'] = '/someRoute';
+
+        $engine = new class extends Engine {
+            public function getInitializedVar()
+            {
+                return $this->initialized;
+            }
+        };
+        
+        // First route that returns true (should continue routing)
+        $engine->route('/someRoute', function () {
+            echo 'first route ran, ';
+            return true;
+        }, true);
+        
+        // Second route that should be found and executed
+        $engine->route('/someRoute', function () {
+            echo 'second route executed!';
+        }, true);
+        
+        $this->expectOutputString('first route ran, second route executed!');
+        $engine->start();
+    }
+
+    public function testDoubleReturnTrueWithMethodMismatchDuringIteration(): void
+    {
+        $_SERVER['REQUEST_METHOD'] = 'POST';
+        $_SERVER['REQUEST_URI'] = '/someRoute';
+
+        $engine = new class extends Engine {
+            public function getInitializedVar()
+            {
+                return $this->initialized;
+            }
+            
+            public function getLoader()
+            {
+                return $this->loader;
+            }
+        };
+        
+        // Mock response to prevent actual headers
+        $engine->getLoader()->register('response', function () {
+            return new class extends Response {
+                public function setRealHeader(string $header_string, bool $replace = true, int $response_code = 0): self
+                {
+                    return $this;
+                }
+            };
+        });
+        
+        // First route that returns true and matches POST
+        $engine->route('POST /someRoute', function () {
+            echo 'first POST route ran, ';
+            return true;
+        }, true);
+        
+        // Second route that matches URL but wrong method (GET) - should be captured for 405
+        $engine->route('GET /someRoute', function () {
+            echo 'should not execute';
+        }, true);
+        
+        // Third route that matches POST and should execute
+        $engine->route('POST /someRoute', function () {
+            echo 'second POST route executed!';
+        }, true);
+        
+        $this->expectOutputString('first POST route ran, second POST route executed!');
+        $engine->start();
+    }
+
+    public function testIteratorReachesEndWithoutMatch(): void
+    {
+        $_SERVER['REQUEST_METHOD'] = 'GET';
+        $_SERVER['REQUEST_URI'] = '/someRoute';
+
+        $engine = new class extends Engine {
+            public function getInitializedVar()
+            {
+                return $this->initialized;
+            }
+        };
+        
+        // Route that returns true (continues iteration)
+        $engine->route('/someRoute', function () {
+            echo 'first route ran, ';
+            return true;
+        }, true);
+        
+        // Route with different URL that won't match
+        $engine->route('/differentRoute', function () {
+            echo 'should not execute';
+        }, true);
+        
+        // No more matching routes - should reach end of iterator and return 404
+        $this->expectOutputString('<h1>404 Not Found</h1><h3>The page you have requested could not be found.</h3>');
+        $engine->start();
+    }
+
 	public function testDoubleStart() 
     {
 		$engine = new Engine();
diff --git a/tests/performance/index.php b/tests/performance/index.php
@@ -0,0 +1,120 @@
+<?php
+
+declare(strict_types=1);
+
+require __DIR__ . '/vendor/autoload.php';
+
+// Route to list all available test routes
+Flight::route('GET /', function () {
+    echo "<h2>Available Test Routes:</h2><ul>";
+    echo "<li><a href='/route0'>/route0</a> (static)</li>";
+    echo "<li><a href='/user1/123'>/user1/123</a> (single param)</li>";
+    echo "<li><a href='/post2/tech/my-article'>/post2/tech/my-article</a> (multiple params)</li>";
+    echo "<li><a href='/api3/456'>/api3/456</a> (regex constraint)</li>";
+    echo "<li>/submit4/document (POST only)</li>";
+    echo "<li><a href='/admin5/dashboard'>/admin5/dashboard</a> (grouped)</li>";
+    echo "<li><a href='/admin5/users/789'>/admin5/users/789</a> (grouped with regex)</li>";
+    echo "<li><a href='/file6/path/to/document.pdf'>/file6/path/to/document.pdf</a> (complex regex)</li>";
+    echo "<li><a href='/resource7/999'>/resource7/999</a> (multi-method)</li>";
+    echo "</ul>";
+    echo "<h3>Performance Test URLs:</h3>";
+    echo "<p>Static routes: /route0, /route8, /route16, /route24, /route32, /route40, /route48</p>";
+    echo "<p>Param routes: /user1/123, /user9/456, /user17/789</p>";
+    echo "<p>Complex routes: /post2/tech/article, /api3/123, /file6/test.txt</p>";
+});
+
+
+for ($i = 0; $i < 50; $i++) {
+    $route_type = $i % 8;
+
+    switch ($route_type) {
+        case 0:
+            // Simple static routes
+            Flight::route("GET /route{$i}", function () use ($i) {
+                echo "This is static route {$i}";
+            });
+            break;
+
+        case 1:
+            // Routes with single parameter
+            Flight::route("GET /user{$i}/@id", function ($id) use ($i) {
+                echo "User route {$i} with ID: {$id}";
+            });
+            break;
+
+        case 2:
+            // Routes with multiple parameters
+            Flight::route("GET /post{$i}/@category/@slug", function ($category, $slug) use ($i) {
+                echo "Post route {$i}: {$category}/{$slug}";
+            });
+            break;
+
+        case 3:
+            // Routes with regex constraints
+            Flight::route("GET /api{$i}/@id:[0-9]+", function ($id) use ($i) {
+                echo "API route {$i} with numeric ID: {$id}";
+            });
+            break;
+
+        case 4:
+            // POST routes with parameters
+            Flight::route("POST /submit{$i}/@type", function ($type) use ($i) {
+                echo "POST route {$i} with type: {$type}";
+            });
+            break;
+
+        case 5:
+            // Grouped routes
+            Flight::group("/admin{$i}", function () use ($i) {
+                Flight::route("GET /dashboard", function () use ($i) {
+                    echo "Admin dashboard {$i}";
+                });
+                Flight::route("GET /users/@id:[0-9]+", function ($id) use ($i) {
+                    echo "Admin user {$i}: {$id}";
+                });
+            });
+            break;
+
+        case 6:
+            // Complex regex patterns
+            Flight::route("GET /file{$i}/@path:.*", function ($path) use ($i) {
+                echo "File route {$i} with path: {$path}";
+            });
+            break;
+
+        case 7:
+            // Multiple HTTP methods
+            Flight::route("GET|POST|PUT /resource{$i}/@id", function ($id) use ($i) {
+                echo "Multi-method route {$i} for resource: {$id}";
+            });
+            break;
+    }
+}
+// Add some predictable routes for easy performance testing
+Flight::route('GET /test-static', function () {
+    $memory_start = memory_get_usage();
+    $memory_peak = memory_get_peak_usage();
+    echo "Static test route";
+    if (isset($_GET['memory'])) {
+        echo "\nMemory: " . round($memory_peak / 1024, 2) . " KB";
+    }
+});
+
+Flight::route('GET /test-param/@id', function ($id) {
+    $memory_start = memory_get_usage();
+    $memory_peak = memory_get_peak_usage();
+    echo "Param test route: {$id}";
+    if (isset($_GET['memory'])) {
+        echo "\nMemory: " . round($memory_peak / 1024, 2) . " KB";
+    }
+});
+
+Flight::route('GET /test-complex/@category/@slug', function ($category, $slug) {
+    $memory_start = memory_get_usage();
+    $memory_peak = memory_get_peak_usage();
+    echo "Complex test route: {$category}/{$slug}";
+    if (isset($_GET['memory'])) {
+        echo "\nMemory: " . round($memory_peak / 1024, 2) . " KB";
+    }
+});
+Flight::start();
diff --git a/tests/performance/performance_tests.sh b/tests/performance/performance_tests.sh
@@ -0,0 +1,67 @@
+#!/bin/bash
+
+# Allow URL to be set via environment variable or first command-line argument, default to localhost for safety
+URL="${URL:-${1:-http://localhost:8080/test-static}}"
+REQUESTS=1000
+CONCURRENCY=10
+ITERATIONS=10
+
+declare -a times=()
+total=0
+
+echo "Benchmarking: $URL"
+echo "Requests per test: $REQUESTS"
+echo "Concurrency: $CONCURRENCY"
+echo "Iterations: $ITERATIONS"
+echo "========================================"
+
+# First, get a baseline memory reading
+echo "Getting memory baseline..."
+memory_response=$(curl -s "${URL}?memory=1")
+baseline_memory=$(echo "$memory_response" | grep "Memory:" | awk '{print $2}')
+echo "Baseline memory usage: ${baseline_memory} KB"
+echo "----------------------------------------"
+
+for i in $(seq 1 $ITERATIONS); do
+    printf "Run %2d/%d: " $i $ITERATIONS
+    
+    # Run ab and extract time per request
+    result=$(ab -n $REQUESTS -c $CONCURRENCY $URL 2>/dev/null)
+    time_per_request=$(echo "$result" | grep "Time per request:" | head -1 | awk '{print $4}')
+    requests_per_sec=$(echo "$result" | grep "Requests per second:" | awk '{print $4}')
+    
+    times+=($time_per_request)
+    total=$(echo "$total + $time_per_request" | bc -l)
+    
+    printf "%.3f ms (%.2f req/s)\n" $time_per_request $requests_per_sec
+done
+
+# Calculate statistics
+average=$(echo "scale=3; $total / $ITERATIONS" | bc -l)
+
+# Find min and max
+min=${times[0]}
+max=${times[0]}
+for time in "${times[@]}"; do
+    if (( $(echo "$time < $min" | bc -l) )); then
+        min=$time
+    fi
+    if (( $(echo "$time > $max" | bc -l) )); then
+        max=$time
+    fi
+done
+
+echo "========================================"
+echo "Results:"
+echo "Average Time per Request: $average ms"
+echo "Min Time per Request: $min ms"
+echo "Max Time per Request: $max ms"
+echo "Range: $(echo "scale=3; $max - $min" | bc -l) ms"
+echo "Baseline Memory Usage: ${baseline_memory} KB"
+
+# Get final memory reading after stress test
+echo "----------------------------------------"
+echo "Getting post-test memory reading..."
+final_memory_response=$(curl -s "${URL}?memory=1")
+final_memory=$(echo "$final_memory_response" | grep "Memory:" | awk '{print $2}')
+echo "Final memory usage: ${final_memory} KB"
