Commit 87567584 authored by Camillo Bruni's avatar Camillo Bruni Committed by V8 LUCI CQ

[runtime][json] Add IncrementalStringBuilder::AppendCStringLiteral

Directly memcpy char* literals if they fit in the current pending
part. This avoids incremental checks for the current part size.

This will improve JSON.stringify for objects with lots of
true, false, null values by roughly 10%;

Drive-by-fix:
- Improve JSON.stringify for empty [] and {}
- Add IncrementalStringBuilder::NoExtend DECHECKs

Bug: v8:12195
Change-Id: I81ebc9e088cf983adbcfb2d768137e4a3cef9a7a
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3260524Reviewed-by: 's avatarToon Verwaest <verwaest@chromium.org>
Commit-Queue: Camillo Bruni <cbruni@chromium.org>
Cr-Commit-Position: refs/heads/main@{#77817}
parent 5e16d853
......@@ -47,7 +47,7 @@ MaybeHandle<Object> CreateDynamicFunction(Isolate* isolate,
IncrementalStringBuilder builder(isolate);
builder.AppendCharacter('(');
builder.AppendCString(token);
builder.AppendCString(" anonymous(");
builder.AppendCStringLiteral(" anonymous(");
if (argc > 1) {
for (int i = 1; i < argc; ++i) {
if (i > 1) builder.AppendCharacter(',');
......@@ -60,14 +60,14 @@ MaybeHandle<Object> CreateDynamicFunction(Isolate* isolate,
}
builder.AppendCharacter('\n');
parameters_end_pos = builder.Length();
builder.AppendCString(") {\n");
builder.AppendCStringLiteral(") {\n");
if (argc > 0) {
Handle<String> body;
ASSIGN_RETURN_ON_EXCEPTION(
isolate, body, Object::ToString(isolate, args.at(argc)), Object);
builder.AppendString(body);
}
builder.AppendCString("\n})");
builder.AppendCStringLiteral("\n})");
ASSIGN_RETURN_ON_EXCEPTION(isolate, source, builder.Finish(), Object);
}
......
......@@ -85,9 +85,9 @@ Local<String> GetFunctionDescription(Local<Function> function) {
auto debug_name =
i::GetWasmFunctionDebugName(isolate, instance, func_index);
i::IncrementalStringBuilder builder(isolate);
builder.AppendCString("function ");
builder.AppendCStringLiteral("function ");
builder.AppendString(debug_name);
builder.AppendCString("() { [native code] }");
builder.AppendCStringLiteral("() { [native code] }");
return Utils::ToLocal(builder.Finish().ToHandleChecked());
}
}
......
......@@ -261,10 +261,10 @@ MaybeHandle<Object> AppendErrorString(Isolate* isolate, Handle<Object> error,
DCHECK(isolate->has_pending_exception());
isolate->clear_pending_exception();
isolate->set_external_caught_exception(false);
builder->AppendCString("<error>");
builder->AppendCStringLiteral("<error>");
} else {
// Formatted thrown exception successfully, append it.
builder->AppendCString("<error: ");
builder->AppendCStringLiteral("<error: ");
builder->AppendString(err_str.ToHandleChecked());
builder->AppendCharacter('>');
}
......@@ -369,7 +369,7 @@ MaybeHandle<Object> ErrorUtils::FormatStackTrace(Isolate* isolate,
Object);
for (int i = 0; i < elems->length(); ++i) {
builder.AppendCString("\n at ");
builder.AppendCStringLiteral("\n at ");
Handle<StackFrameInfo> frame(StackFrameInfo::cast(elems->get(i)), isolate);
SerializeStackFrameInfo(isolate, frame, &builder);
......@@ -389,12 +389,12 @@ MaybeHandle<Object> ErrorUtils::FormatStackTrace(Isolate* isolate,
if (exception_string.is_null()) {
// Formatting the thrown exception threw again, give up.
builder.AppendCString("<error>");
builder.AppendCStringLiteral("<error>");
} else {
// Formatted thrown exception successfully, append it.
builder.AppendCString("<error: ");
builder.AppendCStringLiteral("<error: ");
builder.AppendString(exception_string.ToHandleChecked());
builder.AppendCString("<error>");
builder.AppendCStringLiteral("<error>");
}
}
}
......@@ -658,7 +658,7 @@ MaybeHandle<String> ErrorUtils::ToString(Isolate* isolate,
// the code unit 0x0020 (SPACE), and msg.
IncrementalStringBuilder builder(isolate);
builder.AppendString(name);
builder.AppendCString(": ");
builder.AppendCStringLiteral(": ");
builder.AppendString(msg);
Handle<String> result;
......@@ -745,7 +745,7 @@ Handle<String> BuildDefaultCallSite(Isolate* isolate, Handle<Object> object) {
builder.AppendString(Object::TypeOf(isolate, object));
if (object->IsString()) {
builder.AppendCString(" \"");
builder.AppendCStringLiteral(" \"");
Handle<String> string = Handle<String>::cast(object);
// This threshold must be sufficiently far below String::kMaxLength that
// the {builder}'s result can never exceed that limit.
......@@ -756,20 +756,17 @@ Handle<String> BuildDefaultCallSite(Isolate* isolate, Handle<Object> object) {
string = isolate->factory()->NewProperSubString(string, 0,
kMaxPrintedStringLength);
builder.AppendString(string);
builder.AppendCString("<...>");
builder.AppendCStringLiteral("<...>");
}
builder.AppendCString("\"");
builder.AppendCStringLiteral("\"");
} else if (object->IsNull(isolate)) {
builder.AppendCString(" ");
builder.AppendString(isolate->factory()->null_string());
builder.AppendCStringLiteral(" null");
} else if (object->IsTrue(isolate)) {
builder.AppendCString(" ");
builder.AppendString(isolate->factory()->true_string());
builder.AppendCStringLiteral(" true");
} else if (object->IsFalse(isolate)) {
builder.AppendCString(" ");
builder.AppendString(isolate->factory()->false_string());
builder.AppendCStringLiteral(" false");
} else if (object->IsNumber()) {
builder.AppendCString(" ");
builder.AppendCharacter(' ');
builder.AppendString(isolate->factory()->NumberToString(object));
}
......
This diff is collapsed.
......@@ -937,9 +937,9 @@ Handle<String> NativeCodeFunctionSourceString(
Handle<SharedFunctionInfo> shared_info) {
Isolate* const isolate = shared_info->GetIsolate();
IncrementalStringBuilder builder(isolate);
builder.AppendCString("function ");
builder.AppendCStringLiteral("function ");
builder.AppendString(handle(shared_info->Name(), isolate));
builder.AppendCString("() { [native code] }");
builder.AppendCStringLiteral("() { [native code] }");
return builder.Finish().ToHandleChecked();
}
......
......@@ -452,12 +452,12 @@ Handle<String> NoSideEffectsErrorToString(Isolate* isolate,
IncrementalStringBuilder builder(isolate);
builder.AppendString(name_str);
builder.AppendCString(": ");
builder.AppendCStringLiteral(": ");
if (builder.Length() + msg_str->length() <= String::kMaxLength) {
builder.AppendString(msg_str);
} else {
builder.AppendCString("<a very large string>");
builder.AppendCStringLiteral("<a very large string>");
}
return builder.Finish().ToHandleChecked();
......@@ -501,7 +501,7 @@ MaybeHandle<String> Object::NoSideEffectsToMaybeString(Isolate* isolate,
if (fun_str->length() > 128) {
IncrementalStringBuilder builder(isolate);
builder.AppendString(isolate->factory()->NewSubString(fun_str, 0, 111));
builder.AppendCString("...<omitted>...");
builder.AppendCStringLiteral("...<omitted>...");
builder.AppendString(isolate->factory()->NewSubString(
fun_str, fun_str->length() - 2, fun_str->length()));
......@@ -517,7 +517,7 @@ MaybeHandle<String> Object::NoSideEffectsToMaybeString(Isolate* isolate,
}
IncrementalStringBuilder builder(isolate);
builder.AppendCString("Symbol(");
builder.AppendCStringLiteral("Symbol(");
if (symbol->description().IsString()) {
builder.AppendString(
handle(String::cast(symbol->description()), isolate));
......@@ -555,9 +555,9 @@ MaybeHandle<String> Object::NoSideEffectsToMaybeString(Isolate* isolate,
if (ctor_name->length() != 0) {
IncrementalStringBuilder builder(isolate);
builder.AppendCString("#<");
builder.AppendCStringLiteral("#<");
builder.AppendString(ctor_name);
builder.AppendCString(">");
builder.AppendCharacter('>');
return builder.Finish().ToHandleChecked();
}
......@@ -603,9 +603,9 @@ Handle<String> Object::NoSideEffectsToString(Isolate* isolate,
tag_obj->IsString() ? Handle<String>::cast(tag_obj) : builtin_tag;
IncrementalStringBuilder builder(isolate);
builder.AppendCString("[object ");
builder.AppendCStringLiteral("[object ");
builder.AppendString(tag);
builder.AppendCString("]");
builder.AppendCharacter(']');
return builder.Finish().ToHandleChecked();
}
......
......@@ -419,19 +419,19 @@ Handle<Object> SharedFunctionInfo::GetSourceCodeHarmony(
DCHECK(!shared->name_should_print_as_anonymous());
IncrementalStringBuilder builder(isolate);
builder.AppendCString("function ");
builder.AppendCStringLiteral("function ");
builder.AppendString(Handle<String>(shared->Name(), isolate));
builder.AppendCString("(");
builder.AppendCharacter('(');
Handle<FixedArray> args(Script::cast(shared->script()).wrapped_arguments(),
isolate);
int argc = args->length();
for (int i = 0; i < argc; i++) {
if (i > 0) builder.AppendCString(", ");
if (i > 0) builder.AppendCStringLiteral(", ");
builder.AppendString(Handle<String>(String::cast(args->get(i)), isolate));
}
builder.AppendCString(") {\n");
builder.AppendCStringLiteral(") {\n");
builder.AppendString(source);
builder.AppendCString("\n}");
builder.AppendCStringLiteral("\n}");
return builder.Finish().ToHandleChecked();
}
......
......@@ -195,18 +195,18 @@ MaybeHandle<String> FormatEvalOrigin(Isolate* isolate, Handle<Script> script) {
if (sourceURL->IsString()) return Handle<String>::cast(sourceURL);
IncrementalStringBuilder builder(isolate);
builder.AppendCString("eval at ");
builder.AppendCStringLiteral("eval at ");
if (script->has_eval_from_shared()) {
Handle<SharedFunctionInfo> eval_shared(script->eval_from_shared(), isolate);
auto eval_name = SharedFunctionInfo::DebugName(eval_shared);
if (eval_name->length() != 0) {
builder.AppendString(eval_name);
} else {
builder.AppendCString("<anonymous>");
builder.AppendCStringLiteral("<anonymous>");
}
if (eval_shared->script().IsScript()) {
Handle<Script> eval_script(Script::cast(eval_shared->script()), isolate);
builder.AppendCString(" (");
builder.AppendCStringLiteral(" (");
if (eval_script->compilation_type() == Script::COMPILATION_TYPE_EVAL) {
// Eval script originated from another eval.
Handle<String> str;
......@@ -222,19 +222,19 @@ MaybeHandle<String> FormatEvalOrigin(Isolate* isolate, Handle<Script> script) {
if (Script::GetPositionInfo(eval_script,
Script::GetEvalPosition(isolate, script),
&info, Script::NO_OFFSET)) {
builder.AppendCString(":");
builder.AppendCharacter(':');
builder.AppendInt(info.line + 1);
builder.AppendCString(":");
builder.AppendCharacter(':');
builder.AppendInt(info.column + 1);
}
} else {
builder.AppendCString("unknown source");
builder.AppendCStringLiteral("unknown source");
}
}
builder.AppendCString(")");
builder.AppendCharacter(')');
}
} else {
builder.AppendCString("<anonymous>");
builder.AppendCStringLiteral("<anonymous>");
}
return builder.Finish().ToHandleChecked();
}
......@@ -581,7 +581,8 @@ void AppendFileLocation(Isolate* isolate, Handle<StackFrameInfo> frame,
if (!script_name_or_source_url->IsString() && frame->IsEval()) {
builder->AppendString(
Handle<String>::cast(StackFrameInfo::GetEvalOrigin(frame)));
builder->AppendCString(", "); // Expecting source position to follow.
// Expecting source position to follow.
builder->AppendCStringLiteral(", ");
}
if (IsNonEmptyString(script_name_or_source_url)) {
......@@ -590,7 +591,7 @@ void AppendFileLocation(Isolate* isolate, Handle<StackFrameInfo> frame,
// Source code does not originate from a file and is not native, but we
// can still get the source position inside the source string, e.g. in
// an eval string.
builder->AppendCString("<anonymous>");
builder->AppendCStringLiteral("<anonymous>");
}
int line_number = StackFrameInfo::GetLineNumber(frame);
......@@ -665,7 +666,7 @@ void AppendMethodCall(Isolate* isolate, Handle<StackFrameInfo> frame,
if (IsNonEmptyString(method_name)) {
Handle<String> method_string = Handle<String>::cast(method_name);
if (!StringEndsWithMethodName(isolate, function_string, method_string)) {
builder->AppendCString(" [as ");
builder->AppendCStringLiteral(" [as ");
builder->AppendString(method_string);
builder->AppendCharacter(']');
}
......@@ -678,7 +679,7 @@ void AppendMethodCall(Isolate* isolate, Handle<StackFrameInfo> frame,
if (IsNonEmptyString(method_name)) {
builder->AppendString(Handle<String>::cast(method_name));
} else {
builder->AppendCString("<anonymous>");
builder->AppendCStringLiteral("<anonymous>");
}
}
}
......@@ -687,24 +688,24 @@ void SerializeJSStackFrame(Isolate* isolate, Handle<StackFrameInfo> frame,
IncrementalStringBuilder* builder) {
Handle<Object> function_name = StackFrameInfo::GetFunctionName(frame);
if (frame->IsAsync()) {
builder->AppendCString("async ");
builder->AppendCStringLiteral("async ");
if (frame->IsPromiseAll() || frame->IsPromiseAny()) {
builder->AppendCString("Promise.");
builder->AppendCStringLiteral("Promise.");
builder->AppendString(Handle<String>::cast(function_name));
builder->AppendCString(" (index ");
builder->AppendCStringLiteral(" (index ");
builder->AppendInt(StackFrameInfo::GetSourcePosition(frame));
builder->AppendCString(")");
builder->AppendCharacter(')');
return;
}
}
if (frame->IsMethodCall()) {
AppendMethodCall(isolate, frame, builder);
} else if (frame->IsConstructor()) {
builder->AppendCString("new ");
builder->AppendCStringLiteral("new ");
if (IsNonEmptyString(function_name)) {
builder->AppendString(Handle<String>::cast(function_name));
} else {
builder->AppendCString("<anonymous>");
builder->AppendCStringLiteral("<anonymous>");
}
} else if (IsNonEmptyString(function_name)) {
builder->AppendString(Handle<String>::cast(function_name));
......@@ -712,9 +713,9 @@ void SerializeJSStackFrame(Isolate* isolate, Handle<StackFrameInfo> frame,
AppendFileLocation(isolate, frame, builder);
return;
}
builder->AppendCString(" (");
builder->AppendCStringLiteral(" (");
AppendFileLocation(isolate, frame, builder);
builder->AppendCString(")");
builder->AppendCharacter(')');
}
#if V8_ENABLE_WEBASSEMBLY
......@@ -729,32 +730,32 @@ void SerializeWasmStackFrame(Isolate* isolate, Handle<StackFrameInfo> frame,
} else {
builder->AppendString(Handle<String>::cast(module_name));
if (!function_name->IsNull()) {
builder->AppendCString(".");
builder->AppendCharacter('.');
builder->AppendString(Handle<String>::cast(function_name));
}
}
builder->AppendCString(" (");
builder->AppendCStringLiteral(" (");
}
Handle<Object> url(frame->GetScriptNameOrSourceURL(), isolate);
if (IsNonEmptyString(url)) {
builder->AppendString(Handle<String>::cast(url));
} else {
builder->AppendCString("<anonymous>");
builder->AppendCStringLiteral("<anonymous>");
}
builder->AppendCString(":");
builder->AppendCharacter(':');
const int wasm_func_index = frame->GetWasmFunctionIndex();
builder->AppendCString("wasm-function[");
builder->AppendCStringLiteral("wasm-function[");
builder->AppendInt(wasm_func_index);
builder->AppendCString("]:");
builder->AppendCStringLiteral("]:");
char buffer[16];
SNPrintF(base::ArrayVector(buffer), "0x%x",
StackFrameInfo::GetColumnNumber(frame) - 1);
builder->AppendCString(buffer);
if (has_name) builder->AppendCString(")");
if (has_name) builder->AppendCharacter(')');
}
#endif // V8_ENABLE_WEBASSEMBLY
......
......@@ -876,10 +876,22 @@ uint8_t SeqOneByteString::Get(
}
void SeqOneByteString::SeqOneByteStringSet(int index, uint16_t value) {
DCHECK(index >= 0 && index < length() && value <= kMaxOneByteCharCode);
DCHECK_GE(index, 0);
DCHECK_LT(index, length());
DCHECK_LE(value, kMaxOneByteCharCode);
WriteField<byte>(kHeaderSize + index * kCharSize, static_cast<byte>(value));
}
void SeqOneByteString::SeqOneByteStringSetChars(int index,
const uint8_t* string,
int string_length) {
DCHECK_LE(0, index);
DCHECK_LT(index + string_length, length());
void* address =
reinterpret_cast<void*>(field_address(kHeaderSize + index * kCharSize));
memcpy(address, string, string_length);
}
Address SeqOneByteString::GetCharsAddress() const {
return field_address(kHeaderSize);
}
......
......@@ -689,6 +689,8 @@ class SeqOneByteString
inline uint8_t Get(int index, PtrComprCageBase cage_base,
const SharedStringAccessGuardIfNeeded& access_guard) const;
inline void SeqOneByteStringSet(int index, uint16_t value);
inline void SeqOneByteStringSetChars(int index, const uint8_t* string,
int length);
// Get the address of the characters in this string.
inline Address GetCharsAddress() const;
......
......@@ -48,7 +48,7 @@ RUNTIME_FUNCTION(Runtime_SymbolDescriptiveString) {
DCHECK_EQ(1, args.length());
CONVERT_ARG_HANDLE_CHECKED(Symbol, symbol, 0);
IncrementalStringBuilder builder(isolate);
builder.AppendCString("Symbol(");
builder.AppendCStringLiteral("Symbol(");
if (symbol->description().IsString()) {
builder.AppendString(handle(String::cast(symbol->description()), isolate));
}
......
......@@ -130,6 +130,24 @@ class IncrementalStringBuilder {
}
}
template <int N>
V8_INLINE void AppendCStringLiteral(const char (&literal)[N]) {
// Note that the literal contains the zero char.
const int length = N - 1;
STATIC_ASSERT(length > 0);
if (length == 1) return AppendCharacter(literal[0]);
if (encoding_ == String::ONE_BYTE_ENCODING && CurrentPartCanFit(N)) {
const uint8_t* chars = reinterpret_cast<const uint8_t*>(literal);
SeqOneByteString::cast(*current_part_)
.SeqOneByteStringSetChars(current_index_, chars, length);
current_index_ += length;
if (current_index_ == part_length_) Extend();
DCHECK(HasValidCurrentIndex());
return;
}
return AppendCString(literal);
}
V8_INLINE void AppendCString(const char* s) {
const uint8_t* u = reinterpret_cast<const uint8_t*>(s);
if (encoding_ == String::ONE_BYTE_ENCODING) {
......@@ -190,19 +208,38 @@ class IncrementalStringBuilder {
template <typename DestChar>
class NoExtend {
public:
NoExtend(Handle<String> string, int offset,
NoExtend(String string, int offset,
const DisallowGarbageCollection& no_gc) {
DCHECK(string->IsSeqOneByteString() || string->IsSeqTwoByteString());
DCHECK(string.IsSeqOneByteString() || string.IsSeqTwoByteString());
if (sizeof(DestChar) == 1) {
start_ = reinterpret_cast<DestChar*>(
Handle<SeqOneByteString>::cast(string)->GetChars(no_gc) + offset);
SeqOneByteString::cast(string).GetChars(no_gc) + offset);
} else {
start_ = reinterpret_cast<DestChar*>(
Handle<SeqTwoByteString>::cast(string)->GetChars(no_gc) + offset);
SeqTwoByteString::cast(string).GetChars(no_gc) + offset);
}
cursor_ = start_;
#ifdef DEBUG
string_ = string;
#endif
}
#ifdef DEBUG
~NoExtend() {
DestChar* end;
if (sizeof(DestChar) == 1) {
auto one_byte_string = SeqOneByteString::cast(string_);
end = reinterpret_cast<DestChar*>(one_byte_string.GetChars(no_gc_) +
one_byte_string.length());
} else {
auto two_byte_string = SeqTwoByteString::cast(string_);
end = reinterpret_cast<DestChar*>(two_byte_string.GetChars(no_gc_) +
two_byte_string.length());
}
DCHECK_LE(cursor_, end + 1);
}
#endif
V8_INLINE void Append(DestChar c) { *(cursor_++) = c; }
V8_INLINE void AppendCString(const char* s) {
const uint8_t* u = reinterpret_cast<const uint8_t*>(s);
......@@ -214,6 +251,9 @@ class IncrementalStringBuilder {
private:
DestChar* start_;
DestChar* cursor_;
#ifdef DEBUG
String string_;
#endif
DISALLOW_GARBAGE_COLLECTION(no_gc_)
};
......@@ -242,14 +282,15 @@ class IncrementalStringBuilder {
public:
NoExtendBuilder(IncrementalStringBuilder* builder, int required_length,
const DisallowGarbageCollection& no_gc)
: NoExtend<DestChar>(builder->current_part(), builder->current_index_,
no_gc),
: NoExtend<DestChar>(*(builder->current_part()),
builder->current_index_, no_gc),
builder_(builder) {
DCHECK(builder->CurrentPartCanFit(required_length));
}
~NoExtendBuilder() {
builder_->current_index_ += NoExtend<DestChar>::written();
DCHECK(builder_->HasValidCurrentIndex());
}
private:
......@@ -277,6 +318,8 @@ class IncrementalStringBuilder {
// Finish the current part and allocate a new part.
void Extend();
bool HasValidCurrentIndex() const;
// Shrink current part to the right size.
void ShrinkCurrentPart() {
DCHECK(current_index_ < part_length_);
......@@ -314,6 +357,7 @@ void IncrementalStringBuilder::Append(SrcChar c) {
.SeqTwoByteStringSet(current_index_++, c);
}
if (current_index_ == part_length_) Extend();
DCHECK(HasValidCurrentIndex());
}
} // namespace internal
} // namespace v8
......
......@@ -246,6 +246,10 @@ int IncrementalStringBuilder::Length() const {
return accumulator_->length() + current_index_;
}
bool IncrementalStringBuilder::HasValidCurrentIndex() const {
return current_index_ < part_length_;
}
void IncrementalStringBuilder::Accumulate(Handle<String> new_part) {
Handle<String> new_accumulator;
if (accumulator()->length() + new_part->length() > String::kMaxLength) {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment