tailcallhq / tailcall

High Performance GraphQL Runtime
https://tailcall.run
Apache License 2.0
1.3k stars 257 forks source link

perf: use serde_json_borrow for Synth output #3073

Closed meskill closed 1 week ago

meskill commented 1 month ago

Summary:
Replace using only async_graphql_value::ConstValue with serde_json_borrow::Value for the Sync output. Evaluation and store left intact and that uses ConstValue as before.

Issue Reference(s):
Fixes #... (Replace "..." with the issue number)

Build & Testing:

Checklist:

meskill commented 1 month ago

Wrk changes

posts title

before

Running 30s test @ http://localhost:8000/graphql
  4 threads and 100 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency     2.47ms   13.49ms 360.82ms   99.24%
    Req/Sec    17.39k     1.85k   22.15k    86.81%
  2075331 requests in 30.09s, 10.40GB read
Requests/sec:  68973.83
Transfer/sec:    354.02MB

serde_json_borrow

Running 30s test @ http://localhost:8000/graphql
  4 threads and 100 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency     2.50ms   14.78ms 397.01ms   99.22%
    Req/Sec    18.43k     2.19k   21.42k    89.15%
  2203809 requests in 29.23s, 11.05GB read
  Socket errors: connect 0, read 0, write 0, timeout 100
Requests/sec:  75406.44
Transfer/sec:    387.04MB

big query

before

Running 30s test @ http://localhost:8000/graphql
  4 threads and 100 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency     9.46ms    4.02ms 100.68ms   85.24%
    Req/Sec     2.70k   332.08     5.14k    86.24%
  322219 requests in 30.09s, 15.94GB read
Requests/sec:  10708.26
Transfer/sec:    542.35MB

serde_json_borrow

Running 30s test @ http://localhost:8000/graphql
  4 threads and 100 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency     9.07ms    3.22ms  90.35ms   77.74%
    Req/Sec     2.79k   323.80     3.60k    82.50%
  333021 requests in 30.08s, 16.47GB read
Requests/sec:  11072.28
Transfer/sec:    560.79MB
meskill commented 1 month ago

Flamegraphs

posts title

insert_key from 4.44% to 1.3%

image

big query

insert_key from 4.96% to 0.94%

image

meskill commented 1 month ago

The performance difference is noticeable, but for the big queries the impact is lower showing only around 3% improvement.

And this pr needs several more fixes to work because right now it's only possible to run the benchmarks and tests are failing:

github-actions[bot] commented 1 month ago

🐰 Bencher Report

Branch3073/merge
Testbedbenchmarking-runner

⚠️ WARNING: No Threshold found!

Without a Threshold, no Alerts will ever be generated.

Click here to create a new Threshold
For more information, see the Threshold documentation.
To only post results if a Threshold exists, set the --ci-only-thresholds CLI flag.

Click to view all benchmark results
BenchmarkLatencynanoseconds (ns)
from_json_bench📈 view plot
⚠️ NO THRESHOLD
6,226,600.00
group_by📈 view plot
⚠️ NO THRESHOLD
483.15
input/args.missing📈 view plot
⚠️ NO THRESHOLD
22.10
input/args.nested.existing📈 view plot
⚠️ NO THRESHOLD
41.22
input/args.nested.missing📈 view plot
⚠️ NO THRESHOLD
35.48
input/args.root📈 view plot
⚠️ NO THRESHOLD
37.55
input/headers.existing📈 view plot
⚠️ NO THRESHOLD
30.62
input/headers.missing📈 view plot
⚠️ NO THRESHOLD
29.85
input/value.missing📈 view plot
⚠️ NO THRESHOLD
21.95
input/value.nested.existing📈 view plot
⚠️ NO THRESHOLD
41.44
input/value.nested.missing📈 view plot
⚠️ NO THRESHOLD
36.17
input/value.root📈 view plot
⚠️ NO THRESHOLD
41.55
input/vars.existing📈 view plot
⚠️ NO THRESHOLD
8.09
input/vars.missing📈 view plot
⚠️ NO THRESHOLD
10.67
synth_nested📈 view plot
⚠️ NO THRESHOLD
103,120.00
synth_nested_borrow📈 view plot
⚠️ NO THRESHOLD
39,614.00
test_batched_body📈 view plot
⚠️ NO THRESHOLD
1,859.40
test_batched_body #2📈 view plot
⚠️ NO THRESHOLD
1,517,900.00
test_data_loader📈 view plot
⚠️ NO THRESHOLD
391,500.00
test_handle_request📈 view plot
⚠️ NO THRESHOLD
128,840.00
test_handle_request_jit📈 view plot
⚠️ NO THRESHOLD
51,684.00
test_http_execute_method📈 view plot
⚠️ NO THRESHOLD
15,637.00
with_mustache_expressions📈 view plot
⚠️ NO THRESHOLD
1,111.50
with_mustache_literal📈 view plot
⚠️ NO THRESHOLD
695.59
🐰 View full continuous benchmarking report in Bencher
meskill commented 1 month ago

The performance difference is noticeable, but for the big queries the impact is lower showing only around 3% improvement.

And this pr needs several more fixes to work because right now it's only possible to run the benchmarks and tests are failing:

  • implement clone_from for case of object. The clone_from is introduced to be able to clone from one JsonLike to another JsonLike
  • implement merge for instrospection query because we have different JsonLike entries now and it should be merged differently like it was with ConstValue
  • fix the tests related to changes in exec signature

fixed in recent commits

codecov[bot] commented 1 month ago

Codecov Report

Attention: Patch coverage is 65.47085% with 77 lines in your changes missing coverage. Please review.

Project coverage is 86.74%. Comparing base (c9bef7a) to head (a48b129). Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/core/json/serde.rs 9.09% 30 Missing :warning:
src/core/json/json_like.rs 46.15% 21 Missing :warning:
src/core/json/borrow.rs 60.60% 13 Missing :warning:
src/core/json/graphql.rs 81.48% 5 Missing :warning:
src/core/jit/exec_const.rs 82.60% 4 Missing :warning:
src/core/jit/synth/synth.rs 93.18% 3 Missing :warning:
src/core/jit/response.rs 94.11% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #3073 +/- ## ========================================== - Coverage 86.89% 86.74% -0.16% ========================================== Files 275 275 Lines 26896 27052 +156 ========================================== + Hits 23372 23466 +94 - Misses 3524 3586 +62 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.


🚨 Try these New Features:

github-actions[bot] commented 1 week ago

Action required: PR inactive for 5 days. Status update or closure in 10 days.