perf: JsrtGet/Set property from JsValueRef and more.

[obastemur]

Using Jsrt, there is no direct way to use a JavascriptString object for property identifier.

expensive loop : Get the string from JsValueRef -> Convert to Utf16 -> Lookup / convert to JsPropertyRef... -> etc.. and finally Get OR Set the property!

So.. If we had an interface that accepts JsValueRef, we wouldn't have to do all these stuff.
This alone should change the things for node-chakracore. (AcmeAir perf wise.)

However, I would like to take it one step further and implement a string type that is very similar to Js::PropertyString with some differences.

Js::JsrtString (or however we are going to call)

Structure-wise, apart from being a regular JavascriptString... it will be holding a pointer to PropertyRecord + another pointer for extension / Jsrt call caching purposes. (Memory wise, it will cost +16 bytes on top of standard JavascriptString)

PropertyRecord ptr by default will be nullptr... It will be filled, once thisJsrtString object is used for any property related purposes. Additional usages will be saving from string TO propertyId hashing / lookup... (name it!) And... from GC point.. Yes, as long as this string object is alive, PropertyRecord will be alive too.

JsrtCreateString... will create this new JsrtString by default. (In case the string length is bigger than 128 char, it will create the good old JavascriptString instead.`

** Q/A **

• Why not using Js::PropertyString?
  That would cost +32 bytes. It also expects every string variable to be property string... and will ask for PropertyRecord from the beginning... etc. ( <- some cache related stuff )

• Expected gain?
  Somewhere between 2% to 10% AcmeAIR LTO gain.

• 2% or 10% ? How did you calculate this?
  Average AcmeAIR benchmark request forces the above mentioned "expensive loop" aprox. 50+ times.. In other words, ~80 times each milliseconds. (considering 1730 requests/second rate on the machine I do the benchmark)...

1 ms ==> 2.7Ghz => 2.7 e6 ticks (aprox CPU hertz) / 80times == > So... per each 33750 cpu ticks, we do one of these heavy loops. One loop costs somewhere between 2% to 10% of the available cpu ticks... Hence, this work may end-up giving 2% gain or something around 10%... or less...

• How much change needed on Chakrashim end?
  Not much.

• Chakracore?
  Not much.

I will start hacking soon on this.

Opinions? Objections?

[MikeHolman]

I ran into similar issues with concat strings that are used in property accesses.

For this, I added a LiteralStringWithPropertyStringPtr.It is just a LiteralString with an extra (initially null) pointer field at the end. In case the string is used for property access, we will find/create a PropertyString and fill that field.

It seems like this should fit your needs?

Caveats:
This isn't totally ideal as it causes duplication of the string buffer and an extra indirection to get at the cache, so I'd be happy to see improvements. A number of the JIT and runtime paths for property access are already aware of this, but I wasn't exhaustive, so it is possible you are interested in a path I didn't cover.

[obastemur]

@MikeHolman LiteralStringWithPropertyStringPtr looks promising for what I need. Besides, If I'm not mistaking.. it comes with an extra unused 8 bytes sizeof(LiteralStringWithPropertyStringPtr) == 40 that I can use it for Jsrt front caching purposes (experiment.. may not show up at v1 or ever)

I'm already looking into it. Thanks for pointing this out!

[obastemur]

If I'm not mistaking.. it comes with an extra unused 8 bytes sizeof(LiteralStringWithPropertyStringPtr) == 40 that I can use....

Noticed the ConvertString calls depend on source string ptr size. Not sure if this is going to be an issue.

This isn't totally ideal as it causes duplication of the string buffer and an extra indirection to get at the cache...

https://github.com/Microsoft/ChakraCore/blob/master/lib/Backend/Lower.cpp#L15254

This looks like the only direct use of PropertyString instead of a PropertyRecord

[obastemur]

I'm also planing to wipe JsPropertyId and related stuff from JSRT. Hopefully I'm not missing but I can't see any reason to keep it around?

[obastemur]

JsGetIndexedProperty etc. are also redundant.

CHAKRA_API JsGetIndexedProperty(_In_ JsValueRef object, _In_ JsValueRef index, _Out_ JsValueRef *result)

See JsValueRef index ... well, why two JSRT API methods, taking the same args and doing the same thing?

[obastemur]

Implemented.