Skip to content

Commit 8fd20f2

Browse files
avpfacebook-github-bot
authored andcommitted
Avoid repeated branches while iterating StringView
Summary: `quoteStringForJSON` was operating via indexed access on `StringView`, performing an `isASCII()` check every iteration and leading to slow quoting during `JSON.stringify`. Modify the function to take `UTF16Ref` or `ASCIIRef` directly, so that it knows the type. This increases code size slightly while resulting in significant speedups in `JSON.stringify`. Reviewed By: neildhar Differential Revision: D46117526 fbshipit-source-id: e6b849fc8b4e2e748e05d277e36462b8d97ae095
1 parent 58ac81b commit 8fd20f2

File tree

2 files changed

+30
-13
lines changed

2 files changed

+30
-13
lines changed

include/hermes/Support/JSON.h

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@
1010

1111
#include "hermes/Platform/Unicode/CharacterProperties.h"
1212

13+
#include "llvh/ADT/ArrayRef.h"
14+
1315
namespace hermes {
1416

1517
template <typename Output>
@@ -28,15 +30,15 @@ void appendUTF16Escaped(Output &output, char16_t cp) {
2830
// If there is a valid surrogate pair at position \p i in \p view, then write
2931
// both the high and low surrogate into \p output. Otherwise, write an escaped
3032
// UTF16 value into \p output. \return true if a pair was found.
31-
template <typename Output, typename StringView>
32-
bool handleSurrogate(Output &output, StringView view, size_t i) {
33+
template <typename Output, typename CharT>
34+
bool handleSurrogate(Output &output, llvh::ArrayRef<CharT> view, size_t i) {
3335
char16_t ch = view[i];
3436
assert(
3537
ch >= UNICODE_SURROGATE_FIRST && ch <= UNICODE_SURROGATE_LAST &&
3638
"charcter should be a surrogate character");
3739
// Handle well-formed-ness here: Represent unpaired surrogate code points as
3840
// JSON escape sequences.
39-
if (isHighSurrogate(ch) && i + 1 < view.length()) {
41+
if (isHighSurrogate(ch) && i + 1 < view.size()) {
4042
char16_t next = view[i + 1];
4143
if (isLowSurrogate(next)) {
4244
// We found a surrogate pair. Simply write both of them unescaped to the
@@ -53,16 +55,16 @@ bool handleSurrogate(Output &output, StringView view, size_t i) {
5355
}
5456

5557
/// Quotes a string given by \p view and puts the quoted version into \p output.
56-
/// \p view should be utf16-encoded, and \p output will be as well.
58+
/// \p view must be utf16 or ASCII encoded.
5759
/// \post output is a container that has a sequential list of utf16 characters
5860
/// that can be embedded into a JSON string.
59-
template <typename Output, typename StringView>
60-
void quoteStringForJSON(Output &output, StringView view) {
61+
template <typename Output, typename CharT>
62+
void quoteStringForJSON(Output &output, llvh::ArrayRef<CharT> view) {
6163
// Quote.1.
6264
output.push_back(u'"');
6365
// Quote.2.
64-
for (size_t i = 0; i < view.length(); i++) {
65-
char16_t ch = view[i];
66+
for (size_t i = 0, e = view.size(); i < e; ++i) {
67+
CharT ch = view[i];
6668
#define ESCAPE(ch, replace) \
6769
case ch: \
6870
output.push_back(u'\\'); \
@@ -90,10 +92,19 @@ void quoteStringForJSON(Output &output, StringView view) {
9092
output.push_back(u'a' + (ch % 16 - 10));
9193
}
9294
} else {
93-
if (ch >= UNICODE_SURROGATE_FIRST && ch <= UNICODE_SURROGATE_LAST) {
94-
if (handleSurrogate(output, view, i)) {
95-
// Found a valid surrogate pair, so skip over the next character.
96-
i++;
95+
// This check should only generate code when CharT is more than 1
96+
// byte.
97+
if constexpr (
98+
std::numeric_limits<CharT>::max() >= UNICODE_SURROGATE_FIRST) {
99+
if (ch >= UNICODE_SURROGATE_FIRST && ch <= UNICODE_SURROGATE_LAST) {
100+
if (handleSurrogate(output, view, i)) {
101+
// Found a valid surrogate pair, so skip over the next
102+
// character.
103+
i++;
104+
}
105+
} else {
106+
// Quote.2.d.
107+
output.push_back(ch);
97108
}
98109
} else {
99110
// Quote.2.d.

lib/VM/JSLib/RuntimeJSONUtils.cpp

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -916,7 +916,13 @@ CallResult<bool> JSONStringifyer::operationStr(HermesValue key) {
916916
}
917917

918918
void JSONStringifyer::operationQuote(StringView value) {
919-
quoteStringForJSON(output_, value);
919+
if (value.isASCII()) {
920+
quoteStringForJSON(
921+
output_, ASCIIRef{value.castToCharPtr(), value.length()});
922+
} else {
923+
quoteStringForJSON(
924+
output_, UTF16Ref{value.castToChar16Ptr(), value.length()});
925+
}
920926
}
921927

922928
ExecutionStatus JSONStringifyer::operationJA() {

0 commit comments

Comments
 (0)