Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(21)

Unified Diff: src/builtins/builtins-string-gen.cc

Issue 2813843002: [string] Add a fast path to String.p.replace (Closed)
Patch Set: Don't scan twice Created 3 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | src/code-stub-assembler.h » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: src/builtins/builtins-string-gen.cc
diff --git a/src/builtins/builtins-string-gen.cc b/src/builtins/builtins-string-gen.cc
index 25bf14ce2bd83dbe2bf41dfa31d03235c4fdfdd0..18fbe9a72970654466cb9f1ac9620b13d64abdb2 100644
--- a/src/builtins/builtins-string-gen.cc
+++ b/src/builtins/builtins-string-gen.cc
@@ -19,6 +19,11 @@ class StringBuiltinsAssembler : public CodeStubAssembler {
explicit StringBuiltinsAssembler(compiler::CodeAssemblerState* state)
: CodeStubAssembler(state) {}
+ // ES#sec-getsubstitution
+ Node* GetSubstitution(Node* context, Node* subject_string,
+ Node* match_start_index, Node* match_end_index,
+ Node* replace_string);
+
protected:
Node* DirectStringData(Node* string, Node* string_instance_type) {
// Compute the effective offset of the first character.
@@ -112,6 +117,8 @@ class StringBuiltinsAssembler : public CodeStubAssembler {
Node* search_string_instance_type, Node* position,
std::function<void(Node*)> f_return);
+ Node* IndexOfDollarChar(Node* const context, Node* const string);
+
Node* IsNullOrUndefined(Node* const value);
void RequireObjectCoercible(Node* const context, Node* const value,
const char* method_name);
@@ -991,6 +998,60 @@ void StringBuiltinsAssembler::MaybeCallFunctionAtSymbol(
BIND(&out);
}
+compiler::Node* StringBuiltinsAssembler::IndexOfDollarChar(Node* const context,
+ Node* const string) {
+ CSA_ASSERT(this, IsString(string));
+
+ Node* const dollar_string = HeapConstant(
+ isolate()->factory()->LookupSingleCharacterStringFromCode('$'));
+ Node* const dollar_ix = CallBuiltin(Builtins::kStringIndexOf, context, string,
+ dollar_string, SmiConstant(0));
+
+ CSA_ASSERT(this, TaggedIsSmi(dollar_ix));
+ return dollar_ix;
+}
+
+compiler::Node* StringBuiltinsAssembler::GetSubstitution(
+ Node* context, Node* subject_string, Node* match_start_index,
+ Node* match_end_index, Node* replace_string) {
+ CSA_ASSERT(this, IsString(subject_string));
+ CSA_ASSERT(this, IsString(replace_string));
+ CSA_ASSERT(this, TaggedIsPositiveSmi(match_start_index));
+ CSA_ASSERT(this, TaggedIsPositiveSmi(match_end_index));
+
+ VARIABLE(var_result, MachineRepresentation::kTagged, replace_string);
+ Label runtime(this), out(this);
+
+ // In this primitive implementation we simply look for the next '$' char in
+ // {replace_string}. If it doesn't exist, we can simply return
+ // {replace_string} itself. If it does, then we delegate to
+ // String::GetSubstitution, passing in the index of the first '$' to avoid
+ // repeated scanning work.
+ // TODO(jgruber): Possibly extend this in the future to handle more complex
+ // cases without runtime calls.
+
+ Node* const dollar_index = IndexOfDollarChar(context, replace_string);
+ Branch(SmiIsNegative(dollar_index), &out, &runtime);
+
+ BIND(&runtime);
+ {
+ CSA_ASSERT(this, TaggedIsPositiveSmi(dollar_index));
+
+ Callable substring_callable = CodeFactory::SubString(isolate());
+ Node* const matched = CallStub(substring_callable, context, subject_string,
+ match_start_index, match_end_index);
+ Node* const replacement_string =
+ CallRuntime(Runtime::kGetSubstitution, context, matched, subject_string,
+ match_start_index, replace_string, dollar_index);
+ var_result.Bind(replacement_string);
+
+ Goto(&out);
+ }
+
+ BIND(&out);
+ return var_result.value();
+}
+
// ES6 #sec-string.prototype.replace
TF_BUILTIN(StringPrototypeReplace, StringBuiltinsAssembler) {
Label out(this);
@@ -1033,7 +1094,7 @@ TF_BUILTIN(StringPrototypeReplace, StringBuiltinsAssembler) {
Node* const subject_length = LoadStringLength(subject_string);
Node* const search_length = LoadStringLength(search_string);
- // Fast-path single-char {search}, long {receiver}, and simple string
+ // Fast-path single-char {search}, long cons {receiver}, and simple string
// {replace}.
{
Label next(this);
@@ -1043,11 +1104,10 @@ TF_BUILTIN(StringPrototypeReplace, StringBuiltinsAssembler) {
GotoIf(TaggedIsSmi(replace), &next);
GotoIfNot(IsString(replace), &next);
- Node* const dollar_string = HeapConstant(
- isolate()->factory()->LookupSingleCharacterStringFromCode('$'));
- Node* const dollar_ix =
- CallStub(indexof_callable, context, replace, dollar_string, smi_zero);
- GotoIfNot(SmiIsNegative(dollar_ix), &next);
+ Node* const subject_instance_type = LoadInstanceType(subject_string);
+ GotoIfNot(IsConsStringInstanceType(subject_instance_type), &next);
+
+ GotoIf(TaggedIsPositiveSmi(IndexOfDollarChar(context, replace)), &next);
// Searching by traversing a cons string tree and replace with cons of
// slices works only when the replaced string is a single character, being
@@ -1136,15 +1196,11 @@ TF_BUILTIN(StringPrototypeReplace, StringBuiltinsAssembler) {
BIND(&if_notcallablereplace);
{
Node* const replace_string = CallStub(tostring_callable, context, replace);
-
- // TODO(jgruber): Simplified GetSubstitution implementation in CSA.
- Node* const matched = CallStub(substring_callable, context, subject_string,
- match_start_index, match_end_index);
- Node* const replacement_string =
- CallRuntime(Runtime::kGetSubstitution, context, matched, subject_string,
- match_start_index, replace_string);
- var_result.Bind(CallStub(stringadd_callable, context, var_result.value(),
- replacement_string));
+ Node* const replacement =
+ GetSubstitution(context, subject_string, match_start_index,
+ match_end_index, replace_string);
+ var_result.Bind(
+ CallStub(stringadd_callable, context, var_result.value(), replacement));
Goto(&out);
}
« no previous file with comments | « no previous file | src/code-stub-assembler.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698