Skip to content

Commit

Permalink
Switch to using one tree per method instead of a map of per-method ha…
Browse files Browse the repository at this point in the history
…ndlers on each node

This makes pretty printing annoying, but increases performance!

With n trees instead of one tree, each tree is only split for handlers it actually has, so for HTTP verbs like POST or PUT that tend to have fewer routes, the trees are smaller and faster to traverse. For the HTTP GET tree, there are fewer nodes and I think better cache locality as that tree is traversed the most often. Each verb doesn't pay any traversal penalty for the other trees' size. This also results in more instances of more selective version stores, which means traversing them should be faster at the expense of a bit more memory consumption.

This also makes the constraint implementation (see delvedor#166) easier, and prevents bugs like delvedor#132, and avoids the extra checks we have to do to fix that bug.

This also prevents tree traversal for methods where there are no routes at all, which is a small optimization but kinda nice regardless.

For the pretty printing algorithm, I think a nice pretty print wouldn't be per method and would instead show all routes in the same list, so I added code to merge the separate node trees and then pretty print the merged tree! To make it look pretty I added some "compression" to the tree where branches that only had one branch get compressed down, which if you ask me results in some prettier output, see the tests.

Benchmarks:

```
kamloop ~/C/find-my-way (master) ➜  npm run bench; git checkout one-tree-per-method; npm run bench

> find-my-way@3.0.4 bench /Users/airhorns/Code/find-my-way
> node bench.js

lookup static route x 42,774,309 ops/sec ±0.84% (580 runs sampled)
lookup dynamic route x 3,536,084 ops/sec ±0.70% (587 runs sampled)
lookup dynamic multi-parametric route x 1,842,343 ops/sec ±0.92% (587 runs sampled)
lookup dynamic multi-parametric route with regex x 1,477,768 ops/sec ±0.57% (590 runs sampled)
lookup long static route x 3,350,884 ops/sec ±0.62% (589 runs sampled)
lookup long dynamic route x 2,491,556 ops/sec ±0.63% (585 runs sampled)
lookup static versioned route x 9,241,735 ops/sec ±0.44% (586 runs sampled)
find static route x 36,660,039 ops/sec ±0.76% (581 runs sampled)
find dynamic route x 4,473,753 ops/sec ±0.72% (588 runs sampled)
find dynamic multi-parametric route x 2,202,207 ops/sec ±1.00% (578 runs sampled)
find dynamic multi-parametric route with regex x 1,680,101 ops/sec ±0.76% (579 runs sampled)
find long static route x 4,633,069 ops/sec ±1.04% (588 runs sampled)
find long dynamic route x 3,333,916 ops/sec ±0.76% (586 runs sampled)
find static versioned route x 10,779,325 ops/sec ±0.73% (586 runs sampled)
find long nested dynamic route x 1,379,726 ops/sec ±0.45% (587 runs sampled)
find long nested dynamic route with other method x 1,962,454 ops/sec ±0.97% (587 runs sampled)

> find-my-way@3.0.4 bench /Users/airhorns/Code/find-my-way
> node bench.js

lookup static route x 41,200,005 ops/sec ±0.98% (591 runs sampled)
lookup dynamic route x 3,553,160 ops/sec ±0.28% (591 runs sampled)
lookup dynamic multi-parametric route x 2,047,064 ops/sec ±0.83% (584 runs sampled)
lookup dynamic multi-parametric route with regex x 1,500,267 ops/sec ±0.64% (590 runs sampled)
lookup long static route x 3,406,235 ops/sec ±0.77% (588 runs sampled)
lookup long dynamic route x 2,338,285 ops/sec ±1.60% (589 runs sampled)
lookup static versioned route x 9,239,314 ops/sec ±0.40% (586 runs sampled)
find static route x 35,230,842 ops/sec ±0.92% (578 runs sampled)
find dynamic route x 4,469,776 ops/sec ±0.33% (590 runs sampled)
find dynamic multi-parametric route x 2,237,214 ops/sec ±1.39% (585 runs sampled)
find dynamic multi-parametric route with regex x 1,533,243 ops/sec ±1.04% (581 runs sampled)
find long static route x 4,585,833 ops/sec ±0.51% (588 runs sampled)
find long dynamic route x 3,491,155 ops/sec ±0.45% (589 runs sampled)
find static versioned route x 10,801,810 ops/sec ±0.89% (580 runs sampled)
find long nested dynamic route x 1,418,610 ops/sec ±0.68% (588 runs sampled)
find long nested dynamic route with other method x 2,499,722 ops/sec ±0.38% (587 runs sampled)
```
  • Loading branch information
airhorns committed Oct 28, 2020
1 parent 57e7a6d commit 68d5725
Show file tree
Hide file tree
Showing 4 changed files with 180 additions and 130 deletions.
89 changes: 60 additions & 29 deletions index.js
Expand Up @@ -15,6 +15,7 @@ const assert = require('assert')
const http = require('http')
const fastDecode = require('fast-decode-uri-component')
const isRegexSafe = require('safe-regex2')
const { flattenNode, compressFlattenedNode, prettyPrintFlattenedNode } = require('./lib/pretty-print')
const Node = require('./node')
const NODE_TYPES = Node.prototype.types
const httpMethods = http.METHODS
Expand All @@ -26,6 +27,13 @@ if (!isRegexSafe(FULL_PATH_REGEXP)) {

const acceptVersionStrategy = require('./lib/accept-version')

// Object with property slots for all the HTTP methods with a flat Shape
function MethodMap () {
for (var i = 0; i < http.METHODS.length; i++) {
this[http.METHODS[i]] = null
}
}

function Router (opts) {
if (!(this instanceof Router)) {
return new Router(opts)
Expand All @@ -51,7 +59,7 @@ function Router (opts) {
this.maxParamLength = opts.maxParamLength || 100
this.allowUnsafeRegex = opts.allowUnsafeRegex || false
this.versioning = opts.versioning || acceptVersionStrategy
this.tree = new Node({ versions: this.versioning.storage() })
this.trees = new MethodMap()
this.routes = []
}

Expand Down Expand Up @@ -195,14 +203,19 @@ Router.prototype._on = function _on (method, path, opts, handler, store) {

Router.prototype._insert = function _insert (method, path, kind, params, handler, store, regex, version) {
const route = path
var currentNode = this.tree
var prefix = ''
var pathLen = 0
var prefixLen = 0
var len = 0
var max = 0
var node = null

var currentNode = this.trees[method]
if (currentNode === null) {
currentNode = new Node({ method: method, versions: this.versioning.storage() })
this.trees[method] = currentNode
}

while (true) {
prefix = currentNode.prefix
prefixLen = prefix.length
Expand All @@ -218,10 +231,11 @@ Router.prototype._insert = function _insert (method, path, kind, params, handler
if (len < prefixLen) {
node = new Node(
{
method: method,
prefix: prefix.slice(len),
children: currentNode.children,
kind: currentNode.kind,
handlers: new Node.Handlers(currentNode.handlers),
handler: currentNode.handler,
regex: currentNode.regex,
versions: currentNode.versions
}
Expand All @@ -239,25 +253,26 @@ Router.prototype._insert = function _insert (method, path, kind, params, handler
// the handler should be added to the current node, to a child otherwise
if (len === pathLen) {
if (version) {
assert(!currentNode.getVersionHandler(version, method), `Method '${method}' already declared for route '${route}' version '${version}'`)
currentNode.setVersionHandler(version, method, handler, params, store)
assert(!currentNode.getVersionHandler(version), `Method '${method}' already declared for route '${route}' version '${version}'`)
currentNode.setVersionHandler(version, handler, params, store)
} else {
assert(!currentNode.getHandler(method), `Method '${method}' already declared for route '${route}'`)
currentNode.setHandler(method, handler, params, store)
assert(!currentNode.handler, `Method '${method}' already declared for route '${route}'`)
currentNode.setHandler(handler, params, store)
}
currentNode.kind = kind
} else {
node = new Node({
method: method,
prefix: path.slice(len),
kind: kind,
handlers: null,
regex: regex,
versions: this.versioning.storage()
})
if (version) {
node.setVersionHandler(version, method, handler, params, store)
node.setVersionHandler(version, handler, params, store)
} else {
node.setHandler(method, handler, params, store)
node.setHandler(handler, params, store)
}
currentNode.addChild(node)
}
Expand All @@ -275,31 +290,31 @@ Router.prototype._insert = function _insert (method, path, kind, params, handler
continue
}
// there are not children within the given label, let's create a new one!
node = new Node({ prefix: path, kind: kind, handlers: null, regex: regex, versions: this.versioning.storage() })
node = new Node({ method: method, prefix: path, kind: kind, regex: regex, versions: this.versioning.storage() })
if (version) {
node.setVersionHandler(version, method, handler, params, store)
node.setVersionHandler(version, handler, params, store)
} else {
node.setHandler(method, handler, params, store)
node.setHandler(handler, params, store)
}

currentNode.addChild(node)

// the node already exist
} else if (handler) {
if (version) {
assert(!currentNode.getVersionHandler(version, method), `Method '${method}' already declared for route '${route}' version '${version}'`)
currentNode.setVersionHandler(version, method, handler, params, store)
assert(!currentNode.getVersionHandler(version), `Method '${method}' already declared for route '${route}' version '${version}'`)
currentNode.setVersionHandler(version, handler, params, store)
} else {
assert(!currentNode.getHandler(method), `Method '${method}' already declared for route '${route}'`)
currentNode.setHandler(method, handler, params, store)
assert(!currentNode.handler, `Method '${method}' already declared for route '${route}'`)
currentNode.setHandler(handler, params, store)
}
}
return
}
}

Router.prototype.reset = function reset () {
this.tree = new Node({ versions: this.versioning.storage() })
this.trees = new MethodMap()
this.routes = []
}

Expand Down Expand Up @@ -358,6 +373,9 @@ Router.prototype.lookup = function lookup (req, res, ctx) {
}

Router.prototype.find = function find (method, path, version) {
var currentNode = this.trees[method]
if (!currentNode) return null

if (path.charCodeAt(0) !== 47) { // 47 is '/'
path = path.replace(FULL_PATH_REGEXP, '/')
}
Expand All @@ -370,7 +388,6 @@ Router.prototype.find = function find (method, path, version) {
}

var maxParamLength = this.maxParamLength
var currentNode = this.tree
var wildcardNode = null
var pathLenWildcard = 0
var decoded = null
Expand All @@ -388,8 +405,8 @@ Router.prototype.find = function find (method, path, version) {
// found the route
if (pathLen === 0 || path === prefix) {
var handle = version === undefined
? currentNode.handlers[method]
: currentNode.getVersionHandler(version, method)
? currentNode.handler
: currentNode.getVersionHandler(version)
if (handle !== null && handle !== undefined) {
var paramsObj = {}
if (handle.paramsLength > 0) {
Expand Down Expand Up @@ -419,13 +436,13 @@ Router.prototype.find = function find (method, path, version) {
}

var node = version === undefined
? currentNode.findChild(path, method)
: currentNode.findVersionChild(version, path, method)
? currentNode.findChild(path)
: currentNode.findVersionChild(version, path)

if (node === null) {
node = currentNode.parametricBrother
if (node === null) {
return this._getWildcardNode(wildcardNode, method, originalPath, pathLenWildcard)
return this._getWildcardNode(wildcardNode, originalPath, pathLenWildcard)
}

var goBack = previousPath.charCodeAt(0) === 47 ? previousPath : '/' + previousPath
Expand All @@ -448,7 +465,7 @@ Router.prototype.find = function find (method, path, version) {
// static route
if (kind === NODE_TYPES.STATIC) {
// if exist, save the wildcard child
if (currentNode.wildcardChild !== null && currentNode.wildcardChild.handlers[method] !== null) {
if (currentNode.wildcardChild !== null) {
wildcardNode = currentNode.wildcardChild
pathLenWildcard = pathLen
}
Expand All @@ -457,11 +474,11 @@ Router.prototype.find = function find (method, path, version) {
}

if (len !== prefixLen) {
return this._getWildcardNode(wildcardNode, method, originalPath, pathLenWildcard)
return this._getWildcardNode(wildcardNode, originalPath, pathLenWildcard)
}

// if exist, save the wildcard child
if (currentNode.wildcardChild !== null && currentNode.wildcardChild.handlers[method] !== null) {
if (currentNode.wildcardChild !== null) {
wildcardNode = currentNode.wildcardChild
pathLenWildcard = pathLen
}
Expand Down Expand Up @@ -545,15 +562,15 @@ Router.prototype.find = function find (method, path, version) {
}
}

Router.prototype._getWildcardNode = function (node, method, path, len) {
Router.prototype._getWildcardNode = function (node, path, len) {
if (node === null) return null
var decoded = fastDecode(path.slice(-len))
if (decoded === null) {
return this.onBadUrl !== null
? this._onBadUrl(path.slice(-len))
: null
}
var handle = node.handlers[method]
var handle = node.handler
if (handle !== null && handle !== undefined) {
return {
handler: handle.handler,
Expand Down Expand Up @@ -585,7 +602,21 @@ Router.prototype._onBadUrl = function (path) {
}

Router.prototype.prettyPrint = function () {
return this.tree.prettyPrint('', true)
const root = {
prefix: '/',
nodes: [],
children: {}
}

for (const node of Object.values(this.trees)) {
if (node) {
flattenNode(root, node)
}
}

compressFlattenedNode(root)

return prettyPrintFlattenedNode(root, '', true)
}

for (var i in http.METHODS) {
Expand Down
83 changes: 83 additions & 0 deletions lib/pretty-print.js
@@ -0,0 +1,83 @@
function prettyPrintFlattenedNode (flattenedNode, prefix, tail) {
var paramName = ''
var methods = new Set(flattenedNode.nodes.map(node => node.method))

if (flattenedNode.prefix.includes(':')) {
flattenedNode.nodes.forEach((node, index) => {
var params = node.handler.params
var param = params[params.length - 1]
if (methods.size > 1) {
if (index === 0) {
paramName += param + ` (${node.method})\n`
return
}
paramName += prefix + ' :' + param + ` (${node.method})`
paramName += (index === methods.size - 1 ? '' : '\n')
} else {
paramName = params[params.length - 1] + ` (${node.method})`
}
})
} else if (methods.size) {
paramName = ` (${Array.from(methods).join('|')})`
}

var tree = `${prefix}${tail ? '└── ' : '├── '}${flattenedNode.prefix}${paramName}\n`

prefix = `${prefix}${tail ? ' ' : '│ '}`
const labels = Object.keys(flattenedNode.children)
for (var i = 0; i < labels.length; i++) {
const child = flattenedNode.children[labels[i]]
tree += prettyPrintFlattenedNode(child, prefix, i === (labels.length - 1))
}
return tree
}

function flattenNode (flattened, node) {
if (node.handler) {
flattened.nodes.push(node)
}

if (node.children) {
for (const child of Object.values(node.children)) {
const childPrefixSegments = child.prefix.split(/(?=\/)/) // split on the slash separator but use a regex to lookahead and not actually match it, preserving it in the returned string segments
let cursor = flattened
let parent
for (const segment of childPrefixSegments) {
parent = cursor
cursor = cursor.children[segment]
if (!cursor) {
cursor = {
prefix: segment,
nodes: [],
children: {}
}
parent.children[segment] = cursor
}
}

flattenNode(cursor, child)
}
}
}

function compressFlattenedNode (flattenedNode) {
const childKeys = Object.keys(flattenedNode.children)
if (flattenedNode.nodes.length === 0 && childKeys.length === 1) {
const child = flattenedNode.children[childKeys[0]]
if (child.nodes.length <= 1) {
compressFlattenedNode(child)
flattenedNode.nodes = child.nodes
flattenedNode.prefix += child.prefix
flattenedNode.children = child.children
return flattenedNode
}
}

for (const key of Object.keys(flattenedNode.children)) {
compressFlattenedNode(flattenedNode.children[key])
}

return flattenedNode
}

module.exports = { flattenNode, compressFlattenedNode, prettyPrintFlattenedNode }

0 comments on commit 68d5725

Please sign in to comment.