From 8dd8bd56d17e26961822b8c2ba74adda49419af9 Mon Sep 17 00:00:00 2001 From: Benedikt Meurer Date: Mon, 4 Mar 2019 08:45:08 +0100 Subject: [PATCH] [cleanup] Refactor CodeStubAssembler::NewConsString(). MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove the duplication of the allocation logic via the AllocateOneByteConsString and AllocateTwoByteConsString helpers, and instead just have a diamond to figure out the result map. This reduces code size of the StringAdd_CheckNone builtin and even seems to be beneficial performance wise. It seems to improve the performance on the `bench-dom-serialize.js` test by around 1% just doing this. Drive-by-fix: Remove the `flags` from CodeStubAssembler::StringAdd() and its helpers, since we no longer support pretenuring of string additions (for quite a while now). Bug: v8:8834, v8:8939 Change-Id: Ia23e02c974b5f572930fcd45be0643094ab2fa98 Doc: https://bit.ly/fast-string-concatenation-in-javascript Reviewed-on: https://chromium-review.googlesource.com/c/1498133 Commit-Queue: Benedikt Meurer Reviewed-by: Simon Zünd Cr-Commit-Position: refs/heads/master@{#59993} --- src/code-stub-assembler.cc | 86 ++++++++++++-------------------------- src/code-stub-assembler.h | 26 ++---------- 2 files changed, 30 insertions(+), 82 deletions(-) diff --git a/src/code-stub-assembler.cc b/src/code-stub-assembler.cc index 35372d065e..c7566edc87 100644 --- a/src/code-stub-assembler.cc +++ b/src/code-stub-assembler.cc @@ -3307,52 +3307,9 @@ TNode CodeStubAssembler::AllocateSlicedTwoByteString( offset); } -TNode CodeStubAssembler::AllocateConsString(RootIndex map_root_index, - TNode length, - TNode first, - TNode second, - AllocationFlags flags) { - DCHECK(map_root_index == RootIndex::kConsOneByteStringMap || - map_root_index == RootIndex::kConsStringMap); - Node* result = Allocate(ConsString::kSize, flags); - DCHECK(RootsTable::IsImmortalImmovable(map_root_index)); - StoreMapNoWriteBarrier(result, map_root_index); - StoreObjectFieldNoWriteBarrier(result, ConsString::kLengthOffset, length, - MachineRepresentation::kWord32); - StoreObjectFieldNoWriteBarrier(result, ConsString::kHashFieldOffset, - Int32Constant(String::kEmptyHashField), - MachineRepresentation::kWord32); - bool const new_space = !(flags & kPretenured); - if (new_space) { - StoreObjectFieldNoWriteBarrier(result, ConsString::kFirstOffset, first, - MachineRepresentation::kTagged); - StoreObjectFieldNoWriteBarrier(result, ConsString::kSecondOffset, second, - MachineRepresentation::kTagged); - } else { - StoreObjectField(result, ConsString::kFirstOffset, first); - StoreObjectField(result, ConsString::kSecondOffset, second); - } - return CAST(result); -} - -TNode CodeStubAssembler::AllocateOneByteConsString( - TNode length, TNode first, TNode second, - AllocationFlags flags) { - return AllocateConsString(RootIndex::kConsOneByteStringMap, length, first, - second, flags); -} - -TNode CodeStubAssembler::AllocateTwoByteConsString( - TNode length, TNode first, TNode second, - AllocationFlags flags) { - return AllocateConsString(RootIndex::kConsStringMap, length, first, second, - flags); -} - -TNode CodeStubAssembler::NewConsString(TNode length, - TNode left, - TNode right, - AllocationFlags flags) { +TNode CodeStubAssembler::AllocateConsString(TNode length, + TNode left, + TNode right) { // Added string can be a cons string. Comment("Allocating ConsString"); Node* left_instance_type = LoadInstanceType(left); @@ -3377,8 +3334,8 @@ TNode CodeStubAssembler::NewConsString(TNode length, STATIC_ASSERT(kOneByteDataHintTag != 0); Label one_byte_map(this); Label two_byte_map(this); - TVARIABLE(String, result); - Label done(this, &result); + TVARIABLE(Map, result_map); + Label done(this, &result_map); GotoIf(IsSetWord32(anded_instance_types, kStringEncodingMask | kOneByteDataHintTag), &one_byte_map); @@ -3389,18 +3346,30 @@ TNode CodeStubAssembler::NewConsString(TNode length, &two_byte_map, &one_byte_map); BIND(&one_byte_map); - Comment("One-byte ConsString"); - result = AllocateOneByteConsString(length, left, right, flags); - Goto(&done); + { + Comment("One-byte ConsString"); + result_map = CAST(LoadRoot(RootIndex::kConsOneByteStringMap)); + Goto(&done); + } BIND(&two_byte_map); - Comment("Two-byte ConsString"); - result = AllocateTwoByteConsString(length, left, right, flags); - Goto(&done); + { + Comment("Two-byte ConsString"); + result_map = CAST(LoadRoot(RootIndex::kConsStringMap)); + Goto(&done); + } BIND(&done); - - return result.value(); + Node* result = AllocateInNewSpace(ConsString::kSize); + StoreMapNoWriteBarrier(result, result_map.value()); + StoreObjectFieldNoWriteBarrier(result, ConsString::kLengthOffset, length, + MachineRepresentation::kWord32); + StoreObjectFieldNoWriteBarrier(result, ConsString::kHashFieldOffset, + Int32Constant(String::kEmptyHashField), + MachineRepresentation::kWord32); + StoreObjectFieldNoWriteBarrier(result, ConsString::kFirstOffset, left); + StoreObjectFieldNoWriteBarrier(result, ConsString::kSecondOffset, right); + return CAST(result); } TNode CodeStubAssembler::AllocateNameDictionary( @@ -7197,8 +7166,7 @@ void CodeStubAssembler::MaybeDerefIndirectStrings(Variable* var_left, } TNode CodeStubAssembler::StringAdd(Node* context, TNode left, - TNode right, - AllocationFlags flags) { + TNode right) { TVARIABLE(String, result); Label check_right(this), runtime(this, Label::kDeferred), cons(this), done(this, &result), done_native(this, &result); @@ -7234,7 +7202,7 @@ TNode CodeStubAssembler::StringAdd(Node* context, TNode left, &non_cons); result = - NewConsString(new_length, var_left.value(), var_right.value(), flags); + AllocateConsString(new_length, var_left.value(), var_right.value()); Goto(&done_native); BIND(&non_cons); diff --git a/src/code-stub-assembler.h b/src/code-stub-assembler.h index 5f8cc69559..71616a0576 100644 --- a/src/code-stub-assembler.h +++ b/src/code-stub-assembler.h @@ -1463,26 +1463,10 @@ class V8_EXPORT_PRIVATE CodeStubAssembler TNode parent, TNode offset); - // Allocate a one-byte ConsString with the given length, first and second - // parts. |length| is expected to be tagged, and |first| and |second| are - // expected to be one-byte strings. - TNode AllocateOneByteConsString(TNode length, - TNode first, - TNode second, - AllocationFlags flags = kNone); - // Allocate a two-byte ConsString with the given length, first and second - // parts. |length| is expected to be tagged, and |first| and |second| are - // expected to be two-byte strings. - TNode AllocateTwoByteConsString(TNode length, - TNode first, - TNode second, - AllocationFlags flags = kNone); - // Allocate an appropriate one- or two-byte ConsString with the first and // second parts specified by |left| and |right|. - TNode NewConsString(TNode length, TNode left, - TNode right, - AllocationFlags flags = kNone); + TNode AllocateConsString(TNode length, TNode left, + TNode right); TNode AllocateNameDictionary(int at_least_space_for); TNode AllocateNameDictionary( @@ -2285,7 +2269,7 @@ class V8_EXPORT_PRIVATE CodeStubAssembler // Return a new string object produced by concatenating |first| with |second|. TNode StringAdd(Node* context, TNode first, - TNode second, AllocationFlags flags = kNone); + TNode second); // Check if |string| is an indirect (thin or flat cons) string type that can // be dereferenced by DerefIndirectString. @@ -3426,10 +3410,6 @@ class V8_EXPORT_PRIVATE CodeStubAssembler TNode length, TNode parent, TNode offset); - TNode AllocateConsString(RootIndex map_root_index, - TNode length, TNode first, - TNode second, AllocationFlags flags); - // Allocate a MutableHeapNumber without initializing its value. TNode AllocateMutableHeapNumber();