Skip to content

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

Open
wants to merge 10 commits into
base: feat/rn-78
Choose a base branch
from

Conversation

mrousavy
Copy link
Owner

@mrousavy mrousavy commented Feb 16, 2025

FastString is a highly optimized C++ string wrapper that either holds a std::string_view to raw, zero-copied data (FAST) - or a std::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 - hence char*/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 to getAllKeys(), and 100k calls to getString(). So 300.000 JSI method calls in total, out of which all of them make use of FastString.

const storage = new MMKV();
storage.clearAll();

let string = /* STRING GOES HERE */

const start = performance.now();
for (let i = 0; i < 100_000; i++) {
  storage.set('dummy1', string);
  storage.getAllKeys().map((k) => storage.getString(k));
}
const end = performance.now();
console.log(`Benchmark took ${(end - start).toFixed(0)}ms`);

UTF8

5 Characters

  • Before: 114ms
  • After: 109ms (4% faster)

2.500 Characters

  • Before: 157ms
  • After: 150ms (4% faster)

55.000 Characters

  • Before: 1.888ms
  • After: 1.624ms (14% faster)

990.000 Characters

  • Before: 27.132ms
  • After: 26.089ms (4% faster)

UTF16

5 Characters

  • Before: 128ms
  • After: 126ms (1% faster)

5.000 Characters

  • Before: 7.853ms
  • After: 4.187ms (46% faster)

Mixed strings with a real world app

  • Before: 1.997ms
  • After: 1.707ms (16% faster)

Only jsi::String::utf8(..) vs FastString

  • jsi::String::utf8(...): 547ms
  • FastString: 86ms (84% faster)

Isolated benchmark for UTF8 fast path - that's where it really shines.

Findings

  1. Apparently the 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.
  2. Surprisingly, my UTF16 -> UTF8 conversion is ~40-50% faster than the one by JSI/Hermes. I quickly hacked it together with ChatGPT, but maybe the part that uses NEON/SIMD is faster? Looks like output data is correct too.
  3. I honestly expected more improvements in the UTF8 fast branch for huge strings (like JSON data). But apparently the main overhead is the jsi::Function call, not the String conversion.

@mrousavy mrousavy changed the base branch from main to feat/rn-78 February 16, 2025 21:04
cp = 0xFFFD; // Invalid surrogate pair.
}
} else {
cp = x;
Copy link

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?

Copy link
Owner Author

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..

Copy link

@tmikov tmikov Feb 17, 2025

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?

Copy link
Owner Author

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

Copy link

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.

Copy link
Owner Author

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

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) {
Copy link

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?

Copy link
Owner Author

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

@lingol
Copy link

lingol commented Feb 20, 2025

Hey, I can't help but notice that there are 100k calls to getAllKeys() instead of one call. Or did I miss something in JS that handles map() differently?

@mrousavy
Copy link
Owner Author

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.
A friend already told me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants