crysalead-js / sql-dialect

SQL Builder
MIT License
6 stars 2 forks source link

Sanitization and Node optimizations #5

Closed esatterwhite closed 5 years ago

esatterwhite commented 5 years ago

This PR is to make a few general improvements to take better advantage of Node and eliminate and potential performance problems. The test suite was left un-touched to ensure existing functionality was not broken.

The changes that were made include:

jails commented 5 years ago

Awesome ! Sorry for the late reply, I was a bit overbooked lately, I'm merging it right away :+1: Btw what this optimization really means "Make sure property types remain consistent to avoid map transitions / deopts" ?

esatterwhite commented 5 years ago

V8 tries to make Classes for object types, which once created - can't be changed obviously. There were a few spots where properties on the statement classes where things would default to a string, but but be set to an object or array.

when you do that, v8 has to make a different class definition - which is expensive. and if it deems it so, it will de-optimize the code and use a hash map instead of optimized classes. which is really slow.


const obj = {a: "" } // v8 generates a class
obj.a = [{b: ""}] // v8 generates a different class

that kind of thing.

esatterwhite commented 5 years ago

Thanks for merging. Let me know if you notice any odd behaviors. happy to fix

There were a few places where it seemed the deep merge was necessary, and in others it did not look like it was necessary. So i pulled it out. I could have been wrong ;) but the tests passed