node-chakracore icon indicating copy to clipboard operation
node-chakracore copied to clipboard

Clean up usage of chakra_shim.js

Open jackhorton opened this issue 7 years ago • 2 comments

chakra_shim.js contains implementations of lots of functions that we should probably consider moving back into native, such as hot type checks. The interface to access it is also pretty messy and has lots of repetition in jsrtcontextshim.cc, v8value.cc, and others.

jackhorton avatar Mar 07 '18 01:03 jackhorton

I think this is probably worthwhile to look at the interfaces, but are we sure that it will actually improve performance to move the code back to native? Even some of the simpler looking functions in chakra_shim.js require multiple native function calls into the engine to get equivalent behavior.

Fundamentally I'm +1 for trying to improve the interface, but I'm -0 on moving code back to native unless there's a clear performance or maintenance win in doing so.

kfarnung avatar Mar 07 '18 07:03 kfarnung

We have a lot of type checks that, if moved to the JSRT, would be a single (inlineable by the C++ compiler) native call to check the typeId of the Var. Currently we call out to a 1 line JS function each time that calls Object.prototype.toString on the object and makes a concat string and a === check... it seems like a lot of unnecessary work. With that being said, no, we haven't ever seen this come up specifically in perf investigations.

jackhorton avatar Mar 07 '18 16:03 jackhorton