search

nodejs / node-chakracore

Node.js on ChakraCore :sparkles::turtle::rocket::sparkles:
Other
1.92k stars 342 forks source link

Clean up usage of chakra_shim.js #484

Open jackhorton opened 6 years ago

jackhorton commented 6 years ago

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.

kfarnung commented 6 years ago

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.

jackhorton commented 6 years ago

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.