feat: worker matching

[AlliBalliBaba]

This is an alternative to #1552. instead of having a new directive, this introduces a way to match workers against paths, therefore allowing placing the worker anywhere outside of the public directory..

wip

not sure yet if this is better:

php_server {
    match * worker index.php
}

or this:

php_server {
    worker {
        file index.php
        match *
    }
}

performance:

default php_server wih worker

php_server wih worker and try_files {path} index.php

php_server wih worker and match * (minimizes file operations)

[withinboredom]

I think I prefer the latter of the two options, but I can see the benefits of the first one too.

[AlliBalliBaba]

Thinking about it, the first option makes it maybe clearer that this is only possible inside of php_server.

[nickchomey]

FYI, you might want to explore using pprof's base flag, which allows for showing a flamegraph diff between two profiles.

I think this is the syntax

go tool pprof -http=:8080 -diff_base=base.prof new.prof

You could do 1v2 and 1v3 to easily see where and how much the difference is

[AlliBalliBaba]

I've managed to create a differential flamegraph via this guide

# first profile
go tool pprof -raw -output=cpu1.txt 'http://localhost:2019/debug/pprof/profile?seconds=20'

#second profile
go tool pprof -raw -output=cpu2.txt 'http://localhost:2019/debug/pprof/profile?seconds=20'

# diff flamegraph
./stackcollapse-go.pl ./cpu1.txt > out.folded1
./stackcollapse-go.pl ./cpu2.txt > out.folded2
./difffolded.pl -n out.folded2 out.folded1 | ./flamegraph.pl > /go/src/app/flamegraph.svg

A bit hard to make out what's the difference between 'red' and 'blue' though worker vs match worker

[henderkes]

I'm very strongly in favour of option one, simply because it will allow the match to behave in a general scope, such as for assets (php_server also has a file_server built in, it would also benefit) as well.

[AlliBalliBaba]

@henderkes what would file_server matching look like in your opinion? Should it 404 or just fall through if nothing matches?

php_server {
    match /path1/* worker /anywhere/worker1.php
    match /path2/* worker /anywhere/worker2.php
    match /assets/* fileserver
}

# if nothing matches continue to the next directive

[henderkes]

If /assets/some_asset.png isn't found, it should 404 of course. But that's the same as falling through to the default behaviour of file_server, is it not?

[AlliBalliBaba]

I mean if it matches neither of the paths /assets/* , /path1/* or /path2/* . Currently, php_server would always respond with a 404.

[henderkes]

oh, then it should fall through to the default behaviour.

[AlliBalliBaba]

Hmm makes sense, so the order would be:

• serve {file} if it exists, file_server is enabled and there are no other match file_server directives:
• apply all match worker and match file_server directives in order
• fallback to original try_files implementation

[henderkes]

I was thinking more along the lines of

• at the beginning of the request, loop through match directives in order, if one matches, pass it to that functionality
• no further changes to the code as it is now

[AlliBalliBaba]

Hmm I'm still not sure about match file_server, are there any cases where it wouldn't just be an abbreviation for a route? Otherwise there's also a benefit to having match inside of the worker, since it might allow matching global workers.

The reason the file_server needs to go first in the match worker case is to allow the most common use case: -> file exists: serve file -> file does not exist: forward request to worker

[henderkes]

Otherwise there's also a benefit to having match inside of the worker, since it might allow matching global workers.

That would be quite confusing and might also hurt performance of regularly handled requests, because it has to go through all global workers and try to match the path, even if it was never intended and never produces a match.

The reason the file_server needs to go first in the match worker case is to allow the most common use case: -> file exists: serve file -> file does not exist: forward request to worker

But not with the match directive. It's an explicit tell that all routes with /path1/* should be handled by the worker. There absolutely doesn't and shouldn't be a file_server involved with that at all. If a user configures /path1/* to be handled by a worker but then expects /path1/image.jpg to be served as a file, that's not our problem, just like it isn't Caddy's problem when someone uses

route {
    @assets path /path1*
    handle @assets {
         rewrite worker1.php
         php
    }
}

and then expects an image in that path1 to be served by a file server.

Hmm I'm still not sure about match file_server, are there any cases where it wouldn't just be an abbreviation for a route?

That's pretty much what it would be. The reason it should exist is something like this:

php_server {
    match /path1/images/* file_server
    match /path1/* worker worker1.php
}

And that's also what I envision the match directive being - an abbreviated version of route + matchers + handle directives, but it carries the same performance benefit that you were after and also allows workers in arbitrary paths.

What I think would happen when a php_server (or php) directive is hit in pseudocode:

(FrankenPHPModule* f) handleRequest(httpRequest r) {
    path := r.URL.Path
    for pattern, handler in range f.matches {
        if path matches pattern
            handler(r)
    }
    // fallthrough to current logic
}
    

[AlliBalliBaba]

(FrankenPHPModule* f) handleRequest(httpRequest r) { path := r.URL.Path for pattern, handler in range f.matches { if path matches pattern handler(r) } // fallthrough to current logic }

This would not work since we would never hit the php module via try_files. Matching needs to also happen beforehand on the caddy routing level.

That's pretty much what it would be. The reason it should exist is something like this:

php_server { match /path1/images/* file_server match /path1/* worker worker1.php }

While I can see the benefit of an abbreviation like this, it wouldn't cover the most common use case, which is serve file -> fallback to worker (the default in most frameworks with a worker implementation)

So I'm still a bit torn, but now leaning more towards the worker->match direction, also because the implementation is simpler.

php_server {
  worker {
    file /anywhere/worker.php
    match * 
  }
}

[henderkes]

(FrankenPHPModule* f) handleRequest(httpRequest r) { path := r.URL.Path for pattern, handler in range f.matches { if path matches pattern handler(r) } // fallthrough to current logic }

This would not work since we would never hit the php module via try_files. Matching needs to also happen beforehand on the caddy routing level.

In that case match ... file_server really doesn't make sense. Unless we were to register a different directive, but that's exactly what I wanted to avoid.

That's pretty much what it would be. The reason it should exist is something like this: php_server { match /path1/images/* file_server match /path1/* worker worker1.php }

While I can see the benefit of an abbreviation like this, it wouldn't cover the most common use case, which is serve file -> fallback to worker (the default in most frameworks with a worker implementation)

If serving files is always tried before our php module handler is even hit, wouldn't the equivalent just be

php_server {
    match /path1/* worker worker1.php
    // fall back to normal logic to see if we can match against a worker or use a regular php thread
}

So I'm still a bit torn, but now leaning more towards the worker->match direction, also because the implementation is simpler.

php_server {
  worker {
    file /anywhere/worker.php
    match * 
  }
}

I really don't like the other way around. It just doesn't make sense. The worker doesn't perform any matching, the worker should never even be queried if it didn't match the request.

[AlliBalliBaba]

Yeah these 2 would be equivalent, depends on how you think about it and how this might be extended in the future

php_server {
    match /path/* worker /anywhere/worker.php
}

php_server {
  worker {
    file /anywhere/worker.php
    match /path/*
  }
}

[henderkes]

I see, yeah, I still favour option one simply because of clarity, but since there's no actual benefit, it's not that important.

Damn, I was really hoping we could transform the current "optimal" setup into a simpler version.

sub.domain.tld {
    root /path/to/public
    route {
        @assets {
            path /assets/*
        }
        file_server @assets
        rewrite index.php
        php {
            root /path/to/public {
                worker index.php
            }
        }
    }
}

to

sub.domain.tld {
    php {
        root /path/to/public
        match /assets/* file_server
        match * worker index.php
    }
}

[AlliBalliBaba]

I'd still be fine with going either way, let's see what others think.

Something else to consider is that the following $_SERVER vars will differ from regular cgi spec, since the script is not in the public directory: DOCUMENT_URI, PATH_INFO, PHP_SELF, PATH_TRANSLATED

[withinboredom]

Something else to consider is that the following $_SERVER vars will differ from regular cgi spec, since the script is not in the public directory: DOCUMENT_URI, PATH_INFO, PHP_SELF, PATH_TRANSLATED

I'm not sure that is described in the spec. These relate to the URI (the path from the web) and not related to the document root -- unless I'm mis-reading it (ah, so only PATH_TRANSLATED looks like it refers to the actual file, everything else is from the URI, after rewrites and so forth). Further, you can already symlink out of the document root on other web servers and they do fine.

[AlliBalliBaba]

These 4 $_SERVER vars all somehow relate to the actual script name

So if the request looks like this: /script-name.php/path-name The vars look like this:

DOCUMENT_URI => /script-name.php PATH_INFO => /path-name PHP_SELF => /script-name.php/path-name PATH_TRANSLATED => /path/to/public/path-name

But if the script isn't located in the public directory, they kind of don't make sense. So it probably should be fine to leave them empty. They are usually also not used by modern frameworks.

[withinboredom]

All of those are URI based (with the exception of PATH_TRANSLATED), and they are the file that we would serve if it weren't intercepted by PHP. PATH_TRANSLATED is the actual file being served (ex: /app/public/test.gif in my example below).

array(17) {
  ["DOCUMENT_ROOT"]=>
  string(25) "/Users/withinboredom/code"
  ["REMOTE_ADDR"]=>
  string(9) "127.0.0.1"
  ["REMOTE_PORT"]=>
  string(5) "53054"
  ["SERVER_SOFTWARE"]=>
  string(30) "PHP/8.4.7 (Development Server)"
  ["SERVER_PROTOCOL"]=>
  string(8) "HTTP/1.1"
  ["SERVER_NAME"]=>
  string(9) "127.0.0.1"
  ["SERVER_PORT"]=>
  string(4) "8080"
  ["REQUEST_URI"]=>
  string(9) "/test.gif"
  ["REQUEST_METHOD"]=>
  string(3) "GET"
  ["SCRIPT_NAME"]=>
  string(9) "/test.gif"
  ["SCRIPT_FILENAME"]=>
  string(13) "../router.php"
  ["PHP_SELF"]=>
  string(9) "/test.gif"
  ["HTTP_HOST"]=>
  string(14) "localhost:8080"
  ["HTTP_USER_AGENT"]=>
  string(10) "curl/8.7.1"
  ["HTTP_ACCEPT"]=>
  string(3) "*/*"
  ["REQUEST_TIME_FLOAT"]=>
  float(1750081343.139124)
  ["REQUEST_TIME"]=>
  int(1750081343)
}

^^ a dump using php -S localhost:8080 -t . ../router.php and just dumping $_SERVER.

[nickchomey]

I've managed to create a differential flamegraph via this guide

# first profile
go tool pprof -raw -output=cpu1.txt 'http://localhost:2019/debug/pprof/profile?seconds=20'

#second profile
go tool pprof -raw -output=cpu2.txt 'http://localhost:2019/debug/pprof/profile?seconds=20'

# diff flamegraph
./stackcollapse-go.pl ./cpu1.txt > out.folded1
./stackcollapse-go.pl ./cpu2.txt > out.folded2
./difffolded.pl -n out.folded2 out.folded1 | ./flamegraph.pl > /go/src/app/flamegraph.svg

A bit hard to make out what's the difference between 'red' and 'blue' though worker vs match worker

As I mentioned, it is dead simple to create a diff flamegraph with native pprof capabilities.

Here's one that I made a while back while debugging a perf issue in a CDC ETL tool (ended up reducing processing time by 75% - the green is the "gains" relative to the white base. It'll show red when where's a regression.

You could probably do something like

# first profile
go tool pprof -raw -output=cpu1.txt 'http://localhost:2019/debug/pprof/profile?seconds=20'

#second profile
go tool pprof -raw -output=cpu2.txt 'http://localhost:2019/debug/pprof/profile?seconds=20'

go tool pprof -http=:8080 -diff_base=cpu1.txt cpu2.txt

Except I think you'd want to do 3 profiles and 2 diffs for the scenarios presented in the OP

Though, it'll only be useful if the different profiles have the same functions etc, so as to diff them. It appears from your diagram that they application is quite different so there's just wholesale rather than incremental differences. Still, the native tooling will surely be easier and likely more "compatible"

[AlliBalliBaba]

All of those are URI based (with the exception of PATH_TRANSLATED), and they are the file that we would serve if it weren't intercepted by PHP. PATH_TRANSLATED is the actual file being served (ex: /app/public/test.gif in my example below).+ array(17) { ["DOCUMENT_ROOT"]=>string(25) "/Users/withinboredom/code" ["REMOTE_ADDR"]=>string(9) "127.0.0.1" ["REMOTE_PORT"]=>string(5) "53054" ["SERVER_SOFTWARE"]=>string(30) "PHP/8.4.7 (Development Server)" ["SERVER_PROTOCOL"]=>string(8) "HTTP/1.1" ["SERVER_NAME"]=>string(9) "127.0.0.1" ["SERVER_PORT"]=>string(4) "8080" ["REQUEST_URI"]=>string(9) "/test.gif" ["REQUEST_METHOD"]=>string(3) "GET" ["SCRIPT_NAME"]=>string(9) "/test.gif" ["SCRIPT_FILENAME"]=>string(13) "../router.php" ["PHP_SELF"]=>string(9) "/test.gif" ["HTTP_HOST"]=>string(14) "localhost:8080" ["HTTP_USER_AGENT"]=>string(10) "curl/8.7.1" ["HTTP_ACCEPT"]=>string(3) "/" ["REQUEST_TIME_FLOAT"]=>float(1750081343.139124) ["REQUEST_TIME"]=>int(1750081343) }

I'm confused, your example doesn't contain any of the 4 vars though? (DOCUMENT_URI, PATH_INFO, PHP_SELF, PATH_TRANSLATED)

[AlliBalliBaba]

Btw my github actions are still broken, so either someone with access needs to trigger them or @Alliballibaba2 also needs to join the PHP org.

[henderkes]

Wouldn't it work to push an empty commit with the main?

[withinboredom]

Maybe try rebasing? It looks like the last commit on this branch was before the move?

[AlliBalliBaba]

Nah my account is actually unable to trigger actions, I've opened a few github support tickets since, not sure how it happened (not even using actions myself).

I might close this PR and reopen it from a fork, so @Alliballibaba2 can push to it to trigger actions.

[dunglas]

I find the flame graphs are to read. Can we have some numbers?

[AlliBalliBaba]

Benchmarks are done locally, so take them with a grain of salt. Impact will be higher if the filesystem is a bottleneck:

wrk -c 200 -t 4 - 20 CPU cores in the 'ideal' worker case

Configuration	Requests per second
php_server with worker	~86000 RPS
php_server with worker and try_files {file} index.php	~92000 RPS
php_server with worker and match *	~108000 RPS

The main benefit is being able to place the worker outside of public.

[AlliBalliBaba]

Yes I think this one is ready to merge. I'll add some docs (in this PR or a separate one, whichever you prefer)