Conversation
|
Sorry for the back and forth here, I got blinsided. |
bnoordhuis
left a comment
bnoordhuis
left a comment
There was a problem hiding this comment.
rejected_promise_list is scanned in linear fashion. How big is it expected to grow under normal operations?
I understand in-flight promises are added and removed continuously so I suppose it can grow pretty big in a promise-heavy script?
| if (rt->host_promise_rejection_tracker) | ||
| JS_EnqueueJob(ctx, promise_rejection_tracker_job, 1, &promise); | ||
| rt->host_promise_rejection_tracker(ctx, promise, s->promise_result, | ||
| true, rt->host_promise_rejection_tracker_opaque); |
There was a problem hiding this comment.
Why is is_handled==true? I would've expected false.
There was a problem hiding this comment.
Because here we are attaching a catch AFAIK.
Not sure, I guess I can measure? Any specific test you'd like to run?
Maybe? Any particular worries? |
This reverts a75498b and a4a5a17 (except the tests) and adapts bellard/quickjs@3c39307 which also passes the tests and feels like a better solution overall. Ref: #1038
saghul
force-pushed
the
rework-promise-rejection-tracker
branch
from
fb80890 to
6fddfe8
Compare
|
I don't think we have tests or benchmarks (neither does web-tooling-benchmark) that really stress-tests promises. I'm thinking of a modern node.js server app, they commonly have 100s or 1,000s of promises in-flight at any given moment. |
|
Right. Note that we only really keep track of rejected ones so the rest of the lookups should be fast? Unless you have a ton of rejected ones... Are you ok landing as is and looking into perf improvements if / when necessary? |
|
Yeah, that's fine. Just something to be mindful of. |
2 participants
This reverts a75498b and a4a5a17 (except the tests) and adapts bellard/quickjs@3c39307 which also passes the tests and feels like a better solution overall.
Ref: #1038