From 0df6a2089977a729711f8f14e5f6477076e04773 Mon Sep 17 00:00:00 2001 From: Leszek Swirski Date: Tue, 24 Nov 2020 14:21:10 +0100 Subject: [PATCH] [string] Unify String::IsEqualTo Make String::IsEqualTo use a direct string shape dispatch and a direct call to CompareChars, rather than splitting the behaviour over IsOneByte/IsTwoByte/HasOneBytePrefix. Avoiding GetFlatContent will make this method easier to make efficient while staying string-access-lock safe. Also, redefines the sequential string table key's matcher in terms of this IsEqualTo method. Change-Id: Iab71246e12044ebaeff06f0dbc14d28b3482dcbe Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2557979 Commit-Queue: Igor Sheludko Auto-Submit: Leszek Swirski Reviewed-by: Igor Sheludko Cr-Commit-Position: refs/heads/master@{#71378} --- src/init/bootstrapper.cc | 2 +- src/json/json-parser.cc | 9 +--- src/objects/string-inl.h | 94 ++++++++++++++++++++++++++++++++-------- src/objects/string.cc | 44 +------------------ src/objects/string.h | 12 +++-- 5 files changed, 85 insertions(+), 76 deletions(-) diff --git a/src/init/bootstrapper.cc b/src/init/bootstrapper.cc index 59421d0439..490065200d 100644 --- a/src/init/bootstrapper.cc +++ b/src/init/bootstrapper.cc @@ -81,7 +81,7 @@ bool SourceCodeCache::Lookup(Isolate* isolate, Vector name, Handle* handle) { for (int i = 0; i < cache_.length(); i += 2) { SeqOneByteString str = SeqOneByteString::cast(cache_.get(i)); - if (str.IsOneByteEqualTo(Vector::cast(name))) { + if (str.IsOneByteEqualTo(name)) { *handle = Handle( SharedFunctionInfo::cast(cache_.get(i + 1)), isolate); return true; diff --git a/src/json/json-parser.cc b/src/json/json-parser.cc index 7f5d14c662..575ae86e72 100644 --- a/src/json/json-parser.cc +++ b/src/json/json-parser.cc @@ -986,14 +986,7 @@ bool Matches(const Vector& chars, Handle string) { DCHECK(!string.is_null()); if (chars.length() != string->length()) return false; - - DisallowGarbageCollection no_gc; - if (string->IsOneByteRepresentation()) { - const uint8_t* string_data = string->GetChars(no_gc); - return CompareChars(chars.begin(), string_data, chars.length()) == 0; - } - const uint16_t* string_data = string->GetChars(no_gc); - return CompareChars(chars.begin(), string_data, chars.length()) == 0; + return string->IsEqualTo(chars); } } // namespace diff --git a/src/objects/string-inl.h b/src/objects/string-inl.h index 503001fb7c..2ab54b2ce7 100644 --- a/src/objects/string-inl.h +++ b/src/objects/string-inl.h @@ -316,16 +316,7 @@ class SequentialStringKey final : public StringTableKey { chars_(chars), convert_(convert) {} - bool IsMatch(String s) override { - SharedStringAccessGuardIfNeeded access_guard(s); - DisallowGarbageCollection no_gc; - if (s.IsOneByteRepresentation()) { - const uint8_t* chars = s.GetChars(no_gc, access_guard); - return CompareChars(chars, chars_.begin(), chars_.length()) == 0; - } - const uint16_t* chars = s.GetChars(no_gc, access_guard); - return CompareChars(chars, chars_.begin(), chars_.length()) == 0; - } + bool IsMatch(String s) override { return s.IsEqualTo(chars_); } Handle AsHandle(Isolate* isolate) { if (sizeof(Char) == 1) { @@ -388,13 +379,8 @@ class SeqSubStringKey final : public StringTableKey { bool IsMatch(String string) override { DisallowGarbageCollection no_gc; - if (string.IsOneByteRepresentation()) { - const uint8_t* data = string.GetChars(no_gc); - return CompareChars(string_->GetChars(no_gc) + from_, data, length()) == - 0; - } - const uint16_t* data = string.GetChars(no_gc); - return CompareChars(string_->GetChars(no_gc) + from_, data, length()) == 0; + return string.IsEqualTo( + Vector(string_->GetChars(no_gc) + from_, length())); } template @@ -442,6 +428,80 @@ bool String::Equals(Isolate* isolate, Handle one, Handle two) { return SlowEquals(isolate, one, two); } +template +bool String::IsEqualTo(Vector str, + String::EqualityType eq_type) const { + size_t len = str.size(); + switch (eq_type) { + case EqualityType::kWholeString: + if (static_cast(length()) != len) return false; + break; + case EqualityType::kPrefix: + if (static_cast(length()) < len) return false; + break; + } + + SharedStringAccessGuardIfNeeded access_guard(*this); + DisallowGarbageCollection no_gc; + + class IsEqualToDispatcher : public AllStatic { + public: + static inline bool HandleSeqOneByteString( + SeqOneByteString str, const Char* data, size_t len, + const DisallowGarbageCollection& no_gc, + const SharedStringAccessGuardIfNeeded& access_guard) { + return CompareChars(str.GetChars(no_gc, access_guard), data, len) == 0; + } + static inline bool HandleSeqTwoByteString( + SeqTwoByteString str, const Char* data, size_t len, + const DisallowGarbageCollection& no_gc, + const SharedStringAccessGuardIfNeeded& access_guard) { + return CompareChars(str.GetChars(no_gc, access_guard), data, len) == 0; + } + static inline bool HandleExternalOneByteString( + ExternalOneByteString str, const Char* data, size_t len, + const DisallowGarbageCollection& no_gc, + const SharedStringAccessGuardIfNeeded& access_guard) { + return CompareChars(str.GetChars(), data, len) == 0; + } + static inline bool HandleExternalTwoByteString( + ExternalTwoByteString str, const Char* data, size_t len, + const DisallowGarbageCollection& no_gc, + const SharedStringAccessGuardIfNeeded& access_guard) { + return CompareChars(str.GetChars(), data, len) == 0; + } + static inline bool HandleConsString( + ConsString str, const Char* data, size_t len, + const DisallowGarbageCollection& no_gc, + const SharedStringAccessGuardIfNeeded& access_guard) { + UNREACHABLE(); + } + static inline bool HandleSlicedString( + SlicedString str, const Char* data, size_t len, + const DisallowGarbageCollection& no_gc, + const SharedStringAccessGuardIfNeeded& access_guard) { + UNREACHABLE(); + } + static inline bool HandleThinString( + ThinString str, const Char* data, size_t len, + const DisallowGarbageCollection& no_gc, + const SharedStringAccessGuardIfNeeded& access_guard) { + UNREACHABLE(); + } + static inline bool HandleInvalidString( + String str, const Char* data, size_t len, + const DisallowGarbageCollection& no_gc, + const SharedStringAccessGuardIfNeeded& access_guard) { + UNREACHABLE(); + } + }; + + return StringShape(*this).DispatchToSpecificType( + *this, str.data(), len, no_gc, access_guard); +} + +bool String::IsOneByteEqualTo(Vector str) { return IsEqualTo(str); } + template const Char* String::GetChars(const DisallowGarbageCollection& no_gc) { return StringShape(*this).IsExternal() diff --git a/src/objects/string.cc b/src/objects/string.cc index 838720325b..b2838eddcc 100644 --- a/src/objects/string.cc +++ b/src/objects/string.cc @@ -1302,50 +1302,8 @@ Object String::LastIndexOf(Isolate* isolate, Handle receiver, return Smi::FromInt(last_index); } -template <> -bool String::IsEqualTo(Vector str) { - return IsOneByteEqualTo(str); -} - -template <> -bool String::IsEqualTo(Vector str) { - return IsTwoByteEqualTo(str); -} - bool String::HasOneBytePrefix(Vector str) { - int slen = str.length(); - if (slen > length()) return false; - DisallowGarbageCollection no_gc; - FlatContent content = GetFlatContent(no_gc); - if (content.IsOneByte()) { - return CompareChars(content.ToOneByteVector().begin(), str.begin(), slen) == - 0; - } - return CompareChars(content.ToUC16Vector().begin(), str.begin(), slen) == 0; -} - -bool String::IsOneByteEqualTo(Vector str) { - int slen = length(); - if (str.length() != slen) return false; - DisallowGarbageCollection no_gc; - FlatContent content = GetFlatContent(no_gc); - if (content.IsOneByte()) { - return CompareChars(content.ToOneByteVector().begin(), str.begin(), slen) == - 0; - } - return CompareChars(content.ToUC16Vector().begin(), str.begin(), slen) == 0; -} - -bool String::IsTwoByteEqualTo(Vector str) { - int slen = length(); - if (str.length() != slen) return false; - DisallowGarbageCollection no_gc; - FlatContent content = GetFlatContent(no_gc); - if (content.IsOneByte()) { - return CompareChars(content.ToOneByteVector().begin(), str.begin(), slen) == - 0; - } - return CompareChars(content.ToUC16Vector().begin(), str.begin(), slen) == 0; + return IsEqualTo(str, EqualityType::kPrefix); } namespace { diff --git a/src/objects/string.h b/src/objects/string.h index 1b8b8e3caf..2f44167e6c 100644 --- a/src/objects/string.h +++ b/src/objects/string.h @@ -317,16 +317,14 @@ class String : public TorqueGeneratedString { inline static bool Equals(Isolate* isolate, Handle one, Handle two); - // Dispatches to Is{One,Two}ByteEqualTo. + enum class EqualityType { kWholeString, kPrefix }; template - bool IsEqualTo(Vector str); + inline bool IsEqualTo( + Vector str, + EqualityType eq_type = EqualityType::kWholeString) const; V8_EXPORT_PRIVATE bool HasOneBytePrefix(Vector str); - V8_EXPORT_PRIVATE bool IsOneByteEqualTo(Vector str); - V8_EXPORT_PRIVATE bool IsOneByteEqualTo(Vector str) { - return IsOneByteEqualTo(Vector::cast(str)); - } - bool IsTwoByteEqualTo(Vector str); + V8_EXPORT_PRIVATE inline bool IsOneByteEqualTo(Vector str); // Return a UTF8 representation of the string. The string is null // terminated but may optionally contain nulls. Length is returned