Skip to content

Commit

Permalink
Reduce allocations in per-request methods to improve performance (#395)
Browse files Browse the repository at this point in the history
* Prevent additional allocations when sorting routes for selection

Use of an uninitialized array and the use of sort.Reverse cause
extra allocations in request routing which are unnecessary.

* Reduce allocations in Accept and Content-Type matching

The mime handling methods can do a single pass over the header without
needing to allocate. This improves performance of these methods in
higher traffic API servers.

* Reduce allocations in route selection

The use of new arrays for each phase of detectRoute can be simplified:
each pass filters the existing array and whatever is left at the end
is returned. While somewhat harder to read, this method is a significant
source of allocations.
  • Loading branch information
smarterclayton authored and emicklei committed Mar 27, 2019
1 parent e64cccd commit 8673ba5
Show file tree
Hide file tree
Showing 4 changed files with 68 additions and 51 deletions.
4 changes: 2 additions & 2 deletions curly.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,14 +45,14 @@ func (c CurlyRouter) SelectRoute(

// selectRoutes return a collection of Route from a WebService that matches the path tokens from the request.
func (c CurlyRouter) selectRoutes(ws *WebService, requestTokens []string) sortableCurlyRoutes {
candidates := sortableCurlyRoutes{}
candidates := make(sortableCurlyRoutes, 0, 8)
for _, each := range ws.routes {
matches, paramCount, staticCount := c.matchesRouteByPathTokens(each.pathParts, requestTokens)
if matches {
candidates.add(curlyRoute{each, paramCount, staticCount}) // TODO make sure Routes() return pointers?
}
}
sort.Sort(sort.Reverse(candidates))
sort.Sort(candidates)
return candidates
}

Expand Down
16 changes: 9 additions & 7 deletions curly_route.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,15 @@ type curlyRoute struct {
staticCount int
}

// sortableCurlyRoutes orders by most parameters and path elements first.
type sortableCurlyRoutes []curlyRoute

func (s *sortableCurlyRoutes) add(route curlyRoute) {
*s = append(*s, route)
}

func (s sortableCurlyRoutes) routes() (routes []Route) {
routes = make([]Route, 0, len(s))
for _, each := range s {
routes = append(routes, each.route) // TODO change return type
}
Expand All @@ -31,22 +33,22 @@ func (s sortableCurlyRoutes) Swap(i, j int) {
s[i], s[j] = s[j], s[i]
}
func (s sortableCurlyRoutes) Less(i, j int) bool {
ci := s[i]
cj := s[j]
a := s[j]
b := s[i]

// primary key
if ci.staticCount < cj.staticCount {
if a.staticCount < b.staticCount {
return true
}
if ci.staticCount > cj.staticCount {
if a.staticCount > b.staticCount {
return false
}
// secundary key
if ci.paramCount < cj.paramCount {
if a.paramCount < b.paramCount {
return true
}
if ci.paramCount > cj.paramCount {
if a.paramCount > b.paramCount {
return false
}
return ci.route.Path < cj.route.Path
return a.route.Path < b.route.Path
}
43 changes: 23 additions & 20 deletions jsr311.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,8 @@ func (RouterJSR311) extractParams(pathExpr *pathExpression, matches []string) ma

// http://jsr311.java.net/nonav/releases/1.1/spec/spec3.html#x3-360003.7.2
func (r RouterJSR311) detectRoute(routes []Route, httpRequest *http.Request) (*Route, error) {
ifOk := []Route{}
for _, each := range routes {
candidates := make([]*Route, 0, 8)
for i, each := range routes {
ok := true
for _, fn := range each.If {
if !fn(httpRequest) {
Expand All @@ -76,64 +76,67 @@ func (r RouterJSR311) detectRoute(routes []Route, httpRequest *http.Request) (*R
}
}
if ok {
ifOk = append(ifOk, each)
candidates = append(candidates, &routes[i])
}
}
if len(ifOk) == 0 {
if len(candidates) == 0 {
if trace {
traceLogger.Printf("no Route found (from %d) that passes conditional checks", len(routes))
}
return nil, NewError(http.StatusNotFound, "404: Not Found")
}

// http method
methodOk := []Route{}
for _, each := range ifOk {
previous := candidates
candidates = candidates[:0]
for _, each := range previous {
if httpRequest.Method == each.Method {
methodOk = append(methodOk, each)
candidates = append(candidates, each)
}
}
if len(methodOk) == 0 {
if len(candidates) == 0 {
if trace {
traceLogger.Printf("no Route found (in %d routes) that matches HTTP method %s\n", len(routes), httpRequest.Method)
traceLogger.Printf("no Route found (in %d routes) that matches HTTP method %s\n", len(previous), httpRequest.Method)
}
return nil, NewError(http.StatusMethodNotAllowed, "405: Method Not Allowed")
}

// content-type
contentType := httpRequest.Header.Get(HEADER_ContentType)
inputMediaOk := []Route{}
for _, each := range methodOk {
previous = candidates
candidates = candidates[:0]
for _, each := range previous {
if each.matchesContentType(contentType) {
inputMediaOk = append(inputMediaOk, each)
candidates = append(candidates, each)
}
}
if len(inputMediaOk) == 0 {
if len(candidates) == 0 {
if trace {
traceLogger.Printf("no Route found (from %d) that matches HTTP Content-Type: %s\n", len(methodOk), contentType)
traceLogger.Printf("no Route found (from %d) that matches HTTP Content-Type: %s\n", len(previous), contentType)
}
return nil, NewError(http.StatusUnsupportedMediaType, "415: Unsupported Media Type")
}

// accept
outputMediaOk := []Route{}
previous = candidates
candidates = candidates[:0]
accept := httpRequest.Header.Get(HEADER_Accept)
if len(accept) == 0 {
accept = "*/*"
}
for _, each := range inputMediaOk {
for _, each := range previous {
if each.matchesAccept(accept) {
outputMediaOk = append(outputMediaOk, each)
candidates = append(candidates, each)
}
}
if len(outputMediaOk) == 0 {
if len(candidates) == 0 {
if trace {
traceLogger.Printf("no Route found (from %d) that matches HTTP Accept: %s\n", len(inputMediaOk), accept)
traceLogger.Printf("no Route found (from %d) that matches HTTP Accept: %s\n", len(previous), accept)
}
return nil, NewError(http.StatusNotAcceptable, "406: Not Acceptable")
}
// return r.bestMatchByMedia(outputMediaOk, contentType, accept), nil
return &outputMediaOk[0], nil
return candidates[0], nil
}

// http://jsr311.java.net/nonav/releases/1.1/spec/spec3.html#x3-360003.7.2
Expand Down
56 changes: 34 additions & 22 deletions route.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,28 +78,36 @@ func (r *Route) dispatchWithFilters(wrappedRequest *Request, wrappedResponse *Re
}
}

func stringTrimSpaceCutset(r rune) bool {
return r == ' '
}

// Return whether the mimeType matches to what this Route can produce.
func (r Route) matchesAccept(mimeTypesWithQuality string) bool {
parts := strings.Split(mimeTypesWithQuality, ",")
for _, each := range parts {
var withoutQuality string
if strings.Contains(each, ";") {
withoutQuality = strings.Split(each, ";")[0]
remaining := mimeTypesWithQuality
for {
var mimeType string
if end := strings.Index(remaining, ","); end == -1 {
mimeType, remaining = remaining, ""
} else {
withoutQuality = each
mimeType, remaining = remaining[:end], remaining[end+1:]
}
if quality := strings.Index(mimeType, ";"); quality != -1 {
mimeType = mimeType[:quality]
}
// trim before compare
withoutQuality = strings.Trim(withoutQuality, " ")
if withoutQuality == "*/*" {
mimeType = strings.TrimFunc(mimeType, stringTrimSpaceCutset)
if mimeType == "*/*" {
return true
}
for _, producibleType := range r.Produces {
if producibleType == "*/*" || producibleType == withoutQuality {
if producibleType == "*/*" || producibleType == mimeType {
return true
}
}
if len(remaining) == 0 {
return false
}
}
return false
}

// Return whether this Route can consume content with a type specified by mimeTypes (can be empty).
Expand All @@ -120,29 +128,33 @@ func (r Route) matchesContentType(mimeTypes string) bool {
mimeTypes = MIME_OCTET
}

parts := strings.Split(mimeTypes, ",")
for _, each := range parts {
var contentType string
if strings.Contains(each, ";") {
contentType = strings.Split(each, ";")[0]
remaining := mimeTypes
for {
var mimeType string
if end := strings.Index(remaining, ","); end == -1 {
mimeType, remaining = remaining, ""
} else {
contentType = each
mimeType, remaining = remaining[:end], remaining[end+1:]
}
if quality := strings.Index(mimeType, ";"); quality != -1 {
mimeType = mimeType[:quality]
}
// trim before compare
contentType = strings.Trim(contentType, " ")
mimeType = strings.TrimFunc(mimeType, stringTrimSpaceCutset)
for _, consumeableType := range r.Consumes {
if consumeableType == "*/*" || consumeableType == contentType {
if consumeableType == "*/*" || consumeableType == mimeType {
return true
}
}
if len(remaining) == 0 {
return false
}
}
return false
}

// Tokenize an URL path using the slash separator ; the result does not have empty tokens
func tokenizePath(path string) []string {
if "/" == path {
return []string{}
return nil
}
return strings.Split(strings.Trim(path, "/"), "/")
}
Expand Down

0 comments on commit 8673ba5

Please sign in to comment.