From 781f7685b6b19a83414da248605b6af80e10e064 Mon Sep 17 00:00:00 2001 From: Benedikt Meurer Date: Thu, 2 Nov 2017 07:22:13 +0100 Subject: [PATCH] Reintroduce compile-time --string-slices flag. This partially reverts commit aaebbbaa598e41626805682679df821757eaa479, which removed the --string-slices flag. We reintroduce the flag as a build time flag for an experiment to gather information of how much SliceStrings help with throughput and effective memory use. Bug: v8:7025 Change-Id: I529da91bb7501fe93d83891abf560710f3ecb9d0 Reviewed-on: https://chromium-review.googlesource.com/750681 Reviewed-by: Yang Guo Reviewed-by: Benedikt Meurer Commit-Queue: Benedikt Meurer Cr-Commit-Position: refs/heads/master@{#49068} --- src/code-stub-assembler.cc | 53 ++++++++++++++++--------------- src/factory.cc | 2 +- src/flag-definitions.h | 1 + test/cctest/test-heap-profiler.cc | 1 + test/cctest/test-strings.cc | 5 +++ 5 files changed, 36 insertions(+), 26 deletions(-) diff --git a/src/code-stub-assembler.cc b/src/code-stub-assembler.cc index 63a54455df..cb39bad7c6 100644 --- a/src/code-stub-assembler.cc +++ b/src/code-stub-assembler.cc @@ -4625,37 +4625,39 @@ Node* CodeStubAssembler::SubString(Node* context, Node* string, Node* from, // encoding at this point. Label external_string(this); { - Label next(this); + if (FLAG_string_slices) { + Label next(this); - // Short slice. Copy instead of slicing. - GotoIf(SmiLessThan(substr_length, SmiConstant(SlicedString::kMinLength)), - &next); + // Short slice. Copy instead of slicing. + GotoIf(SmiLessThan(substr_length, SmiConstant(SlicedString::kMinLength)), + &next); - // Allocate new sliced string. + // Allocate new sliced string. - Counters* counters = isolate()->counters(); - IncrementCounter(counters->sub_string_native(), 1); + Counters* counters = isolate()->counters(); + IncrementCounter(counters->sub_string_native(), 1); - Label one_byte_slice(this), two_byte_slice(this); - Branch(IsOneByteStringInstanceType(to_direct.instance_type()), - &one_byte_slice, &two_byte_slice); + Label one_byte_slice(this), two_byte_slice(this); + Branch(IsOneByteStringInstanceType(to_direct.instance_type()), + &one_byte_slice, &two_byte_slice); - BIND(&one_byte_slice); - { - var_result.Bind( - AllocateSlicedOneByteString(substr_length, direct_string, offset)); - Goto(&end); + BIND(&one_byte_slice); + { + var_result.Bind( + AllocateSlicedOneByteString(substr_length, direct_string, offset)); + Goto(&end); + } + + BIND(&two_byte_slice); + { + var_result.Bind( + AllocateSlicedTwoByteString(substr_length, direct_string, offset)); + Goto(&end); + } + + BIND(&next); } - BIND(&two_byte_slice); - { - var_result.Bind( - AllocateSlicedTwoByteString(substr_length, direct_string, offset)); - Goto(&end); - } - - BIND(&next); - // The subject string can only be external or sequential string of either // encoding at this point. GotoIf(to_direct.is_external(), &external_string); @@ -4663,6 +4665,7 @@ Node* CodeStubAssembler::SubString(Node* context, Node* string, Node* from, var_result.Bind(AllocAndCopyStringCharacters( context, direct_string, instance_type, offset, substr_length)); + Counters* counters = isolate()->counters(); IncrementCounter(counters->sub_string_native(), 1); Goto(&end); @@ -4790,7 +4793,7 @@ Node* ToDirectStringAssembler::TryToDirect(Label* if_bailout) { // Sliced string. Fetch parent and correct start index by offset. BIND(&if_issliced); { - if (flags_ & kDontUnpackSlicedStrings) { + if (!FLAG_string_slices || (flags_ & kDontUnpackSlicedStrings)) { Goto(if_bailout); } else { Node* const string = var_string_.value(); diff --git a/src/factory.cc b/src/factory.cc index 9c6db9556d..5ccf6658fb 100644 --- a/src/factory.cc +++ b/src/factory.cc @@ -835,7 +835,7 @@ Handle Factory::NewProperSubString(Handle str, return MakeOrFindTwoCharacterString(isolate(), c1, c2); } - if (length < SlicedString::kMinLength) { + if (!FLAG_string_slices || length < SlicedString::kMinLength) { if (str->IsOneByteRepresentation()) { Handle result = NewRawOneByteString(length).ToHandleChecked(); diff --git a/src/flag-definitions.h b/src/flag-definitions.h index 7f909c3a79..76d1e47783 100644 --- a/src/flag-definitions.h +++ b/src/flag-definitions.h @@ -316,6 +316,7 @@ DEFINE_VALUE_IMPLICATION(optimize_for_size, max_semi_space_size, 1) // Flags for data representation optimizations DEFINE_BOOL(unbox_double_arrays, true, "automatically unbox arrays of doubles") +DEFINE_BOOL_READONLY(string_slices, true, "use string slices") // Flags for Ignition. DEFINE_BOOL(ignition_elide_noneffectful_bytecodes, true, diff --git a/test/cctest/test-heap-profiler.cc b/test/cctest/test-heap-profiler.cc index 1552daf376..ae3b19df66 100644 --- a/test/cctest/test-heap-profiler.cc +++ b/test/cctest/test-heap-profiler.cc @@ -402,6 +402,7 @@ TEST(HeapSnapshotHeapNumbers) { TEST(HeapSnapshotSlicedString) { + if (!i::FLAG_string_slices) return; LocalContext env; v8::HandleScope scope(env->GetIsolate()); v8::HeapProfiler* heap_profiler = env->GetIsolate()->GetHeapProfiler(); diff --git a/test/cctest/test-strings.cc b/test/cctest/test-strings.cc index dbaf01cdea..42c96568e1 100644 --- a/test/cctest/test-strings.cc +++ b/test/cctest/test-strings.cc @@ -1070,6 +1070,7 @@ TEST(ExternalShortStringAdd) { TEST(JSONStringifySliceMadeExternal) { + if (!FLAG_string_slices) return; CcTest::InitializeVM(); // Create a sliced string from a one-byte string. The latter is turned // into a two-byte external string. Check that JSON.stringify works. @@ -1155,6 +1156,7 @@ TEST(CachedHashOverflow) { TEST(SliceFromCons) { + if (!FLAG_string_slices) return; CcTest::InitializeVM(); Factory* factory = CcTest::i_isolate()->factory(); v8::HandleScope scope(CcTest::isolate()); @@ -1221,6 +1223,7 @@ TEST(InternalizeExternal) { } TEST(SliceFromExternal) { + if (!FLAG_string_slices) return; CcTest::InitializeVM(); Factory* factory = CcTest::i_isolate()->factory(); v8::HandleScope scope(CcTest::isolate()); @@ -1241,6 +1244,7 @@ TEST(SliceFromExternal) { TEST(TrivialSlice) { // This tests whether a slice that contains the entire parent string // actually creates a new string (it should not). + if (!FLAG_string_slices) return; CcTest::InitializeVM(); Factory* factory = CcTest::i_isolate()->factory(); v8::HandleScope scope(CcTest::isolate()); @@ -1270,6 +1274,7 @@ TEST(TrivialSlice) { TEST(SliceFromSlice) { // This tests whether a slice that contains the entire parent string // actually creates a new string (it should not). + if (!FLAG_string_slices) return; CcTest::InitializeVM(); v8::HandleScope scope(CcTest::isolate()); v8::Local result;