Add a serialized variant of the Trie-router #402

Joannis · 2024-03-17T17:33:33Z

Since this trie is contiguous memory, routing performance should improve. This does not implement recursive wildcards yet, and I found that those tests are missing too.

In a quick test setup, the gap between no-router and having the old trie router was ~3-4k req/sec on a baseline (with router) of 98k. That gap shrunk by roughly 1k req/sec, leading to an estimated improvement of 25% in a small routing table (PerformanceBenchmarks).

I expect, and seemed to find a more significant performance gap when as the amount of path components increased. The expectation is based on the fact that more trie nodes are created when there are more path components. By keeping the routing table contiguous, it's expected that these scenarios are more performant.

APIs tend to have more path components to organise themselves, with many backends opting for /api/v1-type of prefixes for versioning. /api/v1/users/:id/profile is not a very long route.

TODO

Proper Benchmarks
Add a Recursive Wildcard test
Add a recursive wildcard implementation

Joannis · 2024-03-20T13:46:13Z

Benchmark Results

BinaryTrieRouter
╒═══════════════════════════╤═════════╤═════════╤═════════╤═════════╤═════════╤═════════╤═════════╤═════════╕
│ Metric                    │      p0 │     p25 │     p50 │     p75 │     p90 │     p99 │    p100 │ Samples │
╞═══════════════════════════╪═════════╪═════════╪═════════╪═════════╪═════════╪═════════╪═════════╪═════════╡
│ Malloc (total) *          │      24 │      24 │      24 │      24 │      24 │      24 │      24 │     164 │
├───────────────────────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┤
│ Throughput (# / s) (K)    │     167 │     166 │     165 │     164 │     162 │     158 │     157 │     164 │
├───────────────────────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┤
│ Time (total CPU) (ns) *   │    6001 │    6038 │    6058 │    6091 │    6169 │    6332 │    6353 │     164 │
╘═══════════════════════════╧═════════╧═════════╧═════════╧═════════╧═════════╧═════════╧═════════╧═════════╛

BinaryTrieRouterParameters
╒═══════════════════════════╤═════════╤═════════╤═════════╤═════════╤═════════╤═════════╤═════════╤═════════╕
│ Metric                    │      p0 │     p25 │     p50 │     p75 │     p90 │     p99 │    p100 │ Samples │
╞═══════════════════════════╪═════════╪═════════╪═════════╪═════════╪═════════╪═════════╪═════════╪═════════╡
│ Malloc (total) *          │      28 │      28 │      28 │      28 │      28 │      28 │      28 │     145 │
├───────────────────────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┤
│ Throughput (# / s) (K)    │     148 │     148 │     147 │     145 │     142 │     138 │     138 │     145 │
├───────────────────────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┤
│ Time (total CPU) (ns) *   │    6768 │    6795 │    6820 │    6906 │    7033 │    7201 │    7210 │     145 │
╘═══════════════════════════╧═════════╧═════════╧═════════╧═════════╧═════════╧═════════╧═════════╧═════════╛

TrieRouter
╒═══════════════════════════╤═════════╤═════════╤═════════╤═════════╤═════════╤═════════╤═════════╤═════════╕
│ Metric                    │      p0 │     p25 │     p50 │     p75 │     p90 │     p99 │    p100 │ Samples │
╞═══════════════════════════╪═════════╪═════════╪═════════╪═════════╪═════════╪═════════╪═════════╪═════════╡
│ Malloc (total) *          │      64 │      64 │      64 │      64 │      64 │      64 │      64 │     110 │
├───────────────────────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┤
│ Throughput (# / s) (K)    │     112 │     111 │     110 │     109 │     107 │     105 │     105 │     110 │
├───────────────────────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┤
│ Time (total CPU) (ns) *   │    8966 │    9003 │    9069 │    9191 │    9347 │    9454 │    9514 │     110 │
╘═══════════════════════════╧═════════╧═════════╧═════════╧═════════╧═════════╧═════════╧═════════╧═════════╛

TrieRouterParameters
╒═══════════════════════════╤═════════╤═════════╤═════════╤═════════╤═════════╤═════════╤═════════╤═════════╕
│ Metric                    │      p0 │     p25 │     p50 │     p75 │     p90 │     p99 │    p100 │ Samples │
╞═══════════════════════════╪═════════╪═════════╪═════════╪═════════╪═════════╪═════════╪═════════╪═════════╡
│ Malloc (total) *          │      78 │      78 │      78 │      78 │      78 │      78 │      78 │      90 │
├───────────────────────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┤
│ Throughput (# / s) (K)    │      91 │      90 │      90 │      90 │      89 │      84 │      84 │      90 │
├───────────────────────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┤
│ Time (total CPU) (μs) *   │      11 │      11 │      11 │      11 │      11 │      12 │      12 │      90 │
╘═══════════════════════════╧═════════╧═════════╧═════════╧═════════╧═════════╧═════════╧═════════╧═════════╛

adam-fowler · 2024-03-20T19:59:18Z

Can you post a benchmark comparison, with the original

Joannis · 2024-03-22T10:37:38Z

@adam-fowler the comparison is between TrieRouter / TrieRouterParameters and BinaryTrieRouter / BinaryTrieRouterParameters

Joannis · 2024-03-22T10:38:15Z

I've reduces the bloat from my benchmark comment

adam-fowler · 2024-03-22T15:49:57Z

This is failing CI

adam-fowler

Here are a series of initial comments. These should make the code more readable and more performant

Sources/Hummingbird/Router/BinaryTrie/BinaryTrie+resolve.swift

adam-fowler · 2024-03-23T16:36:39Z

Sources/Hummingbird/Router/BinaryTrie/BinaryTrie+serialize.swift

+
+            // Serialize the path constant
+            try trie.writeLengthPrefixed(as: Integer.self) { buffer in
+                buffer.writeSubstring(path)


Given you only ever write Strings with a prefixed length why don't you add a ByteBuffer. writeLengthPrefixedString()

In actual fact you could fix the issue where you are throwing an error from init and provide more detailing error information.

func writeLengthPrefixedString(_ string: SubString) { do { try trie.writeLengthPrefixed(as: Integer.self) { buffer in buffer.writeSubstring(path) } } catch { preconditionFailure("Failed to write \"\(string)\" into BinaryTrie") } }

adam-fowler · 2024-03-23T16:38:39Z

Sources/Hummingbird/Router/BinaryTrie/BinaryTrie+resolve.swift

+            let _token: Integer = trie.readInteger(),
+            let token = TokenKind(rawValue: _token),
+            let nextSiblingNodeIndex: UInt32 = trie.readInteger()
+        {


Maybe add a ByteBuffer.readNode() that returns a BinaryTrieNode type to improve readability

Sources/Hummingbird/Router/BinaryTrie/BinaryTrie+resolve.swift

Sources/Hummingbird/Router/BinaryTrie/BinaryTrie+serialize.swift

Sources/Hummingbird/Router/BinaryTrie/BinaryTrie+resolve.swift

adam-fowler · 2024-03-23T17:04:17Z

Sources/Hummingbird/Router/BinaryTrie/BinaryTrie+resolve.swift

+    private func descendPath(
+        in trie: inout ByteBuffer,
+        index: UInt16,
+        parameters: inout Parameters,


Given you return the parameters, why is this marked inout?

Sources/Hummingbird/Router/BinaryTrie/BinaryTrie+resolve.swift

Since this trie is contiguous memory, routing performance should improve. Note that

adam-fowler · 2024-04-13T15:11:33Z

Sources/Hummingbird/Router/BinaryTrie/BinaryTrie+resolve.swift

+        return token
+    }
+
+    mutating func readBinaryTrieNode() -> BinaryTrieNode? {


This function doesn't compile and it doesn't seem to be used

adam-fowler · 2024-04-13T15:16:07Z

Sources/Hummingbird/Router/BinaryTrie/BinaryTrie+resolve.swift

+                        return nil
+                    }
+
+                    component = components[components.startIndex]


Can we guarantee at this point that the array is non-empty?

haha. just saw the check above, perhaps you could change this to

guard let component = components.first else { return nil }

adam-fowler · 2024-04-24T08:18:39Z

Sources/Hummingbird/Router/TrieRouter.swift

+import NIOCore
+
+public struct BinaryRouterResponder<Context: BaseRequestContext>: HTTPResponder {
+    let trie: BinaryTrie<EndpointResponders<Context>>


This seems like an odd place to add this type. Might be worthwhile adding its own file. The original RouterResponder has its own file.

adam-fowler · 2024-04-24T08:41:24Z

Sources/Hummingbird/Router/Router.swift

        if self.options.contains(.autoGenerateHeadEndpoints) {
            self.trie.forEach { node in
                node.value?.autoGenerateHeadEndpoint()
            }
        }
-        return RouterResponder(
+        return try! .init(


!? We should remove this.

adam-fowler · 2024-04-24T08:52:47Z

Sources/Hummingbird/Router/BinaryTrie/BinaryTrie+serialize.swift

+
+            // Serialize the path constant
+            try trie.writeLengthPrefixed(as: Integer.self) { buffer in
+                buffer.writeSubstring(path)


In actual fact you could fix the issue where you are throwing an error from init and provide more detailing error information.

func writeLengthPrefixedString(_ string: SubString) { do { try trie.writeLengthPrefixed(as: Integer.self) { buffer in buffer.writeSubstring(path) } } catch { preconditionFailure("Failed to write \"\(string)\" into BinaryTrie") } }

* Added ByteBuffer.writeLengthPrefixedString. Moved ByteBuffer extensions into own file * Move some code about, delete RouterResponder * Use RouterPathTrieBuilder to build BinaryTrie * Test BinaryTrie * Fix bug where deadend doesn't have a index * Add ByteBuffer read/write of BinaryTrieNode * Remove trieRouterBenchmarks * Remove inout variables * Simplified resolve * Fixed matching paths after catchall * Renamed back to RouterResponder * Delete commented out code

adam-fowler

With the changes from #436 this is ready

Joannis requested a review from adam-fowler March 17, 2024 17:33

Joannis marked this pull request as ready for review March 22, 2024 20:33

adam-fowler reviewed Mar 23, 2024

View reviewed changes

Joannis added 6 commits April 11, 2024 18:18

Add a serialized variant of the Trie-router

d4025f8

Since this trie is contiguous memory, routing performance should improve. Note that

fix benchmarks and add BinaryTrie benchmarks

1bb8d8c

Add a benchmark for long paths

6c2f4cb

Fix memcmp and the tests

b5e7225

Implement recursiveWildcard

b2bf10e

Apply some of Adam's suggestions to make BinaryTrie code more readable

a75ae59

Joannis force-pushed the jo/serialized-trie-router branch from a28940e to a75ae59 Compare April 11, 2024 17:07

adam-fowler reviewed Apr 13, 2024

View reviewed changes

adam-fowler reviewed Apr 24, 2024

View reviewed changes

adam-fowler approved these changes May 6, 2024

View reviewed changes

adam-fowler merged commit b67206a into main May 6, 2024
4 of 5 checks passed

adam-fowler deleted the jo/serialized-trie-router branch May 6, 2024 07:38

Joannis mentioned this pull request May 8, 2024

Allow inlining (therefore specializing) the router's functions, improving performance further #440

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a serialized variant of the Trie-router #402

Add a serialized variant of the Trie-router #402

Joannis commented Mar 17, 2024 •

edited

Loading

Joannis commented Mar 20, 2024 •

edited

Loading

adam-fowler commented Mar 20, 2024 •

edited

Loading

Joannis commented Mar 22, 2024 •

edited

Loading

Joannis commented Mar 22, 2024

adam-fowler commented Mar 22, 2024

adam-fowler left a comment

adam-fowler Mar 23, 2024

adam-fowler Apr 24, 2024

adam-fowler Mar 23, 2024

adam-fowler Mar 23, 2024

adam-fowler Apr 13, 2024

adam-fowler Apr 13, 2024

adam-fowler Apr 13, 2024

adam-fowler Apr 24, 2024

adam-fowler Apr 24, 2024

adam-fowler Apr 24, 2024

adam-fowler left a comment

Add a serialized variant of the Trie-router #402

Add a serialized variant of the Trie-router #402

Conversation

Joannis commented Mar 17, 2024 • edited Loading

TODO

Joannis commented Mar 20, 2024 • edited Loading

Benchmark Results

adam-fowler commented Mar 20, 2024 • edited Loading

Joannis commented Mar 22, 2024 • edited Loading

Joannis commented Mar 22, 2024

adam-fowler commented Mar 22, 2024

adam-fowler left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adam-fowler left a comment

Choose a reason for hiding this comment

Joannis commented Mar 17, 2024 •

edited

Loading

Joannis commented Mar 20, 2024 •

edited

Loading

adam-fowler commented Mar 20, 2024 •

edited

Loading

Joannis commented Mar 22, 2024 •

edited

Loading