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

perf(core): rewrite event loop #16377

Closed
wants to merge 12 commits into from

Conversation

littledivy
Copy link
Member

@littledivy littledivy commented Oct 21, 2022

This PR removes the JS promise ID -> promise mapping used to represent promises in Rust. Also gets rid of batching results to opresolve by calling the resolver function handler directly from Rust.

A promise is created in JS and the resolver function is passed to the op. The op grabs a global reference of the resolver and keeps it around for the duration of the pending future. In JsRuntime::resolve_async_ops, we call the resolver directly with the result.

Benchmarks

`crypto.subtle.digest`
# this patch
time 967 ms rate 103412
time 1031 ms rate 96993
time 942 ms rate 106157
time 937 ms rate 106723
time 935 ms rate 106951
time 944 ms rate 105932
# main
time 1158 ms rate 86355
time 1203 ms rate 83125
time 1061 ms rate 94250
time 1066 ms rate 93808
time 1069 ms rate 93545
time 1067 ms rate 93720

this patch:

sha256_new

main:
sha256_main

`writeFile`
# main

time 185 ms rate 54054
time 167 ms rate 59880
time 167 ms rate 59880
time 165 ms rate 60606
time 163 ms rate 61349
time 165 ms rate 60606
time 170 ms rate 58823
# this patch

time 139 ms rate 71942
time 143 ms rate 69930
time 144 ms rate 69444
time 144 ms rate 69444
time 143 ms rate 69930
time 143 ms rate 69930
time 145 ms rate 68965

write_file_new
write_file_main

Cache API
# this patch

benchmark                  time (avg)             (min … max)       p75       p99      p995
------------------------------------------------------------- -----------------------------
cache_storage_open      22.19 µs/iter  (18.58 µs … 303.42 µs)     23 µs  35.04 µs  40.54 µs
cache_storage_has       14.75 µs/iter   (10.79 µs … 92.58 µs)  15.54 µs     33 µs  43.12 µs
cache_storage_delete    20.07 µs/iter  (15.08 µs … 571.29 µs)     20 µs  48.17 µs  62.54 µs
cache_put_body_10_KiB   18.39 ms/iter    (2.44 ms … 20.23 ms)  19.87 ms  20.23 ms  20.23 ms
cache_put_no_body       45.96 µs/iter     (34.17 µs … 1.8 ms)   45.5 µs  92.62 µs 132.38 µs
cache_match             46.21 µs/iter   (38.46 µs … 10.05 ms)  45.58 µs     77 µs  89.62 µs
cache_delete            17.89 µs/iter    (15.46 µs … 1.14 ms)  18.17 µs  27.46 µs  30.88 µs
# main

benchmark                  time (avg)             (min … max)       p75       p99      p995
------------------------------------------------------------- -----------------------------
cache_storage_open      23.49 µs/iter  (19.54 µs … 309.58 µs)  24.46 µs  41.62 µs  43.04 µs
cache_storage_has       15.94 µs/iter     (8.5 µs … 97.33 µs)     17 µs   24.5 µs  26.54 µs
cache_storage_delete    20.69 µs/iter  (10.42 µs … 108.04 µs)  22.12 µs  30.33 µs  32.33 µs
cache_put_body_10_KiB   19.61 ms/iter   (18.69 ms … 20.78 ms)  20.01 ms  20.78 ms  20.78 ms
cache_put_no_body       48.15 µs/iter  (36.29 µs … 961.46 µs)  46.62 µs 119.08 µs 143.29 µs
cache_match             46.45 µs/iter    (38.92 µs … 8.15 ms)  46.75 µs  62.33 µs  73.75 µs
cache_delete             21.5 µs/iter    (12.62 µs … 1.04 ms)  22.17 µs  40.75 µs  52.88 µs
Batching 10 byte `writeFile`s (100_000 promises)
# main
time 743 ms rate 134589
time 729 ms rate 137174
time 804 ms rate 124378
time 773 ms rate 129366
time 952 ms rate 105042
time 823 ms rate 121506
time 783 ms rate 127713
time 783 ms rate 127713
time 790 ms rate 126582
# this patch
time 727 ms rate 137551
time 790 ms rate 126582
time 772 ms rate 129533
time 796 ms rate 125628
time 726 ms rate 137741
time 752 ms rate 132978
time 736 ms rate 135869
time 772 ms rate 129533
time 783 ms rate 127713
time 769 ms rate 130039

core/01_core.js Outdated
}
let p = newPromise();
// Save the id on the promise so it can later be ref'ed or unref'ed
p[promiseIdSymbol] = promiseId;
Copy link
Contributor

Choose a reason for hiding this comment

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

The promise ID symbol has to be set on the promise returned by opAsync, not on the intermediate promise, since otherwise the ID is not available to extensions that need to use {un,}refOp.

@littledivy littledivy changed the title perf(core): remove promise ring perf(core): rewrite event loop Oct 24, 2022
@littledivy
Copy link
Member Author

Most of the changes have landed in main (except the promise ring removal)

@littledivy littledivy closed this Nov 10, 2022
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.

2 participants