fix: resolve event-loop blocking when aborting a fetch#2660
fix: resolve event-loop blocking when aborting a fetch#2660Uzlopak wants to merge 1 commit intonodejs:mainfrom
Conversation
| const markResourceTiming = (nodeMajor > 18 || (nodeMajor === 18 && nodeMinor >= 2)) | ||
| ? performance.markResourceTiming | ||
| : () => {} |
There was a problem hiding this comment.
This showed hot in the flame graphs, low hanging fruit.
| for (const client of this[kClients]) { | ||
| if (!client[kNeedDrain]) { | ||
| return client | ||
| } | ||
| } | ||
|
|
||
| if (!this[kConnections] || this[kClients].length < this[kConnections]) { | ||
| dispatcher = this[kFactory](this[kUrl], this[kOptions]) | ||
| const dispatcher = this[kFactory](this[kUrl], this[kOptions]) | ||
| this[kAddClient](dispatcher) | ||
| return dispatcher |
There was a problem hiding this comment.
this was also kind of slow while benchmarking. Low hanging fruit
| function abortFetch (p, request, responseObject, error) { | ||
| // 1. Reject promise with error. | ||
| p.reject(error) | ||
| setImmediate(() => p.reject(error)) |
There was a problem hiding this comment.
This shouldn't be necessary according to spec. Are we sure this is a spec bug and not something else?
There was a problem hiding this comment.
Something is clogging the event loop. As I see other tests fail. So this is not the solution for the issue.
calling setImmediate resulted in declogging it. Still investigating.
There was a problem hiding this comment.
I think this is an issue in v8, I vaguely recall an issue in node core where promises weren't being GC'ed in a loop
| // https://w3c.github.io/resource-timing/#dfn-mark-resource-timing | ||
| function markResourceTiming (timingInfo, originalURL, initiatorType, globalThis, cacheState) { | ||
| if (nodeMajor > 18 || (nodeMajor === 18 && nodeMinor >= 2)) { | ||
| performance.markResourceTiming(timingInfo, originalURL.href, initiatorType, globalThis, cacheState) |
There was a problem hiding this comment.
originalURL is being passed instead of the stringified url
ronag
left a comment
ronag
left a comment
There was a problem hiding this comment.
maybe leave optimization to separate PR?
|
Yes it is an upstream issue. |
|
I don't think this needed to be closed. |
|
Lets discuss this first in the issue. I put the other optimizations into a different PR. |
3 participants
Resolves #2198
This relates to...
Rationale
Changes
Features
Bug Fixes
Breaking Changes and Deprecations
Status