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

Remove Eager read, write, accept #1959

Merged
merged 1 commit into from
Mar 18, 2019
Merged

Remove Eager read, write, accept #1959

merged 1 commit into from
Mar 18, 2019

Conversation

ry
Copy link
Member

@ry ry commented Mar 18, 2019

The eager code is very complicated for very little benefit. By removing it, tail latency (which is VERY BAD currently) is reduced by half. Absolute req/sec is slightly slower tho - I still think it makes sense to remove.

Here I'm running ./target/release/deno tests/http_bench.ts -A

Without eager (this branch)

~/src/deno> third_party/wrk/mac/wrk -d 10 -c 10 http:https://127.0.0.1:4500/
Running 10s test @ http:https://127.0.0.1:4500/
  2 threads and 10 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency     5.90ms   29.21ms 425.93ms   95.69%
    Req/Sec    16.55k     3.08k   22.18k    83.17%
  332652 requests in 10.10s, 16.18MB read
Requests/sec:  32931.55
Transfer/sec:      1.60MB

With eager (master)

~/src/deno> third_party/wrk/mac/wrk -d 10 -c 10 http:https://127.0.0.1:4500/
Running 10s test @ http:https://127.0.0.1:4500/
  2 threads and 10 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency    26.06ms  122.80ms   1.35s    95.18%
    Req/Sec    17.46k     3.01k   24.06k    76.00%
  347308 requests in 10.00s, 16.89MB read
Requests/sec:  34722.65
Transfer/sec:      1.69MB

Furthermore this suggests that maybe the tail latency problems experienced in deno_core_http_bench are due to an over-eager event loop (the while loop in deno_core::Isolate::poll())

This is no longer a perf improvement
@ry ry requested a review from piscisaureus March 18, 2019 22:11
@ry ry merged commit 34a2aa4 into denoland:master Mar 18, 2019
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

2 participants