http: refactor agent.destroy()#28596
Uh oh!
There was an error while loading. Please reload this page.
Conversation
This change refactor the previous triple-nested loop and make it more readable by using iterators.
Uh oh!
There was an error while loading. Please reload this page.
I'm guessing this probably doesn't need a benchmark but mentioning it in case I'm wrong about that. |
Trott commented Jul 9, 2019
@nodejs/http |
Trott commented Jul 9, 2019
@mscdex ^^^^^ |
starkwang commented Jul 9, 2019
1 similar comment
nodejs-github-bot commented Jul 9, 2019
lib/_http_agent.js Outdated
| }; | ||
| functiondestroySockets(sockets){ | ||
| for(constsetKeyinsockets){ |
There was a problem hiding this comment.
briefer:
| for(constsetKeyinsockets){ | |
| for(constsocketofObject.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
| }; | ||
| functiondestroySockets(sockets){ | ||
| for(constsetKeyinsockets){ |
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.
starkwangforce-pushed the refactor-http-agent-destory branch from 9430bf2 to 73da794Comparestarkwang commented Jul 15, 2019
@BridgeAR Could you take another look? : ) |
BridgeAR commented Jan 12, 2020
This seems to be superseded by #30958. |
10 participants
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