feat(routing): add regex chunking to route regex

[blackshadev]

What

Add regex chunking to route regex. When loading in massive amounts of routes a singular regex can become to large for PCRE to process. A solution for this is to split it up in multiple separate regexes, this is called chunking. With this PR is should be possible to load in as many routes as you like, as long as your memory can hold it.

In order to facilitate this I needed to rework how we build up the regex, instead of using recursion and having the stack implicitly as a php call stack. I needed to implement it manually such that, when it is detected we require a new chunk, we could rebuild the regex prefix based on the stack and also finish it based on it.

Outstanding debates

RouteMatchingRegexesBuilder performance considerations

I isolated the regex buildin into src/Tempest/Http/src/Routing/Construction/RouteMatchingRegexesBuilder.php and one singular function. This is mostly because of performance reasons. The inner part of the loop is called so many times, every function call needed to count. So there is a trade-off between readability and performance. Currently I opted to go slightly more towards the performance side. If you think it should be implemented into multiple methods, do know it comes at a cost. (If needed, we can use the benchmarks to signify the cost of alterations).

This is also the reason for using null as a end marker instead of wrapping all nodes into a VO which also would translate to an end-marker. I saw a performance drop of arround 60%, I went from 120μs to 190μs

PHPunit memory limit

I bumped the phpunit memory limit. I couldn't find a "leak" in the newly added code. And it mainly breaks on the integration part. My expectation is the integration tests are already quite hefty on memory and my changes tipped it over the limit. I used memprof to find any culprits, but I couldn't find any glaring issues, most memory consumption is not in "my" code or even near it.

[brendt]

Great PR 👍

[coveralls]

Pull Request Test Coverage Report for Build 11799155234

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

• For more information on this, see Tracking coverage changes with pull request builds.
• To avoid this issue with future PRs, see these Recommended CI Configurations.
• For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

• 71 of 73 (97.26%) changed or added relevant lines in 5 files are covered.
• 19 unchanged lines in 2 files lost coverage.
• Overall coverage increased (+0.1%) to 82.551%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/Tempest/Http/src/Routing/Matching/MatchingRegex.php	11	13	84.62%

Files with Coverage Reduction	New Missed Lines	%
src/Tempest/Console/src/ConsoleApplication.php	7	10.0%
src/Tempest/Http/src/HttpApplication.php	12	0.0%

Totals
Change from base Build 11770148738:	0.1%
Covered Lines:	7333
Relevant Lines:	8883

---

💛 - Coveralls

[brendt]

Do you wanna take a look at the merge conflict? Other than that I'm fine merging this 👍

It's not a blocker for me, but I do like for null to have meaning. So to me it makes more sense that RouteMatch can be nullable instead being notFound.

Look at it from a language perspective: "I have a route match" — that always means that there is a matching route. When you say "I don't have a route match", it means there is no matching route (ie. null).

[blackshadev]

Do you wanna take a look at the merge conflict? Other than that I'm fine merging this 👍

Done

It's not a blocker for me, but I do like for null to have meaning. So to me it makes more sense that RouteMatch can be nullable instead being notFound.

Look at it from a language perspective: "I have a route match" — that always means that there is a matching route. When you say "I don't have a route match", it means there is no matching route (ie. null).

done

It is a matter of opinion I changed it to null. I do not like null because you end up with allot of null checks everywhere. While not all nulls are equal. So in our case the Route match was not found. Which is something different than an error (maybe a case for later) or a route that is missing (as in, not completely loaded in). By returning null you will be missing these cases. But as state, it is a matter of opinion, and I do not see any harm in going your way on this. More so because it is a bit more in the "tempest" style.

[brendt]

Thanks! Great PR 💪