Request based constraints implementation

[AyoubElk]

This PR is to add support for request based constraints, following the discussions in issues fastify#2498 and find-my-way#165

Please note that this is still a WIP and tests are failing, but I'd like to start a discussion and get some feedback and direction.

01. API updates:

a. Constructor:

Instead of opts.versioning that allows specifying the versioning strategy, the constructor now takes opts.constrainingStrategies that allows the user to override default strategies with custom ones.

The strategy object maintains the same structure as previously, with a single change of deriveVersion to deriveConstraint to be more generic.

The default strategies supported are version and host.

Right now host only supports exact match

b. Route registration (.on):

When registering a route, the user can provide a constraints property in the opts parameter, this property is a flat object containing key-values constraints:

{
	version: '1.0.0',
	host: 'auth.airhorns.dev'
}

c. Routing (.find):

When finding a route, the find method now expects a function constraintsExtractor instead of a constraints object.

In the current codebase the use of versioned routes storage is only done if deriveVersion returns a non empty value.

This works well for the version constraint, which has to be explicitly set in the accept-version header. For other constraints like the header host it will always contain a value, which would imply the constrained storage will always be used.

In order to avoid that, the new solution that would support all use cases needs to derive the constraints for each tree node we check, since each node knows which constraints it supports.

The constraintsExtractor method expects a list of constraint keys that should be extracted from the request.

02. Internals:

Internally, these are some of the main changes that were made:

a. accept-constraints:

Function that takes an optional strategies object, allowing the user to override/extend the existing strategies. Returns an object that allows creating instances of the ConstraintsStore bound to the available strategies via the storage property, and getConstraintsExtractor function that returns a constraintsExtractor function with access to the request object.

b. constraints-store:

An instance of the ConstraintsStore is created for each node, it takes a list of strategies that it should use, where each strategy has its own storage:

ConstraintsStore exposes 2 methods:

• set(constraints, store): Given a constraints object, we loop through its properties and call the appropriate strategy's set() method. We store the constraints object as well, which we'll need in the .get() method:

  https://github.com/AyoubElk/find-my-way/blob/edd00c98f5eaaad5fd6f09b7ac645e3d2ab8d9bf/lib/constraints-store.js#L19-L22

• get(constraints, method): Given a constraints object, calling the get method will result in a call to the get method of all the supported strategies. Since the strategies store handlers independently, if one of them returns null then we can know that no match that fulfills all constraints is found and we return null, otherwise we return the stored handlers.

  https://github.com/AyoubElk/find-my-way/blob/edd00c98f5eaaad5fd6f09b7ac645e3d2ab8d9bf/lib/constraints-store.js#L38

  We also compare the given constraints object with the one we stored previously, but there seems to be an edge case here that I haven't considered before writing this.

  https://github.com/AyoubElk/find-my-way/blob/edd00c98f5eaaad5fd6f09b7ac645e3d2ab8d9bf/lib/constraints-store.js#L39-L40

  Consider these routes:

  router.on('GET', '/users', { constraints: { version: '1.0.0' } }, handler)
  router.on('GET', '/users', { constraints: { host: 'auth.airhorns.dev' } }, handler)

  Should this scenario be possible? If so, and given a request that can match both, which one should be matched

c. node:

The main changes here are two new methods and one new property:

• kConstraints: A set that contains the constraint keys applicable to this node. But given the edge case mentioned above, it would be better to have an extra property that stores an array of constraints and try to match starting from the most to the least restricted routes. What do you think?
• getMatchingHandler(constraintsExtractor, method): Extracts the applicable constraints from the req object if any, and calls getConstraintsHandler(), if no matching handler is found it calls getHandler() as well in case we have a non-constrained route available.
• findMatchingChild(constraintsExtractor, path, method): replaces both findChild() and findVersionChild()

Looking forward to hearing your thoughts!

[mcollina]

This is missing tests, so it's a bit hard to know how this would be used. I've left a few comments.

Does this worsen the benchmarks? I think it does from looking at code, but you should be able to optimize it.

cc @delvedor

[mcollina]

What do you mean?

[AyoubElk]

I meant should the validation code check the properties inside opts.constraints and make sure there are strategies to handle each one of them?

[mcollina]

Yes please, thanks.

[mcollina]

Why is this from console?

[AyoubElk]

Good catch, it was auto-imported by mistake.

[mcollina]

Yes, thanks

[mcollina]

Please use a for(;;) loop

[mcollina]

Not only it suffer from the problem you described in the comment, it is also highly inefficient. What is the purpose of this check? Maybe we can do without?

[AyoubElk]

To illustrate the purpose of that check, let's take these routes as an example:

router.on('GET', '/users', { constraints: { version: '2.0.0', host: 'www.airhorns.dev' } }, handler) // Route 1
router.on('GET', '/users', { constraints: { version: '1.*.0', host: 'auth.airhorns.dev' } }, handler)   // Route 2

Using the constraints extracted from a request: { version: '1.0.0', host: 'www.airhorns.com' }, when we call the get method of the ConstraintsStore, each constraint is checked separately using its defined strategy, so in this case the version constraint will match route 2, while the host constraint will match route 1. Adding this comparison allows us to avoid this edge case.

I agree that it isn't the most efficient solution, in fact now that I think of it it's wrong because the comparison will fail in non-exact match cases (in this example the version constraint has a wildcard, while the request has the exact version).

I'll try to come up with an alternative.

[airhorns]

Been talking with @AyoubElk off platform, want to post some answers here so all can see:

Considering these two routes

router.on('GET', '/users', { constraints: { version: '1.0.0' } }, handler)
router.on('GET', '/users', { constraints: { host: 'auth.airhorns.dev' } }, handler2)

Should this scenario be possible? If so, and given a request that can match both, which one should be matched?

For a request that could match both of the constraint sets, I think we should match what fastify does now for route matching precedence, which is just use whichever route is matched first. I forget if it's the first defined route or the last defined route, but, there is an ordering in which the routes were all required and registered and I assume some existing semantics for in what order the routes are tested. If those semantics don't exist already and it's undefined behaviour, then I think we should actually match that too in order to not make a bigger breaking change. I hope it doesn't depend on object key insertion order, though that is stable in node now.

In a similar vein, if the user can declare multiple routes with the same method/path but different constraints for each, I'd assume we'll match the most restrictive route:
In this case, if route 1 (has 1 constraint) and route 3 (has 2 constraints) match the request, we'll use the 3rd:

router.on('GET', '/users', { constraints: { version: '1.0.0' } }, handler)
router.on('GET', '/users', { constraints: { host: 'admin.airhorns.dev' } }, handler2)
router.on('GET', '/users', { constraints: { host: 'auth.airhorns.dev', version: '1.0.0' } }, handler3)

In the previous example however, we have 1 constraint for each one of the routes, so we can't base the matching on the number of constraints. Maybe the order of the routes should then be used and go with the last defined route?

I'd say the same thing: match the first route checked that passes the constraint checks, and then hopefully there's already a well defined order that routes are checked in. I think any other algorithm around most restrictive is going to be unpredictable and therefore surprising because restrictive might mean different things to different people. I think as a developer if I defined two routes in a file, and then added a constraint to the second route, which caused it to start getting checked first and matched first, I'd be surprised.

FWIW this ordered route matching how the Rails router works, I am a one trick pony when it comes to this kind of thing.

[AyoubElk]

Thanks @airhorns for the detailed explanation.

I think we should match what fastify does now for route matching precedence, which is just use whichever route is matched first. I forget if it's the first defined route or the last defined route, but, there is an ordering in which the routes were all required and registered and I assume some existing semantics for in what order the routes are tested.
If those semantics don't exist already and it's undefined behaviour, then I think we should actually match that too in order to not make a bigger breaking change. I hope it doesn't depend on object key insertion order, though that is stable in node now.

Right now, find-my-way matches the routes following this order, which is based solely on the route path.

This allows it to deterministically figure out which node to match, but not which handler inside that node should be returned in case different ones are available.

I'd say the same thing: match the first route checked that passes the constraint checks, and then hopefully there's already a well defined order that routes are checked in. I think any other algorithm around most restrictive is going to be unpredictable and therefore surprising because restrictive might mean different things to different people. I think as a developer if I defined two routes in a file, and then added a constraint to the second route, which caused it to start getting checked first and matched first, I'd be surprised.
FWIW this ordered route matching how the Rails router works, I am a one trick pony when it comes to this kind of thing.

In the example I provided, we have the same method and path, which will always be matched to the same node. At the node-level, a few things happen:

• During route registration: we take note of which constraints exist in the property: kConstraints (This is necessary because constraints like host will always be present in the request, so we need a way to figure out which constraints to use for each node)
• During routing: we derive constraints from the request and then for each node we only pick the constraints that are relevant to it.

Thanks to this, matching the first handler that passes the constraints would be simple to implement and deterministic, because the order at which the routes are registered is the same order of the constraints available in the kConstraints property.

If no other solution is suggested, I'll implement it this way.

[AyoubElk]

Updates

I've made some updates during this weekend to fix some issues I encountered and make some improvements based on benchmark results.

This is an overview and explaination of some of the main changes:

1. Replace getConstraintsExtractor with deriveConstraints

Previously, lookup called getConstraintsExtractror(req, ctx) and passed the resulting function constraintsExtractor(kConstraints) to the find method.

This is necessary because constraints like host will always be present in the request, and we need a way to allow each node to extract the constraints relevant to it.

However, this also meant that we had to derive the constraints from the request for each node that needs to do so, which adds unnecessary overhead.

Right now, deriveConstraints is called only once, and an object with all the derived constraints is passed down to nodes which can then access only the properties they need from it.

2. Update constraintsStore to centralize store storage in a shared map

This was done mainly to solve the issue described in this comment. We needed a way to make sure the different constraints actually stored the same handler.

Instead of stringifying and comparing the constraints object (which is an inefficient operation), I added a shared map property where each store is mapped to a unique id, and that id is passed down to the store of each strategy used.

We can now simply compare those ids.

3. Change strategy object format

When running the benchmark, I tracked down the main performance degradation down to the deriveConstraints function, which dropped performance by half by simply looping through stategies and calling deriveConstraint for each one of them.

deriveConstraints: function (req, ctx) {
      const derivedConstraints = {}
      let value, hasConstraint = false
      for (var i = 0; i < strategies.length; i++) {
        value = strategies[i].deriveConstraint(req, ctx)
        if (value) {
          hasConstraint = true
          derivedConstraints[strategies[i].name] = value
        }
      }

      return hasConstraint ? derivedConstraints : null
    }

Interestingly, the issue happened only when accessing the strategies object/array (comparison n°3 below) using the loop variable, and disappeared when using a literal value. The issue also disappeared when keeping access using the loop variable and removing the call to the deriveConstraint function.

So it seemed to me that the combination fo both dynamic property access and function call somehow caused the V8 engine to do some heavy operations that degraded performance.

I never ran into anything like this before, and given some reading I've done it seems that the main reason is Javascript hidden classes, which are created and updated as the object changes to allow the V8 engine to avoid dynamic property lookups, perform optimizations and inline caching.

Eventually, I figured out that providing the properties at the prototype level would allow V8 to know the object structure in advance. This worked and brought performance back up considerably (comparison n°4 below).

a. Using prototype

function acceptVersion() { }
acceptVersion.prototype.name = 'version'
acceptVersion.prototype.storage = SemVerStore
acceptVersion.prototype.deriveConstraint = function (req, ctx) {
  return req.headers['version']
}

b. Using a normal object:

{
  name: 'version',
  storage: SemVerStore,
  deriveConstraint: function (req, ctx) {
    return req.headers['accept-version']
  }
}

This works well, but would mean the user would have to provide any custom strategy using the new format (which is not as straight-forward as the first)

I wasn't able to find any other solution that maintains performance levels, but if there is some other way you're aware of to implement/optimize this and keep the previous strategy format, please let me know.

Benchmarks comparison

On my MacBook Pro Retina Mid 2014 (i7, 16GB of RAM), these are the results of running the benchmarks:

1. Master branch

❯ node bench.js   
lookup static route x 39,864,317 ops/sec ±0.54% (586 runs sampled)
lookup dynamic route x 3,055,819 ops/sec ±0.39% (588 runs sampled)
lookup dynamic multi-parametric route x 1,584,679 ops/sec ±0.64% (583 runs sampled)
lookup dynamic multi-parametric route with regex x 1,158,698 ops/sec ±0.96% (584 runs sampled)
lookup long static route x 2,570,186 ops/sec ±0.41% (582 runs sampled)
lookup long dynamic route x 2,024,055 ops/sec ±0.33% (590 runs sampled)
lookup static versioned route x 7,154,573 ops/sec ±0.41% (585 runs sampled)
find static route x 29,247,517 ops/sec ±0.60% (573 runs sampled)
find dynamic route x 3,472,771 ops/sec ±0.41% (586 runs sampled)
find dynamic multi-parametric route x 1,874,547 ops/sec ±0.70% (590 runs sampled)
find dynamic multi-parametric route with regex x 1,407,869 ops/sec ±0.29% (590 runs sampled)
find long static route x 3,787,256 ops/sec ±0.24% (589 runs sampled)
find long dynamic route x 2,759,278 ops/sec ±0.27% (590 runs sampled)
find static versioned route x 8,826,557 ops/sec ±0.39% (590 runs sampled)

2. Request-based-constraints branch

❯ node bench.js
Routes registered successfully...
lookup static route x 34,997,182 ops/sec ±0.37% (587 runs sampled)
lookup dynamic route x 2,862,363 ops/sec ±0.56% (585 runs sampled)
lookup dynamic multi-parametric route x 1,576,788 ops/sec ±0.25% (589 runs sampled)
lookup dynamic multi-parametric route with regex x 1,196,899 ops/sec ±0.31% (589 runs sampled)
lookup long static route x 2,475,820 ops/sec ±0.41% (586 runs sampled)
lookup long dynamic route x 1,906,568 ops/sec ±0.29% (589 runs sampled)
lookup static versioned route x 16,925,526 ops/sec ±0.46% (579 runs sampled)
lookup static constrained (version & host) route x 8,802,996 ops/sec ±0.55% (587 runs sampled)
find static route x 28,607,663 ops/sec ±0.59% (579 runs sampled)
find dynamic route x 3,449,980 ops/sec ±0.31% (588 runs sampled)
find dynamic multi-parametric route x 1,912,809 ops/sec ±0.37% (590 runs sampled)
find dynamic multi-parametric route with regex x 1,339,933 ops/sec ±0.78% (588 runs sampled)
find long static route x 3,428,143 ops/sec ±0.31% (587 runs sampled)
find long dynamic route x 2,635,874 ops/sec ±0.31% (584 runs sampled)
find static versioned route x 4,678,909 ops/sec ±0.32% (588 runs sampled)
find static constrained (version & host) route x 4,003,335 ops/sec ±0.37% (588 runs sampled)

3. Strategies object/array

Code for this can be found in the branch benchmark-strategy-object-vs-array, where I used a strategies object in acceptConstraints instead of an array.

You can notice a significant performance drop in the lines in bold (compared to request-based-constraints branch):

❯ node bench.js
Routes registered successfully...
**lookup static route x 18,470,823 ops/sec ±0.58% (586 runs sampled)**
lookup dynamic route x 2,544,969 ops/sec ±0.86% (585 runs sampled)
lookup dynamic multi-parametric route x 1,487,122 ops/sec ±0.49% (588 runs sampled)
lookup dynamic multi-parametric route with regex x 1,154,611 ops/sec ±0.32% (590 runs sampled)
lookup long static route x 2,320,796 ops/sec ±0.33% (587 runs sampled)
lookup long dynamic route x 1,834,193 ops/sec ±0.53% (582 runs sampled)
**lookup static versioned route x 11,161,286 ops/sec ±0.42% (583 runs sampled)
lookup static constrained (version & host) route x 6,775,507 ops/sec ±0.36% (584 runs sampled)**
find static route x 28,863,575 ops/sec ±0.62% (578 runs sampled)
find dynamic route x 3,445,673 ops/sec ±0.47% (587 runs sampled)
find dynamic multi-parametric route x 1,894,014 ops/sec ±0.35% (588 runs sampled)
find dynamic multi-parametric route with regex x 1,353,584 ops/sec ±0.26% (589 runs sampled)
find long static route x 3,525,118 ops/sec ±0.29% (588 runs sampled)
find long dynamic route x 2,609,847 ops/sec ±1.00% (588 runs sampled)
find static versioned route x 4,626,360 ops/sec ±0.42% (586 runs sampled)
find static constrained (version & host) route x 3,760,848 ops/sec ±0.50% (585 runs sampled)

4. Strategy format object vs prototype

Code for this can be found in the branch benchmark-strategy-object-vs-prototype, where I used defined strategies as simple objects instead of using prototype.

You can notice a significant performance drop in the lines in bold (compared to request-based-constraints branch):

❯ node bench.js
Routes registered successfully...
**lookup static route x 19,416,833 ops/sec ±0.30% (589 runs sampled)**
lookup dynamic route x 2,739,224 ops/sec ±0.32% (587 runs sampled)
lookup dynamic multi-parametric route x 1,547,496 ops/sec ±0.42% (590 runs sampled)
lookup dynamic multi-parametric route with regex x 1,195,527 ops/sec ±0.24% (590 runs sampled)
lookup long static route x 2,433,980 ops/sec ±0.17% (590 runs sampled)
lookup long dynamic route x 1,885,178 ops/sec ±0.30% (588 runs sampled)
**lookup static versioned route x 3,975,875 ops/sec ±0.37% (586 runs sampled)
lookup static constrained (version & host) route x 2,537,698 ops/sec ±0.42% (590 runs sampled)**
find static route x 30,144,666 ops/sec ±0.62% (576 runs sampled)
find dynamic route x 3,513,484 ops/sec ±0.35% (584 runs sampled)
find dynamic multi-parametric route x 1,835,354 ops/sec ±0.62% (584 runs sampled)
find dynamic multi-parametric route with regex x 1,304,781 ops/sec ±0.43% (591 runs sampled)
find long static route x 3,536,402 ops/sec ±0.22% (591 runs sampled)
find long dynamic route x 2,697,523 ops/sec ±0.17% (590 runs sampled)
find static versioned route x 4,922,200 ops/sec ±0.19% (590 runs sampled)
find static constrained (version & host) route x 4,030,548 ops/sec ±0.39% (588 runs sampled)

Next steps

This is still a WIP, I'd appreciate any feedback, especially regarding the issue described in the last comment by @airhorns.

Also, it seems that the benchmark returns inconsistent results, depending on which tests we run. I'm not sure if this is the normal behaviour, but I expected that test executions should be independent from each other.

Below are some example outputs when I comment out tests in bench.js for request-based-constraints branch:

❯ node bench.js
Routes registered successfully...
lookup static versioned route x 36,250,898 ops/sec ±0.28% (591 runs sampled)
find static versioned route x 7,019,433 ops/sec ±0.44% (588 runs sampled)

❯ node bench.js
Routes registered successfully...
lookup static versioned route x 34,855,606 ops/sec ±0.48% (585 runs sampled)
find static versioned route x 6,734,253 ops/sec ±1.31% (578 runs sampled)

❯ node bench.js
Routes registered successfully...
lookup static route x 33,630,554 ops/sec ±0.77% (585 runs sampled)
find static route x 50,909,812 ops/sec ±1.26% (564 runs sampled)

❯ node bench.js
Routes registered successfully...
lookup static route x 36,130,011 ops/sec ±0.38% (590 runs sampled)

❯ node bench.js
Routes registered successfully...
find static route x 49,153,773 ops/sec ±2.16% (589 runs sampled)

❯ node bench.js 
Routes registered successfully...
lookup static constrained (version & host) route x 16,030,286 ops/sec ±0.36% (590 runs sampled)

[mcollina]

I know the benchmarks could be quite flaky and they'll need some work. Howwever, the performance regression is there. I don't understand if your other branch fixes it or not.

[AyoubElk]

I know the benchmarks could be quite flaky and they'll need some work.

Yeah, and interestingly lookup seems to outperform find calls, which seems counter-intuitive since lookup derives constraints and calls find

Howwever, the performance regression is there. I don't understand if your other branch fixes it or not.

@mcollina In the benchmark section, you can compare the master branch benchmarks with those of request-based-constraint branch.

This table should make it easier to compare both (the bolded values are those where request-based-constraints branch outperformed the master branch):

Benchmark	master	request-based-constraints
lookup static route	39,864,317 ops/sec ±0.54% (586 runs sampled)	34,997,182 ops/sec ±0.37% (587 runs sampled)
lookup dynamic route	3,055,819 ops/sec ±0.39% (588 runs sampled)	2,862,363 ops/sec ±0.56% (585 runs sampled)
lookup dynamic multi-parametric route	1,584,679 ops/sec ±0.64% (583 runs sampled)	1,576,788 ops/sec ±0.25% (589 runs sampled)
lookup dynamic multi-parametric route with regex	1,158,698 ops/sec ±0.96% (584 runs sampled)	1,196,899 ops/sec ±0.31% (589 runs sampled)
lookup long static route	2,570,186 ops/sec ±0.41% (582 runs sampled)	2,475,820 ops/sec ±0.41% (586 runs sampled)
lookup long dynamic route	2,024,055 ops/sec ±0.33% (590 runs sampled)	1,906,568 ops/sec ±0.29% (589 runs sampled)
lookup static versioned route	7,154,573 ops/sec ±0.41% (585 runs sampled)	16,925,526 ops/sec ±0.46% (579 runs sampled)
lookup static constrained (version & host) route	-	8,802,996 ops/sec ±0.55% (587 runs sampled)
find static route	29,247,517 ops/sec ±0.60% (573 runs sampled)	28,607,663 ops/sec ±0.59% (579 runs sampled)
find dynamic route	3,472,771 ops/sec ±0.41% (586 runs sampled)	3,449,980 ops/sec ±0.31% (588 runs sampled)
find dynamic multi-parametric route	1,874,547 ops/sec ±0.70% (590 runs sampled)	1,912,809 ops/sec ±0.37% (590 runs sampled)
find dynamic multi-parametric route with regex	1,407,869 ops/sec ±0.29% (590 runs sampled)	1,339,933 ops/sec ±0.78% (588 runs sampled)
find long static route	3,787,256 ops/sec ±0.24% (589 runs sampled)	3,428,143 ops/sec ±0.31% (587 runs sampled)
find long dynamic route	2,759,278 ops/sec ±0.27% (590 runs sampled)	2,635,874 ops/sec ±0.31% (584 runs sampled)
find static versioned route	8,826,557 ops/sec ±0.39% (590 runs sampled)	4,678,909 ops/sec ±0.32% (588 runs sampled)
find static constrained (version & host) route	-	4,003,335 ops/sec ±0.37% (588 runs sampled)

I'd like to note that in the current WIP code, the non-constrained routes only have the minimum constraint related code executed for them, which is:

• The function deriveConstraints which is obviously necessary
• Check at the node level of which constraints apply to determine if the normal or constrained handler store should be checked (Still not done, waiting for feedback on the issue outlined above by @airhorns)

This is the main reason for the slight performance drop in the non-constrained routes

I understand the importance of performance for fastify/find-my-way and that we're aiming to maintain or even improve the benchmarks, but is there some acceptable regression amount that we can still work with? Or should the new code outperform the current one on all tests?

[mcollina]

I would say there is no regression amount that is acceptable in the static & dynamic cases. I'm happy to take some regression on the versioned route path as it's more generic (good work!).

[AyoubElk]

I've made some updates since last time, mainly to improve performance:

• Revert to previous strategy object format and add a function to convert it to prototype format
• For default strategies (version and host), inline their constraints derivation inside the deriveConstraints function
• In case custom strategies are provided, dynamically update the deriveConstraints function to add code for faster processing
• Add regex matching for host store

These are the current benchmarks:

// request-based-constraints branch
❯ node bench
Routes registered successfully...
lookup static route x 40,244,114 ops/sec ±0.43% (585 runs sampled)
lookup dynamic route x 2,908,414 ops/sec ±1.03% (587 runs sampled)
lookup dynamic multi-parametric route x 1,521,684 ops/sec ±0.93% (582 runs sampled)
lookup dynamic multi-parametric route with regex x 1,208,451 ops/sec ±0.39% (582 runs sampled)
lookup long static route x 2,532,517 ops/sec ±0.42% (588 runs sampled)
lookup long dynamic route x 2,013,666 ops/sec ±0.37% (588 runs sampled)
lookup static versioned route x 4,489,601 ops/sec ±0.30% (587 runs sampled)
lookup static constrained (version & host) route x 4,365,146 ops/sec ±0.55% (581 runs sampled)
find static route x 28,441,579 ops/sec ±1.28% (581 runs sampled)
find dynamic route x 3,542,673 ops/sec ±0.26% (589 runs sampled)
find dynamic multi-parametric route x 1,921,871 ops/sec ±0.24% (589 runs sampled)
find dynamic multi-parametric route with regex x 1,368,976 ops/sec ±0.26% (592 runs sampled)
find long static route x 3,508,879 ops/sec ±0.22% (591 runs sampled)
find long dynamic route x 2,692,937 ops/sec ±0.17% (591 runs sampled)
find static versioned route x 5,445,838 ops/sec ±0.21% (589 runs sampled)
find static constrained (version & host) route x 5,439,364 ops/sec ±0.20% (590 runs sampled)

// master branch
❯ node bench.js
lookup static route x 41,257,268 ops/sec ±0.53% (590 runs sampled)
lookup dynamic route x 2,992,159 ops/sec ±0.47% (587 runs sampled)
lookup dynamic multi-parametric route x 1,592,345 ops/sec ±0.88% (578 runs sampled)
lookup dynamic multi-parametric route with regex x 1,154,690 ops/sec ±0.57% (582 runs sampled)
lookup long static route x 2,639,660 ops/sec ±0.40% (585 runs sampled)
lookup long dynamic route x 2,041,826 ops/sec ±0.42% (587 runs sampled)
lookup static versioned route x 6,604,706 ops/sec ±0.59% (581 runs sampled)
find static route x 28,475,395 ops/sec ±1.07% (579 runs sampled)
find dynamic route x 3,429,277 ops/sec ±0.75% (588 runs sampled)
find dynamic multi-parametric route x 1,914,887 ops/sec ±0.33% (589 runs sampled)
find dynamic multi-parametric route with regex x 1,360,918 ops/sec ±0.50% (585 runs sampled)
find long static route x 3,687,065 ops/sec ±0.82% (587 runs sampled)
find long dynamic route x 2,755,498 ops/sec ±0.43% (582 runs sampled)
find static versioned route x 8,435,637 ops/sec ±0.35% (585 runs sampled)

There is still an open issue that needs to be discussed, which is defining a deterministic constrained route matching order:

It was first described in this comment: #166 (comment)

@airhorns made a good suggestion of matching the first route that matches the constraints, however it seems that it won't work because for example the Semverstore doesn't return the first route that matches the constraints, it returns the handler with the highest version available (which isn't necessarily the first one defined)

router.on('GET', '/users', { constraints: { version: '1.1.0', host: 'admin.airhorns.dev' } }, handler1)
router.on('GET', '/users', { constraints: { version: '1.2.0', host: '*.airhorns.dev' } }, handler2)

For a request with accept-version='1.x' and host='admin.airhorns.dev':

• The semver store will match the highest available version and return handler2
• The host store will return handler1

In this case, the constraint store will have 2 different handlers, and will consider that no match was found and return null

[airhorns]

I feel like the max version ordering for the semver store makes sense semantically and I think we shouldn't break it, but that is indeed annoying! Versions have an ordering where you can definitely sort one version as greater than another, but hosts don't, so I still think we need some other predictable way to sort the routes by a host constraint, and I think the most sensible way would be by addition order.

Would it be possible to just special case it? When there's more than one handler, check if there's a version constraint and use the max version if so, and otherwise just use the first one?

[airhorns]

@AyoubElk I don't mind taking a stab at the semantics I just mentioned -- do you have any unpushed code though that I should base changes on?

[AyoubElk]

@AyoubElk I don't mind taking a stab at the semantics I just mentioned -- do you have any unpushed code though that I should base changes on?

@airhorns yes, I updated the tests to support the new constraints format (One test is failing currently where a default handler is returned when it shouldn't, I'm trying to come up with a fix for it).

Regarding the semantics you described, I think they might work as a special case but will probably yield unexpected results in case custom strategies are added (the version strategy can be overridden, or the user might want to have another constraint take precedence, etc..). I'm trying to figure out a better way for this as well