Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

compressor filter: add benchmark #10464

Merged
merged 32 commits into from
Apr 24, 2020

Conversation

rgs1
Copy link
Member

@rgs1 rgs1 commented Mar 20, 2020

I am debugging a case of slow responses possibly due to compression, so
throwing in some benchmarks to get some perf data about this filter.

$ ./bazel-bin/test/extensions/filters/http/common/compressor/compressor_filter_speed_test
2020-03-20 00:15:07
Running ./bazel-bin/test/extensions/filters/http/common/compressor/compressor_filter_speed_test
Run on (8 X 2300 MHz CPU s)
CPU Caches:
  L1 Data 32K (x4)
  L1 Instruction 32K (x4)
  L2 Unified 262K (x4)
  L3 Unified 6291K (x1)
Load Average: 2.17, 2.14, 2.00
***WARNING*** Library was built as DEBUG. Timings may be affected.
---------------------------------------------------------
Benchmark               Time             CPU   Iterations
---------------------------------------------------------
...
compressFull/0/manual_time              14.1 ms         14.3 ms           48
compressFull/1/manual_time              7.06 ms         7.22 ms          104
compressFull/2/manual_time              5.17 ms         5.33 ms          123
compressFull/3/manual_time              15.4 ms         15.5 ms           45
compressFull/4/manual_time              10.1 ms         10.3 ms           69
compressFull/5/manual_time              15.8 ms         16.0 ms           40
compressFull/6/manual_time              15.3 ms         15.5 ms           42
compressFull/7/manual_time              9.91 ms         10.1 ms           71
compressFull/8/manual_time              15.8 ms         16.0 ms           45
compressChunks16384/0/manual_time       13.4 ms         13.5 ms           52
compressChunks16384/1/manual_time       6.33 ms         6.48 ms          111
compressChunks16384/2/manual_time       5.09 ms         5.27 ms          147
compressChunks16384/3/manual_time       15.1 ms         15.3 ms           46
compressChunks16384/4/manual_time       9.61 ms         9.78 ms           71
compressChunks16384/5/manual_time       14.5 ms         14.6 ms           47
compressChunks16384/6/manual_time       14.0 ms         14.1 ms           48
compressChunks16384/7/manual_time       9.20 ms         9.36 ms           76
compressChunks16384/8/manual_time       14.5 ms         14.6 ms           48
compressChunks8192/0/manual_time        14.3 ms         14.5 ms           50
compressChunks8192/1/manual_time        6.80 ms         6.96 ms          100
compressChunks8192/2/manual_time        5.21 ms         5.36 ms          135
compressChunks8192/3/manual_time        14.9 ms         15.0 ms           47
compressChunks8192/4/manual_time        9.71 ms         9.87 ms           68
compressChunks8192/5/manual_time        15.9 ms         16.1 ms           45
...

Signed-off-by: Raul Gutierrez Segales rgs@pinterest.com

I am debugging a case of slow responses possibly due to compression, so
throwing in some benchmarks to get some perf data about this filter.

```
$ ./bazel-bin/test/extensions/filters/http/common/compressor/compressor_filter_speed_test
2020-03-20 00:15:07
Running ./bazel-bin/test/extensions/filters/http/common/compressor/compressor_filter_speed_test
Run on (8 X 2300 MHz CPU s)
CPU Caches:
  L1 Data 32K (x4)
  L1 Instruction 32K (x4)
  L2 Unified 262K (x4)
  L3 Unified 6291K (x1)
Load Average: 2.17, 2.14, 2.00
***WARNING*** Library was built as DEBUG. Timings may be affected.
---------------------------------------------------------
Benchmark               Time             CPU   Iterations
---------------------------------------------------------
FilterCompress   24674140 ns     24616586 ns           29
```

Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>
@rgs1 rgs1 requested a review from dio as a code owner March 20, 2020 03:17
@rojkov
Copy link
Member

rojkov commented Mar 20, 2020

Thanks! This looks good to me.

BTW, what kind of slowness you're debugging? Bad latency or throughput?

@rgs1
Copy link
Member Author

rgs1 commented Mar 20, 2020

BTW, what kind of slowness you're debugging? Bad latency or throughput?

Latency -- with big responses (> 100kb)...

Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>
Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>
@rgs1
Copy link
Member Author

rgs1 commented Mar 20, 2020

/retest

@repokitteh-read-only
Copy link

🔨 rebuilding ci/circleci: coverage (failed build)

🐱

Caused by: a #10464 (comment) was created by @rgs1.

see: more, trace.

Raul Gutierrez Segales added 11 commits March 21, 2020 19:45
Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>
Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>
* spelling
* const-ness

Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>
Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>
Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>
Turns out, instantiating this Mock takes ~6ms (!!).

Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>
Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>
Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>
Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>
This brings time down to < 5ms (way better!). Turns we were
using a lot of time calling TestUtility::feedBufferWithRandomCharacters().

Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>
@rgs1
Copy link
Member Author

rgs1 commented Mar 25, 2020

Once #10508 lands, I'll update this to exercise different chunk sizes. We can still land this as is, and I'll do a follow-up -- no need to wait.

@stale
Copy link

stale bot commented Apr 1, 2020

This pull request has been automatically marked as stale because it has not had activity in the last 7 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

@stale stale bot added stale stalebot believes this issue/PR has not been touched recently and removed stale stalebot believes this issue/PR has not been touched recently labels Apr 1, 2020
Raul Gutierrez Segales added 4 commits April 1, 2020 18:13
Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>
Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>
Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>
Raul Gutierrez Segales added 2 commits April 2, 2020 11:12
Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>
@rgs1
Copy link
Member Author

rgs1 commented Apr 3, 2020

PTAL @rojkov @dio

rojkov
rojkov previously approved these changes Apr 6, 2020
Copy link
Member

@rojkov rojkov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! LGTM

Raul Gutierrez Segales added 4 commits April 8, 2020 13:02
rojkov
rojkov previously approved these changes Apr 14, 2020
Raul Gutierrez Segales added 2 commits April 14, 2020 09:54
This actually makes the difference between different chunk size mostly
dissapear. I suspect the previous differences were due to chunks being
the same, since the seed for the random data isn't changing.

Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>
@rgs1
Copy link
Member Author

rgs1 commented Apr 14, 2020

@rojkov @dio one last change -- using the same input for the different chunk sizes, the compression times are mostly the same....

Raul Gutierrez Segales added 2 commits April 15, 2020 18:32
Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>
@rgs1
Copy link
Member Author

rgs1 commented Apr 16, 2020

@dio PTAL.

@rgs1
Copy link
Member Author

rgs1 commented Apr 23, 2020

ping?

Copy link
Member

@dio dio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! This looks good to me.

@mattklein123 mattklein123 self-assigned this Apr 24, 2020
@mattklein123 mattklein123 merged commit 100c957 into envoyproxy:master Apr 24, 2020
spenceral added a commit to spenceral/envoy that referenced this pull request Apr 27, 2020
Signed-off-by: Spencer Lewis <slewis@squareup.com>

* master:
  fault injection: add support for setting gRPC status (envoyproxy#10841)
  tests: tag tests that fail on Windows with fails_on_windows (envoyproxy#10940)
  Fix typo on Postgres Proxy documentation. (envoyproxy#10930)
  fuzz: improve header/data stop/continue modeling in HCM fuzzer. (envoyproxy#10931)
  gzip filter: allow setting zlib compressor's chunk size (envoyproxy#10508)
  http: replace vector/reserve with InlinedVector in codec helper (envoyproxy#10941)
  stats: add utilities to create stats from a vector of tokens, mixing dynamic and symbolic elements. (envoyproxy#10735)
  hcm: avoid invoking 100-continue handling on decode filter. (envoyproxy#10929)
  prometheus stats: Correctly group lines of the same metric name. (envoyproxy#10833)
  status: Fix ASAN error in Status payload handling (envoyproxy#10906)
  path: Fix merge slash for paths ending with slash and present query args (envoyproxy#10922)
  compressor filter: add benchmark (envoyproxy#10464)
  xray: expected_span_name is not captured by the lambda with MSVC (envoyproxy#10934)
  ci: update before purge in cleanup (envoyproxy#10938)
  tracer: Improve test coverage for x-ray (envoyproxy#10890)
  Revert "init: order dynamic resource initialization to make RTDS always be first (envoyproxy#10362)" (envoyproxy#10919)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants