http: refactor agent.destroy() #28596
Conversation
This change refactor the previous triple-nested loop and make it more readable by using iterators.
|
I'm guessing this probably doesn't need a benchmark but mentioning it in case I'm wrong about that. |
|
@nodejs/http |
@mscdex ^^^^^ |
1 similar comment
lib/_http_agent.js
Outdated
| }; | ||
|
|
||
| function destroySockets(sockets) { | ||
| for (const setKey in sockets) { |
There was a problem hiding this comment.
briefer:
| for (const setKey in sockets) { | |
| for (const socket of Object.values(sockets).flat()) { |
There was a problem hiding this comment.
But probably also much less performant.
There was a problem hiding this comment.
in changes the behavior though. Before, the prototype would not be checked but now it does.
I do not know why flat was suggested. It should already be a flat array?
I think in this specific case it should be fine to use Object.values, since it should not be a hot path to destroy the sockets.
lib/_http_agent.js
Outdated
| }; | ||
|
|
||
| function destroySockets(sockets) { | ||
| for (const setKey in sockets) { |
There was a problem hiding this comment.
in changes the behavior though. Before, the prototype would not be checked but now it does.
I do not know why flat was suggested. It should already be a flat array?
I think in this specific case it should be fine to use Object.values, since it should not be a hot path to destroy the sockets.
starkwang
force-pushed
the
refactor-http-agent-destory
branch
from
9430bf2 to
73da794
Compare
|
@BridgeAR Could you take another look? : ) |
|
This seems to be superseded by #30958. |
This change refactor the previous triple-nested loop and make it more readable by using iterators.
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes