node
node copied to clipboard
lib: reduce url getters on `makeRequireFunction`
Reduce calling getters of URL due to the lazy loading of URL.
Ref: https://github.com/nodejs/performance/issues/92
Review requested:
- [ ] @nodejs/loaders
- [ ] @nodejs/modules
CI: https://ci.nodejs.org/job/node-test-pull-request/52298/
CI: https://ci.nodejs.org/job/node-test-pull-request/52300/
CI: https://ci.nodejs.org/job/node-test-pull-request/52314/
Thank you for this!
I gather from this and related PRs that URL objects in general are more costly than primitive strings? Should we make an effort to try to scrub the modules code of URL objects whenever possible, converting to strings instead?
@GeoffreyBooth URL object is not costly, but accessing their attributes except href is costly. For example, calling url.protocol multiple times, will create multiple different strings, rather than the same one. This is caused by the lazy getters of the class. I'm currently trying to reduce URL initializations and multiple getters of the same URL instance across the loaders.
For example, calling
url.protocolmultiple times, will create multiple different strings, rather than the same one.
I would think it should be possible to “cache” protocol in whatever way that href is currently stored? So that calling protocol repeatedly is no more costly than calling href, assuming the latter is “inexpensive” (in the sense that it’s not worth trying to refactor to something even simpler).
I would think it should be possible to “cache”
protocolin whatever way thathrefis currently stored? So that callingprotocolrepeatedly is no more costly than callinghref, assuming the latter is “inexpensive” (in the sense that it’s not worth trying to refactor to something even simpler).
This might be an (micro)-optimization worth investigating for, but adding a branch and accessing the cache parameter overhead might be greater than just simply reduce calling it. I simply don't have the data right now to back the claim if one approach is better than another.
CI: https://ci.nodejs.org/job/node-test-pull-request/52326/
CI: https://ci.nodejs.org/job/node-test-pull-request/52333/
CI: https://ci.nodejs.org/job/node-test-pull-request/52346/
Rebased and force-pushed.
CI: https://ci.nodejs.org/job/node-test-pull-request/52400/
Landed in 578ffe1edb3ee2acbe89b9db5555d966209584ba