Rework promise rejection tracker #1058
Conversation
|
Sorry for the back and forth here, I got blinsided. |
| @@ -51060,7 +51016,8 @@ static __exception int perform_promise_then(JSContext *ctx, | |||
| if (s->promise_state == JS_PROMISE_REJECTED && !s->is_handled) { | |||
| JSRuntime *rt = ctx->rt; | |||
| 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. |
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