search

isaacs / node-primordials

https://isaacs.github.io/node-primordials/
11 stars 3 forks source link

Use `ReflectApply` instead of bind+call #2

Open aduh95 opened 1 year ago

aduh95 commented 1 year ago

It should perform better, and is also more readable imho.

isaacs commented 1 year ago

Hm, you're probably right. Going to hold off on landing this until it's feature complete, though. At that point, the next painful step will be porting path-scurry to use it, to see just how bad the perf impact is.

I may end up essentially just making all of these things fn.apply(thisp, args) anyway (which is imo even more readable and likely more performant than ReflectApply, albeit not hardened at all).

isaacs commented 1 year ago

The reason for that weird idiom is because I'm trying (for now) to mirror node's implementation as closely as possible, with the main difference that the exports are unrolled for the benefit of TypeScript.

aduh95 commented 1 year ago

Reflect.apply is ok perf wise IIRC, on par with fn.apply. It’s slower if you have to allocate an array, but in these cases the array is already there.