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

Implement cache mechanism for GMT date strings. #2012

Merged
merged 1 commit into from Sep 23, 2021
Merged

Conversation

kubo39
Copy link
Contributor

@kubo39 kubo39 commented Dec 20, 2017

This patch improves performance about 5~8% slighty.
Full bench log and bench.sh is here.

Benchmarks

master (da8d305)

+ git rev-parse HEAD
da8d30598bdf9401d512a46a077c62b061926e18
(...)
~/dev/dlang/vibe.d/examples/bench-http-server ~/dev/dlang/vibe.d ~/dev/dlang
+ wrk -c 1000 -d 10s http://localhost:8080/
Running 10s test @ http://localhost:8080/
  2 threads and 1000 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency    27.89ms   71.86ms   1.26s    97.89%
    Req/Sec    25.07k     2.12k   31.55k    89.90%
  498926 requests in 10.10s, 401.59MB read
Requests/sec:  49418.30
Transfer/sec:     39.78MB
+ wrk -c 1000 -d 10s http://localhost:8080/empty
Running 10s test @ http://localhost:8080/empty
  2 threads and 1000 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency    11.96ms    8.32ms 323.13ms   85.31%
    Req/Sec    38.73k     3.22k   45.56k    91.88%
  768749 requests in 10.10s, 122.43MB read
Requests/sec:  76132.29
Transfer/sec:     12.13MB
+ wrk -c 1000 -d 10s http://localhost:8080/static/10
Running 10s test @ http://localhost:8080/static/10
  2 threads and 1000 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency    12.94ms    8.12ms 300.31ms   77.79%
    Req/Sec    33.67k     3.68k   39.95k    76.02%
  669984 requests in 10.08s, 113.73MB read
  Requests/sec:  66489.79
Transfer/sec:     11.29MB
+ wrk -c 1000 -d 10s http://localhost:8080/static/1k
Running 10s test @ http://localhost:8080/static/1k
  2 threads and 1000 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency    14.13ms   10.93ms 353.32ms   77.19%
    Req/Sec    33.51k     3.24k   38.76k    85.35%
  665977 requests in 10.07s, 743.10MB read
Requests/sec:  66106.13
Transfer/sec:     73.76MB
+ wrk -c 1000 -d 10s http://localhost:8080/static/10k
Running 10s test @ http://localhost:8080/static/10k
  2 threads and 1000 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency    15.08ms   14.98ms 506.88ms   95.79%
    Req/Sec    30.78k     3.76k   39.15k    69.54%
  611313 requests in 10.07s, 5.79GB read
Requests/sec:  60696.30
Transfer/sec:    588.74MB

date (850dffd)

+ git rev-parse HEAD
850dffdafc0a965f9ace15ce8966a2234616be89
(...)
+ wrk -c 1000 -d 10s http://localhost:8080/
Running 10s test @ http://localhost:8080/
  2 threads and 1000 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency    20.97ms   28.31ms 679.65ms   98.73%
    Req/Sec    25.69k     1.32k   29.24k    66.16%
  510926 requests in 10.07s, 411.24MB read
Requests/sec:  50723.68
Transfer/sec:     40.83MB
+ wrk -c 1000 -d 10s http://localhost:8080/empty
Running 10s test @ http://localhost:8080/empty
  2 threads and 1000 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency    12.26ms   11.94ms 485.38ms   96.26%
    Req/Sec    40.70k     1.93k   46.86k    78.79%
  807708 requests in 10.02s, 128.64MB read
Requests/sec:  80573.95
Transfer/sec:     12.83MB
+ wrk -c 1000 -d 10s http://localhost:8080/static/10
Running 10s test @ http://localhost:8080/static/10
  2 threads and 1000 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency    12.93ms    8.69ms 226.83ms   77.53%
    Req/Sec    33.95k     3.69k   46.03k    73.85%
  675091 requests in 10.07s, 114.60MB read
Requests/sec:  67012.88
Transfer/sec:     11.38MB
+ wrk -c 1000 -d 10s http://localhost:8080/static/1k
Running 10s test @ http://localhost:8080/static/1k
  2 threads and 1000 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency    14.57ms   16.58ms 392.76ms   96.75%
    Req/Sec    35.41k     2.09k   38.31k    85.35%
  705432 requests in 10.08s, 787.12MB read
Requests/sec:  70010.43
Transfer/sec:     78.12MB
+ wrk -c 1000 -d 10s http://localhost:8080/static/10k
Running 10s test @ http://localhost:8080/static/10k
  2 threads and 1000 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency    14.68ms    8.44ms 282.04ms   78.79%
    Req/Sec    32.07k     3.20k   42.02k    80.20%
  637054 requests in 10.10s, 6.03GB read
Requests/sec:  63074.95
Transfer/sec:    611.82MB

@dlang-bot dlang-bot added the WIP label Dec 20, 2017
@s-ludwig
Copy link
Member

Wow, I'm surprised that this has such an effect, I never saw this in a profiler output (in constrast to two other factors that have a huge optimization potential). Two things that are currently still wrong in the changed code: The code uses Clock.currTime() (which, if called without an UTC() argument, actually used to be rather expensive) instead of the time argument, and the result is cached potentially until after the life time of the underlying allocator, which is logically created and destroyed per request, so that it could lead to use-after-free errors.

Maybe the speedup is simply based on the fact that it saves the allocator calls (should be cheap unless the first memory pool gets exhausted)? In that case another possibility would be to use a FixedAppender stored in HTTPServerResponse, like it's done with m_contentLengthBuffer in HTTPClientRequest.

@kubo39
Copy link
Contributor Author

kubo39 commented Dec 20, 2017

Thank you for your review!

The code uses Clock.currTime() (which, if called without an UTC() argument, actually used to be rather expensive) instead of the time argument, and the result is cached potentially until after the life time of the underlying allocator, which is logically created and destroyed per request, so that it could lead to use-after-free errors.

Exactly. I think that it seems better to call clock_gettime directly, without SysTime. (Ooops, I never considered portability.)

In that case another possibility would be to use a FixedAppender stored in HTTPServerResponse, like it's done with m_contentLengthBuffer in HTTPClientRequest.

Using FixedAppender!(string, 29) seems good, I want to investigate more.

@MartinNowak MartinNowak removed the WIP label Feb 28, 2018
@wilzbach wilzbach removed the WIP label Apr 2, 2018
@wilzbach wilzbach added this to the 0.8.4 milestone Apr 2, 2018
@wilzbach wilzbach modified the milestones: 0.8.4, 0.8.5 Jul 19, 2018
@kubo39 kubo39 changed the title [WIP] Implement cache mechanism for GMT date strings. Implement cache mechanism for GMT date strings. Jul 26, 2019
@kubo39
Copy link
Contributor Author

kubo39 commented Jul 26, 2019

Old LDC with libevent or libasync causes segmentation fault, I'm not sure why.

@kubo39
Copy link
Contributor Author

kubo39 commented Sep 7, 2019

Update benchmark result.

kubo39 added a commit to kubo39/vibe-http that referenced this pull request Sep 9, 2019
Caching the rendering of the Date header for performance.
This is ported from vibe-d/vibe.d#2012
kubo39 added a commit to kubo39/vibe-http that referenced this pull request Sep 9, 2019
Caching the rendering of the Date header for performance.
This is ported from vibe-d/vibe.d#2012
@kubo39
Copy link
Contributor Author

kubo39 commented Sep 30, 2019

ping?

Copy link
Contributor

@Geod24 Geod24 left a comment

Choose a reason for hiding this comment

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

Just a couple nits, ping me when addressed

http/vibe/http/server.d Outdated Show resolved Hide resolved
http/vibe/http/server.d Outdated Show resolved Hide resolved
http/vibe/http/server.d Outdated Show resolved Hide resolved
@kubo39
Copy link
Contributor Author

kubo39 commented Sep 21, 2021

@Geod24 fixed and all tests are passed.

@Geod24 Geod24 merged commit f6f020b into vibe-d:master Sep 23, 2021
@kubo39 kubo39 deleted the date branch September 23, 2021 10:43
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.

None yet

6 participants