-
-
Notifications
You must be signed in to change notification settings - Fork 275
feat: Implement FastString
- a potential zero-copy jsi::String
#806
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: feat/rn-78
Are you sure you want to change the base?
Conversation
package/cpp/UTFConversion.h
Outdated
cp = 0xFFFD; // Invalid surrogate pair. | ||
} | ||
} else { | ||
cp = x; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could also be the first part of an incomplete surrogate pair, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tbh I don't understand the full code in all of it's glory, ChatGPT wrote most of it. I think for a release version I might use a maintained UTF library instead..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The conversion code looks pretty good to me actually, except a couple of minor things like this. BTW, why is the entire function in the header file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea - is that a bad idea? Thought it might be better for inlining. Only used in one place anyways
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should get linker errors unless you declare it as inline
, I am surprised you haven't. I don't think this one particularly benefits from being inlined.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yea - I mean it's only used in a single translation unit. But I moved it to .cpp now anyways
package/cpp/UTFConversion.h
Outdated
uint16x8_t ge_d800 = vcgeq_u16(vec, v_d800); | ||
uint16x8_t le_dfff = vcleq_u16(vec, v_dfff); | ||
uint16x8_t surrogate = vandq_u16(ge_d800, le_dfff); | ||
if (vmaxvq_u16(surrogate) != 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this available on armv7 NEON?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It indeed isn't. Need to implement manually
Hey, I can't help but notice that there are 100k calls to |
Hey nice to see you around here! Yea there are 100k instead of 1 call, it was very late when I built this and I messed it up, lol. |
FastString
is a highly optimized C++ string wrapper that either holds astd::string_view
to raw, zero-copied data (FAST) - or astd::string
(slower fallback path).This is possible because react-native 0.78 introduced a new JSI API -
jsi::String::getStringData(...)
- which allows us to unsafely get the String's raw data without a copy - hencechar*
/std::string_view
.Because the runtime might choose to implement things differently, we don't know whether we're going to get one part or multiple parts - and it can be UTF8 or UTF16. So we need fallbacks (slow paths) for those cases. That's where
std::string
comes into play.Benchmarks
Performance varies - some strings can be zero-copy optimized (UTF8), some can't (UTF16).
So if you use emojis or special characters, your string is gonna be a bit slower.
This Benchmarks 100k calls to
set
, 100k calls togetAllKeys()
, and 100k calls togetString()
. So 300.000 JSI method calls in total, out of which all of them make use ofFastString
.UTF8
5 Characters
2.500 Characters
55.000 Characters
990.000 Characters
UTF16
5 Characters
5.000 Characters
Mixed strings with a real world app
Only
jsi::String::utf8(..)
vsFastString
jsi::String::utf8(...)
: 547msFastString
: 86ms (84% faster)Isolated benchmark for UTF8 fast path - that's where it really shines.
Findings
jsi::String
conversion is only a small piece of the puzzle - it just makes up for ~4% of the total execution time in best cases.jsi::Function
call, not the String conversion.