Skip to content

Commit

Permalink
v3: fix benchmark results related to handler, next (#2130)
Browse files Browse the repository at this point in the history
  • Loading branch information
efectn authored Oct 10, 2022
1 parent 7fb50f1 commit 23b8170
Show file tree
Hide file tree
Showing 3 changed files with 70 additions and 13 deletions.
14 changes: 12 additions & 2 deletions ctx.go
Original file line number Diff line number Diff line change
Expand Up @@ -728,16 +728,26 @@ func (c *DefaultCtx) Next() (err error) {
err = c.route.Handlers[c.indexHandler](c)
} else {
// Continue handler stack
_, err = c.app.next(c, c.app.newCtxFunc != nil)
if c.app.newCtxFunc != nil {
_, err = c.app.nextCustom(c)
} else {
_, err = c.app.next(c)
}
}
return err
}

// RestartRouting instead of going to the next handler. This may be usefull after
// changing the request path. Note that handlers might be executed again.
func (c *DefaultCtx) RestartRouting() error {
var err error

c.indexRoute = -1
_, err := c.app.next(c, c.app.newCtxFunc != nil)
if c.app.newCtxFunc != nil {
_, err = c.app.nextCustom(c)
} else {
_, err = c.app.next(c)
}
return err
}

Expand Down
63 changes: 55 additions & 8 deletions router.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ func (r *Route) match(detectionPath, path string, params *[maxParams]string) (ma
return false
}

func (app *App) next(c CustomCtx, customCtx bool) (match bool, err error) {
func (app *App) nextCustom(c CustomCtx) (match bool, err error) {
// Get stack length
tree, ok := app.treeStack[c.getMethodINT()][c.getTreePath()]
if !ok {
Expand Down Expand Up @@ -136,22 +136,64 @@ func (app *App) next(c CustomCtx, customCtx bool) (match bool, err error) {
// If c.Next() does not match, return 404
err = NewError(StatusNotFound, "Cannot "+c.Method()+" "+c.getPathOriginal())

var isMethodExist bool
if customCtx {
isMethodExist = methodExistCustom(c)
} else {
isMethodExist = methodExist(c.(*DefaultCtx))
// If no match, scan stack again if other methods match the request
// Moved from app.handler because middleware may break the route chain
if !c.getMatched() && methodExistCustom(c) {
err = ErrMethodNotAllowed
}
return
}

func (app *App) next(c *DefaultCtx) (match bool, err error) {
// Get stack length
tree, ok := app.treeStack[c.methodINT][c.treePath]
if !ok {
tree = app.treeStack[c.methodINT][""]
}
lenr := len(tree) - 1

// Loop over the route stack starting from previous index
for c.indexRoute < lenr {
// Increment route index
c.indexRoute++

// Get *Route
route := tree[c.indexRoute]

// Check if it matches the request path
match = route.match(c.detectionPath, c.path, &c.values)

// No match, next route
if !match {
continue
}
// Pass route reference and param values
c.route = route

// Non use handler matched
if !c.matched && !route.use {
c.matched = true
}

// Execute first handler of route
c.indexHandler = 0
err = route.Handlers[0](c)
return match, err // Stop scanning the stack
}

// If c.Next() does not match, return 404
err = NewError(StatusNotFound, "Cannot "+c.method+" "+c.pathOriginal)

// If no match, scan stack again if other methods match the request
// Moved from app.handler because middleware may break the route chain
if !c.getMatched() && isMethodExist {
if !c.matched && methodExist(c) {
err = ErrMethodNotAllowed
}
return
}

func (app *App) handler(rctx *fasthttp.RequestCtx) {
// Handler for default ctxs
var c CustomCtx
if app.newCtxFunc != nil {
c = app.AcquireCtx().(CustomCtx)
Expand All @@ -173,7 +215,12 @@ func (app *App) handler(rctx *fasthttp.RequestCtx) {
}

// Find match in stack
_, err := app.next(c, app.newCtxFunc != nil)
var err error
if app.newCtxFunc != nil {
_, err = app.nextCustom(c)
} else {
_, err = app.next(c.(*DefaultCtx))
}
if err != nil {
if catch := c.App().ErrorHandler(c, err); catch != nil {
_ = c.SendStatus(StatusInternalServerError)
Expand Down
6 changes: 3 additions & 3 deletions router_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -596,7 +596,7 @@ func Benchmark_Router_Next(b *testing.B) {
b.ResetTimer()
for n := 0; n < b.N; n++ {
c.indexRoute = -1
res, err = app.next(c, false)
res, err = app.next(c)
}
require.NoError(b, err)
require.True(b, res)
Expand Down Expand Up @@ -772,10 +772,10 @@ func Benchmark_Router_Github_API(b *testing.B) {
for n := 0; n < b.N; n++ {
c.URI().SetPath(routesFixture.TestRoutes[i].Path)

ctx := app.AcquireCtx().(CustomCtx)
ctx := app.AcquireCtx().(*DefaultCtx)
ctx.Reset(c)

match, err = app.next(ctx, false)
match, err = app.next(ctx)
app.ReleaseCtx(ctx)
}

Expand Down

1 comment on commit 23b8170

@ReneWerner87
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.

Benchmark suite Current: 23b8170 Previous: 925d5d0 Ratio
Benchmark_AcquireCtx 1325 ns/op 1568 B/op 5 allocs/op 661.9 ns/op 1568 B/op 5 allocs/op 2.00
Benchmark_Cache 13791 ns/op 49369 B/op 6 allocs/op 318.6 ns/op 16 B/op 2 allocs/op 43.29
Benchmark_Cache_AdditionalHeaders 1188 ns/op 592 B/op 9 allocs/op 416.9 ns/op 16 B/op 2 allocs/op 2.85
Benchmark_Limiter 619.7 ns/op 72 B/op 2 allocs/op 308 ns/op 8 B/op 1 allocs/op 2.01

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.