perf: cache computations in WithRequestDocumentRoot (#1154)

caddy/caddy.go

@@ -7,6 +7,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"github.com/dunglas/frankenphp/internal/fastabs"
 	"github.com/prometheus/client_golang/prometheus"
 	"net/http"
 	"path/filepath"
@@ -270,7 +271,7 @@ func (f *FrankenPHPModule) Provision(ctx caddy.Context) error {
 	}
 
 	if !needReplacement(f.Root) {
-		root, err := filepath.Abs(f.Root)
+		root, err := fastabs.FastAbs(f.Root)
 		if err != nil {
 			return fmt.Errorf("unable to make the root path absolute: %w", err)
 		}

frankenphp_test.go

@@ -9,6 +9,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"github.com/dunglas/frankenphp/internal/fastabs"
 	"io"
 	"log"
 	"mime/multipart"
@@ -921,12 +922,13 @@ func testRejectInvalidHeaders(t *testing.T, opts *testOptions) {
 // To run this fuzzing test use: go test -fuzz FuzzRequest
 // TODO: Cover more potential cases
 func FuzzRequest(f *testing.F) {
+	absPath, _ := fastabs.FastAbs("./testdata/")
+
 	f.Add("hello world")
 	f.Add("😀😅🙃🤩🥲🤪😘😇😉🐘🧟")
 	f.Add("%00%11%%22%%33%%44%%55%%66%%77%%88%%99%%aa%%bb%%cc%%dd%%ee%%ff")
 	f.Add("\x00\x01\x02\x03\x04\x05\x06\x07\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f")
 	f.Fuzz(func(t *testing.T, fuzzedString string) {
-		absPath, _ := filepath.Abs("./testdata/")
 		runTest(t, func(handler func(http.ResponseWriter, *http.Request), _ *httptest.Server, _ int) {
 			req := httptest.NewRequest("GET", "http://example.com/server-variable", nil)
 			req.URL = &url.URL{RawQuery: "test=" + fuzzedString, Path: "/server-variable.php/" + fuzzedString}

internal/fastabs/filepath.go

@@ -0,0 +1,13 @@
+//go:build !unix
+
+package fastabs
+
+import (
+	"path/filepath"
+)
+
+// FastAbs can't be optimized on Windows because the
+// syscall.FullPath function takes an input.
+func FastAbs(path string) (string, error) {
+	return filepath.Abs(path)
+}

internal/fastabs/filepath_unix.go

@@ -0,0 +1,23 @@
+package fastabs
+
+import (
+	"os"
+	"path/filepath"
+)
+
+// FastAbs is an optimized version of filepath.Abs for Unix systems,
+// since we don't expect the working directory to ever change once
+// Caddy is running. Avoid the os.Getwd syscall overhead.
+func FastAbs(path string) (string, error) {
+	if filepath.IsAbs(path) {
+		return filepath.Clean(path), nil
+	}
+
+	if wderr != nil {
+		return "", wderr
+	}
+
+	return filepath.Join(wd, path), nil
+}
+
+var wd, wderr = os.Getwd()

internal/watcher/watch_pattern.go

@@ -3,6 +3,7 @@
 package watcher
 
 import (
+	"github.com/dunglas/frankenphp/internal/fastabs"
 	"path/filepath"
 	"strings"
 
@@ -34,7 +35,7 @@ func parseFilePattern(filePattern string) (*watchPattern, error) {
 	w := &watchPattern{}
 
 	// first we clean the pattern
-	absPattern, err := filepath.Abs(filePattern)
+	absPattern, err := fastabs.FastAbs(filePattern)
 	if err != nil {
 		return nil, err
 	}

metrics.go

@@ -1,7 +1,7 @@
 package frankenphp
 
 import (
-	"path/filepath"
+	"github.com/dunglas/frankenphp/internal/fastabs"
 	"regexp"
 	"sync"
 	"time"
@@ -126,7 +126,7 @@ func (m *PrometheusMetrics) StopWorker(name string, reason StopReason) {
 }
 
 func (m *PrometheusMetrics) getIdentity(name string) (string, error) {
-	actualName, err := filepath.Abs(name)
+	actualName, err := fastabs.FastAbs(name)
 	if err != nil {
 		return name, err
 	}

request_options.go

@@ -1,7 +1,10 @@
 package frankenphp
 
 import (
+	"github.com/dunglas/frankenphp/internal/fastabs"
 	"path/filepath"
+	"sync"
+	"sync/atomic"
 
 	"go.uber.org/zap"
 )
@@ -9,6 +12,11 @@ import (
 // RequestOption instances allow to configure a FrankenPHP Request.
 type RequestOption func(h *FrankenPHPContext) error
 
+var (
+	documentRootCache    sync.Map
+	documentRootCacheLen atomic.Uint32
+)
+
 // WithRequestDocumentRoot sets the root directory of the PHP application.
 // if resolveSymlink is true, oath declared as root directory will be resolved
 // to its absolute value after the evaluation of any symbolic links.
@@ -17,20 +25,29 @@ type RequestOption func(h *FrankenPHPContext) error
 // symlink is changed without PHP being restarted; enabling this
 // directive will set $_SERVER['DOCUMENT_ROOT'] to the real directory path.
 func WithRequestDocumentRoot(documentRoot string, resolveSymlink bool) RequestOption {
-	return func(o *FrankenPHPContext) error {
-		// make sure file root is absolute
-		root, err := filepath.Abs(documentRoot)
-		if err != nil {
-			return err
+	return func(o *FrankenPHPContext) (err error) {
+		v, ok := documentRootCache.Load(documentRoot)
+		if !ok {
+			// make sure file root is absolute
+			v, err = fastabs.FastAbs(documentRoot)
+			if err != nil {
+				return err
+			}
+
+			// prevent the cache to grow forever, this is a totally arbitrary value
+			if documentRootCacheLen.Load() < 1024 {
+				documentRootCache.LoadOrStore(documentRoot, v)
+				documentRootCacheLen.Add(1)
+			}
 		}
 
 		if resolveSymlink {
-			if root, err = filepath.EvalSymlinks(root); err != nil {
+			if v, err = filepath.EvalSymlinks(v.(string)); err != nil {
 				return err
 			}
 		}
 
-		o.documentRoot = root
+		o.documentRoot = v.(string)
 
 		return nil
 	}

static-builder.Dockerfile

@@ -103,7 +103,7 @@ RUN go mod graph | awk '{if ($1 !~ "@") print $2}' | xargs go get
 WORKDIR /go/src/app
 COPY --link *.* ./
 COPY --link caddy caddy
-COPY --link internal/watcher internal/watcher
+COPY --link internal internal
 
 RUN --mount=type=secret,id=github-token GITHUB_TOKEN=$(cat /run/secrets/github-token) ./build-static.sh && \
 	rm -Rf dist/static-php-cli/source/*

worker.go

@@ -5,6 +5,7 @@ package frankenphp
 import "C"
 import (
 	"fmt"
+	"github.com/dunglas/frankenphp/internal/fastabs"
 	"net/http"
 	"path/filepath"
 	"sync"
@@ -63,7 +64,7 @@ func initWorkers(opt []workerOpt) error {
 }
 
 func newWorker(o workerOpt) (*worker, error) {
-	absFileName, err := filepath.Abs(o.fileName)
+	absFileName, err := fastabs.FastAbs(o.fileName)
 	if err != nil {
 		return nil, fmt.Errorf("worker filename is invalid %q: %w", o.fileName, err)
 	}