diff --git a/src/api/api.cc b/src/api/api.cc index dedbd5db66..ad9aeccbb2 100644 --- a/src/api/api.cc +++ b/src/api/api.cc @@ -9333,7 +9333,7 @@ void v8::Isolate::LocaleConfigurationChangeNotification() { #ifdef V8_INTL_SUPPORT i_isolate->ResetDefaultLocale(); - i_isolate->ClearCachedIcuObjects(); + i_isolate->clear_cached_icu_objects(); #endif // V8_INTL_SUPPORT } diff --git a/src/builtins/builtins-intl.cc b/src/builtins/builtins-intl.cc index cff87636cb..b1eb0a0a78 100644 --- a/src/builtins/builtins-intl.cc +++ b/src/builtins/builtins-intl.cc @@ -1009,7 +1009,8 @@ BUILTIN(CollatorInternalCompare) { // 7. Return CompareStrings(collator, X, Y). icu::Collator* icu_collator = collator->icu_collator().raw(); CHECK_NOT_NULL(icu_collator); - return *Intl::CompareStrings(isolate, *icu_collator, string_x, string_y); + return Smi::FromInt( + Intl::CompareStrings(isolate, *icu_collator, string_x, string_y)); } // ecma402 #sec-%segmentiteratorprototype%.next diff --git a/src/builtins/builtins-string.cc b/src/builtins/builtins-string.cc index 950cefd7ba..d94976bab2 100644 --- a/src/builtins/builtins-string.cc +++ b/src/builtins/builtins-string.cc @@ -140,21 +140,25 @@ BUILTIN(StringPrototypeLocaleCompare) { HandleScope handle_scope(isolate); isolate->CountUsage(v8::Isolate::UseCounterFeature::kStringLocaleCompare); - const char* method = "String.prototype.localeCompare"; + static const char* const kMethod = "String.prototype.localeCompare"; #ifdef V8_INTL_SUPPORT - TO_THIS_STRING(str1, method); + TO_THIS_STRING(str1, kMethod); Handle str2; ASSIGN_RETURN_FAILURE_ON_EXCEPTION( isolate, str2, Object::ToString(isolate, args.atOrUndefined(isolate, 1))); - RETURN_RESULT_OR_FAILURE( - isolate, Intl::StringLocaleCompare( - isolate, str1, str2, args.atOrUndefined(isolate, 2), - args.atOrUndefined(isolate, 3), method)); + base::Optional result = Intl::StringLocaleCompare( + isolate, str1, str2, args.atOrUndefined(isolate, 2), + args.atOrUndefined(isolate, 3), kMethod); + if (!result.has_value()) { + DCHECK(isolate->has_pending_exception()); + return ReadOnlyRoots(isolate).exception(); + } + return Smi::FromInt(result.value()); #else DCHECK_LE(2, args.length()); - TO_THIS_STRING(str1, method); + TO_THIS_STRING(str1, kMethod); Handle str2; ASSIGN_RETURN_FAILURE_ON_EXCEPTION(isolate, str2, Object::ToString(isolate, args.at(1))); diff --git a/src/execution/isolate.cc b/src/execution/isolate.cc index c630cb73fa..de7378acd5 100644 --- a/src/execution/isolate.cc +++ b/src/execution/isolate.cc @@ -4800,48 +4800,47 @@ void Isolate::CollectSourcePositionsForAllBytecodeArrays() { } #ifdef V8_INTL_SUPPORT -namespace { -std::string GetStringFromLocale(Handle locales_obj) { - DCHECK(locales_obj->IsString() || locales_obj->IsUndefined()); - if (locales_obj->IsString()) { - return std::string(String::cast(*locales_obj).ToCString().get()); - } - return ""; +namespace { + +std::string GetStringFromLocales(Isolate* isolate, Handle locales) { + if (locales->IsUndefined(isolate)) return ""; + return std::string(String::cast(*locales).ToCString().get()); } + +bool StringEqualsLocales(Isolate* isolate, const std::string& str, + Handle locales) { + if (locales->IsUndefined(isolate)) return str == ""; + return Handle::cast(locales)->IsEqualTo( + base::VectorOf(str.c_str(), str.length())); +} + } // namespace icu::UMemory* Isolate::get_cached_icu_object(ICUObjectCacheType cache_type, - Handle locales_obj) { - std::string locale = GetStringFromLocale(locales_obj); - auto value = icu_object_cache_.find(cache_type); - if (value == icu_object_cache_.end()) return nullptr; - - ICUCachePair pair = value->second; - if (pair.first != locale) return nullptr; - - return pair.second.get(); + Handle locales) { + const ICUObjectCacheEntry& entry = + icu_object_cache_[static_cast(cache_type)]; + return StringEqualsLocales(this, entry.locales, locales) ? entry.obj.get() + : nullptr; } -void Isolate::set_icu_object_in_cache( - ICUObjectCacheType cache_type, Handle locales_obj, - std::shared_ptr icu_formatter) { - std::string locale = GetStringFromLocale(locales_obj); - ICUCachePair pair = std::make_pair(locale, icu_formatter); - - auto it = icu_object_cache_.find(cache_type); - if (it == icu_object_cache_.end()) { - icu_object_cache_.insert({cache_type, pair}); - } else { - it->second = pair; - } +void Isolate::set_icu_object_in_cache(ICUObjectCacheType cache_type, + Handle locales, + std::shared_ptr obj) { + icu_object_cache_[static_cast(cache_type)] = { + GetStringFromLocales(this, locales), std::move(obj)}; } void Isolate::clear_cached_icu_object(ICUObjectCacheType cache_type) { - icu_object_cache_.erase(cache_type); + icu_object_cache_[static_cast(cache_type)] = ICUObjectCacheEntry{}; } -void Isolate::ClearCachedIcuObjects() { icu_object_cache_.clear(); } +void Isolate::clear_cached_icu_objects() { + for (int i = 0; i < kICUObjectCacheTypeCount; i++) { + clear_cached_icu_object(static_cast(i)); + } +} #endif // V8_INTL_SUPPORT diff --git a/src/execution/isolate.h b/src/execution/isolate.h index e7908eac6a..92a4e74399 100644 --- a/src/execution/isolate.h +++ b/src/execution/isolate.h @@ -1350,18 +1350,18 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory { default_locale_ = locale; } - // enum to access the icu object cache. enum class ICUObjectCacheType{ kDefaultCollator, kDefaultNumberFormat, kDefaultSimpleDateFormat, kDefaultSimpleDateFormatForTime, kDefaultSimpleDateFormatForDate}; + static constexpr int kICUObjectCacheTypeCount = 5; icu::UMemory* get_cached_icu_object(ICUObjectCacheType cache_type, Handle locales); void set_icu_object_in_cache(ICUObjectCacheType cache_type, - Handle locale, + Handle locales, std::shared_ptr obj); void clear_cached_icu_object(ICUObjectCacheType cache_type); - void ClearCachedIcuObjects(); + void clear_cached_icu_objects(); #endif // V8_INTL_SUPPORT @@ -2047,14 +2047,18 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory { #ifdef V8_INTL_SUPPORT std::string default_locale_; - struct ICUObjectCacheTypeHash { - std::size_t operator()(ICUObjectCacheType a) const { - return static_cast(a); - } + // The cache stores the most recently accessed {locales,obj} pair for each + // cache type. + struct ICUObjectCacheEntry { + std::string locales; + std::shared_ptr obj; + + ICUObjectCacheEntry() = default; + ICUObjectCacheEntry(std::string locales, std::shared_ptr obj) + : locales(locales), obj(std::move(obj)) {} }; - typedef std::pair> ICUCachePair; - std::unordered_map - icu_object_cache_; + + ICUObjectCacheEntry icu_object_cache_[kICUObjectCacheTypeCount]; #endif // V8_INTL_SUPPORT // true if being profiled. Causes collection of extra compile info. diff --git a/src/objects/intl-objects.cc b/src/objects/intl-objects.cc index 99a7d62098..9c6af2fa53 100644 --- a/src/objects/intl-objects.cc +++ b/src/objects/intl-objects.cc @@ -999,7 +999,7 @@ MaybeHandle Intl::StringLocaleConvertCase(Isolate* isolate, } } -MaybeHandle Intl::StringLocaleCompare( +base::Optional Intl::StringLocaleCompare( Isolate* isolate, Handle string1, Handle string2, Handle locales, Handle options, const char* method) { // We only cache the instance when locales is a string/undefined and @@ -1025,9 +1025,9 @@ MaybeHandle Intl::StringLocaleCompare( isolate); Handle collator; - ASSIGN_RETURN_ON_EXCEPTION( - isolate, collator, - New(isolate, constructor, locales, options, method), Object); + MaybeHandle maybe_collator = + New(isolate, constructor, locales, options, method); + if (!maybe_collator.ToHandle(&collator)) return {}; if (can_cache) { isolate->set_icu_object_in_cache( Isolate::ICUObjectCacheType::kDefaultCollator, locales, @@ -1038,26 +1038,19 @@ MaybeHandle Intl::StringLocaleCompare( } // ecma402/#sec-collator-comparestrings -Handle Intl::CompareStrings(Isolate* isolate, - const icu::Collator& icu_collator, - Handle string1, - Handle string2) { - Factory* factory = isolate->factory(); - +int Intl::CompareStrings(Isolate* isolate, const icu::Collator& icu_collator, + Handle string1, Handle string2) { // Early return for identical strings. if (string1.is_identical_to(string2)) { - return factory->NewNumberFromInt(UCollationResult::UCOL_EQUAL); + return UCollationResult::UCOL_EQUAL; } // Early return for empty strings. if (string1->length() == 0) { - return factory->NewNumberFromInt(string2->length() == 0 - ? UCollationResult::UCOL_EQUAL - : UCollationResult::UCOL_LESS); - } - if (string2->length() == 0) { - return factory->NewNumberFromInt(UCollationResult::UCOL_GREATER); + return string2->length() == 0 ? UCollationResult::UCOL_EQUAL + : UCollationResult::UCOL_LESS; } + if (string2->length() == 0) return UCollationResult::UCOL_GREATER; string1 = String::Flatten(isolate, string1); string2 = String::Flatten(isolate, string2); @@ -1070,7 +1063,7 @@ Handle Intl::CompareStrings(Isolate* isolate, if (!string_piece2.empty()) { result = icu_collator.compareUTF8(string_piece1, string_piece2, status); DCHECK(U_SUCCESS(status)); - return factory->NewNumberFromInt(result); + return result; } } @@ -1078,8 +1071,7 @@ Handle Intl::CompareStrings(Isolate* isolate, icu::UnicodeString string_val2 = Intl::ToICUUnicodeString(isolate, string2); result = icu_collator.compare(string_val1, string_val2, status); DCHECK(U_SUCCESS(status)); - - return factory->NewNumberFromInt(result); + return result; } // ecma402/#sup-properties-of-the-number-prototype-object diff --git a/src/objects/intl-objects.h b/src/objects/intl-objects.h index 122ca4b746..ec5f45e36c 100644 --- a/src/objects/intl-objects.h +++ b/src/objects/intl-objects.h @@ -158,13 +158,14 @@ class Intl { V8_WARN_UNUSED_RESULT static MaybeHandle ConvertToLower( Isolate* isolate, Handle s); - V8_WARN_UNUSED_RESULT static MaybeHandle StringLocaleCompare( + V8_WARN_UNUSED_RESULT static base::Optional StringLocaleCompare( Isolate* isolate, Handle s1, Handle s2, Handle locales, Handle options, const char* method); - V8_WARN_UNUSED_RESULT static Handle CompareStrings( - Isolate* isolate, const icu::Collator& collator, Handle s1, - Handle s2); + V8_WARN_UNUSED_RESULT static int CompareStrings(Isolate* isolate, + const icu::Collator& collator, + Handle s1, + Handle s2); // ecma402/#sup-properties-of-the-number-prototype-object V8_WARN_UNUSED_RESULT static MaybeHandle NumberToLocaleString(