router: add benchmarks for route matching #13320

nareddyt · 2020-09-29T23:53:20Z

Commit Message:
router: add benchmarks for route matching

Additional Description:
Measure the speed of doing a route match against a route table of varying sizes. Why? Currently, route matching is linear in first-to-win ordering.

We benchmark with 3 different type of route matchers: exact path, path prefix, and regex that represents common OpenAPI path templating.

Here are the timings I get when running on my workstation (compiled in opt). As expected, route matching is O(n) based on the route table size.

Run on (12 X 4500 MHz CPU s)
CPU Caches:
  L1 Data 32 KiB (x6)
  L1 Instruction 32 KiB (x6)
  L2 Unified 1024 KiB (x6)
  L3 Unified 8448 KiB (x1)
Load Average: 1.95, 2.00, 2.23
------------------------------------------------------------------------------------
Benchmark                                          Time             CPU   Iterations
------------------------------------------------------------------------------------
bmRouteTableSizeWithPathPrefixMatch/1            611 ns          611 ns      1149623
bmRouteTableSizeWithPathPrefixMatch/2            715 ns          715 ns       985355
bmRouteTableSizeWithPathPrefixMatch/4            889 ns          888 ns       793001
bmRouteTableSizeWithPathPrefixMatch/8           1238 ns         1238 ns       570156
bmRouteTableSizeWithPathPrefixMatch/16          2046 ns         2045 ns       341722
bmRouteTableSizeWithPathPrefixMatch/32          3454 ns         3454 ns       201103
bmRouteTableSizeWithPathPrefixMatch/64          6440 ns         6439 ns       108867
bmRouteTableSizeWithPathPrefixMatch/128        13185 ns        13184 ns        53151
bmRouteTableSizeWithPathPrefixMatch/256        25774 ns        25769 ns        27079
bmRouteTableSizeWithPathPrefixMatch/512        57157 ns        57143 ns        12298
bmRouteTableSizeWithPathPrefixMatch/1024      138813 ns       138808 ns         5110
bmRouteTableSizeWithPathPrefixMatch/2048      290029 ns       289997 ns         2396
bmRouteTableSizeWithPathPrefixMatch/4096      773725 ns       773695 ns          919
bmRouteTableSizeWithPathPrefixMatch/8192     1752555 ns      1752309 ns          396
bmRouteTableSizeWithPathPrefixMatch/16384    3733729 ns      3733540 ns          186
bmRouteTableSizeWithExactPathMatch/1             608 ns          608 ns      1144239
bmRouteTableSizeWithExactPathMatch/2             710 ns          710 ns       990553
bmRouteTableSizeWithExactPathMatch/4             880 ns          880 ns       803334
bmRouteTableSizeWithExactPathMatch/8            1224 ns         1224 ns       574644
bmRouteTableSizeWithExactPathMatch/16           2065 ns         2065 ns       338375
bmRouteTableSizeWithExactPathMatch/32           3462 ns         3461 ns       198839
bmRouteTableSizeWithExactPathMatch/64           6385 ns         6385 ns       109882
bmRouteTableSizeWithExactPathMatch/128         13257 ns        13256 ns        53856
bmRouteTableSizeWithExactPathMatch/256         25840 ns        25839 ns        27063
bmRouteTableSizeWithExactPathMatch/512         57826 ns        57817 ns        12218
bmRouteTableSizeWithExactPathMatch/1024       137411 ns       137406 ns         5089
bmRouteTableSizeWithExactPathMatch/2048       287451 ns       287426 ns         2433
bmRouteTableSizeWithExactPathMatch/4096       768610 ns       768577 ns          914
bmRouteTableSizeWithExactPathMatch/8192      1769165 ns      1768988 ns          395
bmRouteTableSizeWithExactPathMatch/16384     3868125 ns      3868023 ns          184
bmRouteTableSizeWithRegexMatch/1                 688 ns          688 ns      1013629
bmRouteTableSizeWithRegexMatch/2                 885 ns          885 ns       793198
bmRouteTableSizeWithRegexMatch/4                1249 ns         1249 ns       575334
bmRouteTableSizeWithRegexMatch/8                1999 ns         1999 ns       349547
bmRouteTableSizeWithRegexMatch/16               3851 ns         3851 ns       181748
bmRouteTableSizeWithRegexMatch/32               7006 ns         7006 ns        99469
bmRouteTableSizeWithRegexMatch/64              13388 ns        13387 ns        52591
bmRouteTableSizeWithRegexMatch/128             26278 ns        26275 ns        26104
bmRouteTableSizeWithRegexMatch/256             57930 ns        57925 ns        12117
bmRouteTableSizeWithRegexMatch/512            160308 ns       160292 ns         4345
bmRouteTableSizeWithRegexMatch/1024           324708 ns       324680 ns         2113
bmRouteTableSizeWithRegexMatch/2048           801549 ns       801490 ns          845
bmRouteTableSizeWithRegexMatch/4096          2306977 ns      2306349 ns          286
bmRouteTableSizeWithRegexMatch/8192          4668115 ns      4667039 ns          154
bmRouteTableSizeWithRegexMatch/16384         9219490 ns      9218921 ns           73

Risk Level:
None

Testing:
Added benchmark

Docs Changes:
None

Release Notes:
None

Signed-off-by: Teju Nareddy <[email protected]>

nareddyt · 2020-09-29T23:54:21Z

cc @qiwzhang @TAOXUY

test/common/router/config_impl_speed_test.cc

Signed-off-by: Teju Nareddy <[email protected]>

jmarantz

@envoyproxy/senior-maintainers

Looks great modulo one nit, and senior maintainer approval.

test/benchmark/main.cc

mattklein123

One comment, thank you!

/wait

test/benchmark/main.cc

antoniovicente

Thanks for improving benchmark coverage.

test/common/router/config_impl_speed_test.cc

test/benchmark/main.cc

antoniovicente · 2020-10-01T14:39:27Z

test/common/router/BUILD

+
+envoy_cc_benchmark_binary(
+    name = "config_impl_speed_test",
+    srcs = ["config_impl_speed_test.cc"],


There's an open conversation about name of these targets and source files in #13327

speed_test makes sense for consistency with current sources, but calling them benchmark and benchmark_test may be better long term. Let's see what we decide on that other PR.

Sure, I'll wait for a few days.

Signed-off-by: Teju Nareddy <[email protected]>

# Conflicts: # test/common/router/BUILD Signed-off-by: Teju Nareddy <[email protected]>

Signed-off-by: Teju Nareddy <[email protected]>

mattklein123 · 2020-10-05T18:08:17Z

@jmarantz Antonio is out this week. Would you mind taking a look at this? I'm not sure what is still outstanding.

@qiwzhang

) Previously, we made a single route when running ESPv2 in sidecar mode. But now we plan to deprecate the path matcher filter and support configuring timeouts for the local backend (#321). To do this, we should switch to configuring the Envoy route table for individual routes for even the local backend. There is one extra feature introduced by this PR: We need to support matching wildcard HTTP Methods when using a [Custom Http Pattern](https://github.com/googleapis/googleapis/blob/cb7fc620590382a4a2ea6ffdf6f51ae0e77bbbb5/google/api/http.proto#L343). This just required an if statement and extra test in `route_generator.go`. **High-risk change** Summarized comments from me and @qiwzhang: > We have to use regex on “path” to support url_template. But path_matcher supports many complicated forms that regex could not support. Currently we use these regex to replace url templates, but not all templates are supported. > I think it is fine, our endpoints users are not using very complicated url templates. We only need to support the url templates from openapi spec, and options.http.rules from grpc proto file. We are still in Beta (not GA), so it should be fine. **Testing Done** - Fix lots of unit and integration tests. - Micro benchmark of the performance impact of switching to per-route config instead of path prefix: envoyproxy/envoy#13320 Signed-off-by: Teju Nareddy <[email protected]>

jmarantz

Looks great with one minor comment to address parallel to @envoyproxy/senior-maintainers review

test/benchmark/main.cc

mattklein123

Nice, LGTM other than remaining comment.

/wait

test/benchmark/main.cc

Signed-off-by: Teju Nareddy <[email protected]>

mattklein123

Thanks!

nareddyt added 3 commits September 29, 2020 14:46

router: basic route match benchmarks

3222abb

Signed-off-by: Teju Nareddy <[email protected]>

Fix benchmarking and logging

c10971e

Signed-off-by: Teju Nareddy <[email protected]>

Format

c0319d9

Signed-off-by: Teju Nareddy <[email protected]>

tonya11en reviewed Sep 30, 2020

View reviewed changes

test/common/router/config_impl_speed_test.cc Outdated Show resolved Hide resolved

nareddyt requested a review from tonya11en September 30, 2020 02:11

mattklein123 assigned jmarantz Sep 30, 2020

Lint and fix assertion

a9984cd

Signed-off-by: Teju Nareddy <[email protected]>

jmarantz previously approved these changes Sep 30, 2020

View reviewed changes

test/benchmark/main.cc Outdated Show resolved Hide resolved

mattklein123 requested changes Oct 1, 2020

View reviewed changes

test/benchmark/main.cc Outdated Show resolved Hide resolved

repokitteh-read-only bot added the waiting label Oct 1, 2020

mattklein123 self-assigned this Oct 1, 2020

antoniovicente self-assigned this Oct 1, 2020

antoniovicente reviewed Oct 1, 2020

View reviewed changes

test/common/router/config_impl_speed_test.cc Outdated Show resolved Hide resolved

test/common/router/config_impl_speed_test.cc Outdated Show resolved Hide resolved

test/benchmark/main.cc Outdated Show resolved Hide resolved

antoniovicente reviewed Oct 1, 2020

View reviewed changes

nareddyt mentioned this pull request Oct 1, 2020

b/170162456: Enable route config for local backend in sidecar mode GoogleCloudPlatform/esp-v2#358

Merged

Review comments

ed40c65

Signed-off-by: Teju Nareddy <[email protected]>

nareddyt dismissed jmarantz’s stale review via ed40c65 October 2, 2020 20:03

repokitteh-read-only bot removed the waiting label Oct 2, 2020

nareddyt requested review from mattklein123, jmarantz and antoniovicente October 2, 2020 20:04

nareddyt added 2 commits October 2, 2020 13:09

Merge branch 'master' into benchmark-route-match

377196c

# Conflicts: # test/common/router/BUILD Signed-off-by: Teju Nareddy <[email protected]>

Fix merge

9c0a155

Signed-off-by: Teju Nareddy <[email protected]>

mattklein123 removed their assignment Oct 5, 2020

jmarantz previously approved these changes Oct 6, 2020

View reviewed changes

test/benchmark/main.cc Outdated Show resolved Hide resolved

mattklein123 requested changes Oct 6, 2020

View reviewed changes

test/benchmark/main.cc Outdated Show resolved Hide resolved

repokitteh-read-only bot added the waiting label Oct 6, 2020

mattklein123 self-assigned this Oct 6, 2020

Review comment

1909b78

Signed-off-by: Teju Nareddy <[email protected]>

nareddyt dismissed jmarantz’s stale review via 1909b78 October 6, 2020 21:10

repokitteh-read-only bot removed the waiting label Oct 6, 2020

nareddyt requested review from mattklein123 and jmarantz October 6, 2020 21:10

mattklein123 approved these changes Oct 6, 2020

View reviewed changes

mattklein123 merged commit 3fac88f into envoyproxy:master Oct 6, 2020

nareddyt deleted the benchmark-route-match branch October 7, 2020 00:24

nareddyt mentioned this pull request Mar 1, 2021

support URI Template based route match #7763

Open

nareddyt mentioned this pull request Mar 15, 2021

router: add benchmarks for URL template path matcher #15493

Closed

yanjunxiang-google mentioned this pull request May 27, 2021

Extending Envoy route matching benchmark testing to cover variable path-rule length case #16704

Closed

yanjunxiang-google mentioned this pull request Dec 1, 2023

Increase limit of path_match_policy.path_template variables #31018

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

router: add benchmarks for route matching #13320

router: add benchmarks for route matching #13320

nareddyt commented Sep 29, 2020 •

edited

Loading

nareddyt commented Sep 29, 2020

jmarantz left a comment

mattklein123 left a comment

antoniovicente left a comment

antoniovicente Oct 1, 2020

nareddyt Oct 2, 2020

mattklein123 commented Oct 5, 2020

jmarantz left a comment

mattklein123 left a comment

mattklein123 left a comment

router: add benchmarks for route matching #13320

router: add benchmarks for route matching #13320

Conversation

nareddyt commented Sep 29, 2020 • edited Loading

nareddyt commented Sep 29, 2020

jmarantz left a comment

Choose a reason for hiding this comment

mattklein123 left a comment

Choose a reason for hiding this comment

antoniovicente left a comment

Choose a reason for hiding this comment

antoniovicente Oct 1, 2020

Choose a reason for hiding this comment

nareddyt Oct 2, 2020

Choose a reason for hiding this comment

mattklein123 commented Oct 5, 2020

jmarantz left a comment

Choose a reason for hiding this comment

mattklein123 left a comment

Choose a reason for hiding this comment

mattklein123 left a comment

Choose a reason for hiding this comment

nareddyt commented Sep 29, 2020 •

edited

Loading