Add RegexpCompileFunc to override regexp.Compile

[titpetric]

trafficstars

What type of PR is this? (check all applicable)

• [x] Refactor
• [x] Feature
• [ ] Bug Fix
• [x] Optimization
• [ ] Documentation Update

Description

Route creation is slow as it always invokes regexp.Compile, increasing memory and cpu pressure in cases where route creation is common (e.g. an api gateway reloading changes). This PR adds a exported RegexpCompileFunc variable, that's possible to override with other implementations, adding caches. Added a benchmark.

Related Tickets & Documents

# go test -bench=NewRouter -benchmem -memprofile memprofile.out -cpuprofile profile.out -count 10
goos: linux
goarch: amd64
pkg: github.com/gorilla/mux
cpu: 11th Gen Intel(R) Core(TM) i7-1185G7 @ 3.00GHz
BenchmarkNewRouter-8             	   36786	     34450 ns/op	   26327 B/op	     374 allocs/op
BenchmarkNewRouter-8             	   44839	     31840 ns/op	   26327 B/op	     374 allocs/op
BenchmarkNewRouter-8             	   48939	     23419 ns/op	   26327 B/op	     374 allocs/op
BenchmarkNewRouter-8             	   52255	     25949 ns/op	   26327 B/op	     374 allocs/op
BenchmarkNewRouter-8             	   44684	     23150 ns/op	   26329 B/op	     374 allocs/op
BenchmarkNewRouter-8             	   41522	     35047 ns/op	   26328 B/op	     374 allocs/op
BenchmarkNewRouter-8             	   48354	     35461 ns/op	   26329 B/op	     374 allocs/op
BenchmarkNewRouter-8             	   33850	     38467 ns/op	   26330 B/op	     374 allocs/op
BenchmarkNewRouter-8             	   36920	     33917 ns/op	   26330 B/op	     374 allocs/op
BenchmarkNewRouter-8             	   40767	     29980 ns/op	   26329 B/op	     374 allocs/op
BenchmarkNewRouterRegexpFunc-8   	  303121	      3738 ns/op	    2161 B/op	      73 allocs/op
BenchmarkNewRouterRegexpFunc-8   	  311847	      4184 ns/op	    2161 B/op	      73 allocs/op
BenchmarkNewRouterRegexpFunc-8   	  300144	      4212 ns/op	    2161 B/op	      73 allocs/op
BenchmarkNewRouterRegexpFunc-8   	  329636	      4528 ns/op	    2161 B/op	      73 allocs/op
BenchmarkNewRouterRegexpFunc-8   	  277980	      5136 ns/op	    2161 B/op	      73 allocs/op
BenchmarkNewRouterRegexpFunc-8   	  363416	      4578 ns/op	    2161 B/op	      73 allocs/op
BenchmarkNewRouterRegexpFunc-8   	  328996	      4231 ns/op	    2161 B/op	      73 allocs/op
BenchmarkNewRouterRegexpFunc-8   	  327628	      3699 ns/op	    2161 B/op	      73 allocs/op
BenchmarkNewRouterRegexpFunc-8   	  300370	      3955 ns/op	    2161 B/op	      73 allocs/op
BenchmarkNewRouterRegexpFunc-8   	  297345	      4115 ns/op	    2161 B/op	      73 allocs/op
PASS
ok  	github.com/gorilla/mux	35.091s

Benchmark: NewRouter

Average runtime per iteration: ~34450 ns/op Average memory usage per iteration: ~26327 B/op Average allocations per iteration: ~374 allocs/op

Benchmark: NewRouterRegexpFunc

Average runtime per iteration: ~4184 ns/op (approx. 87.8% improvement) Average memory usage per iteration: ~2161 B/op (approx. 91.8% reduction) Average allocations per iteration: ~73 allocs/op (approx. 80.5% reduction)

As per go doc, using *regexp.Regexp concurrently is safe.

Added/updated tests?

• [x] Yes
• [ ] No, and this is why: please replace this line with details on why tests have not been included
• [ ] I need help with writing tests

Run verifications and test

• [ ] make verify is passing
• [x] make test is passing

[codecov[bot]]

Codecov Report

Merging #731 (7784a7c) into main (3401478) will not change coverage. The diff coverage is 100.00%.

:exclamation: Current head 7784a7c differs from pull request most recent head 9efbf04. Consider uploading reports for the commit 9efbf04 to get more accurate results

@@           Coverage Diff           @@
##             main     #731   +/-   ##
=======================================
  Coverage   78.04%   78.04%           
=======================================
  Files           5        5           
  Lines         902      902           
=======================================
  Hits          704      704           
  Misses        142      142           
  Partials       56       56           

Files Changed	Coverage Δ
mux.go	83.52% <100.00%> (ø)
regexp.go	83.05% <100.00%> (ø)

[coreydaley]

@titpetric Thank you for the pull request! We will need a little bit of time to digest this and review it, but it is definitely towards the top of our queue.

[titpetric]

@coreydaley Let me know if there's absolutely anything I can do to improve it.

I've considered adding a .SetRegexpCompile(func(...)...) as an API addition, but wen't with the 'less is more' approach. It would be possible to update this in runtime with some mutex protections. That sounds fine until you consider how many extra bugs can rise up when conccurency is at play.

Explicit control of the regex compile func behaviour is a single shot - if someone wants to control that during the router lifetime, just implementing the compile func signature enables it. It's expected there would be some sort of smarter eviction implementations behind it. Overthinking it just leads to more unnecessary problems, keeping things simple is hard sometimes.

[AlexVulaj]

@coreydaley This looks good to me, though I'll defer to you for a merge since I know you were looking at it first.

While I'm in favor of giving consumers the freedom to optimize their route creation, I believe such an action comes with the implicit disclaimer that if someone is going to play with around with it, they should know what they're doing and are on their own if it breaks something.

[AlexVulaj]

Another note - all of the changes from this PR seem to also be present in https://github.com/gorilla/mux/pull/732

[titpetric]

Another note - all of the changes from this PR seem to also be present in #732

Correct, regexp is it's own optimization scope (this PR). The second PR needs a rebase when this is merged, that addresses actual memory pressure, while this one just gives us an ability to use a cached/compiled *regexp.Regexp.

[titpetric]

• rebased
• marked testing.TB internal func param as _ (leave in place)