Skip to content

Short-circuit logic to get locale that requires special case handling. #937

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions Benchmarks/Benchmarks/Internationalization/BenchmarkLocale.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
//===----------------------------------------------------------------------===//
//
// This source file is part of the Swift.org open source project
//
// Copyright (c) 2022-2023 Apple Inc. and the Swift project authors
// Licensed under Apache License v2.0 with Runtime Library Exception
//
// See https://swift.org/LICENSE.txt for license information
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
//
//===----------------------------------------------------------------------===//

import Benchmark
import func Benchmark.blackHole

#if FOUNDATION_FRAMEWORK // This test uses CFString
import Foundation

let benchmarks = {
Benchmark.defaultConfiguration.maxIterations = 1_000
Benchmark.defaultConfiguration.maxDuration = .seconds(3)
Benchmark.defaultConfiguration.scalingFactor = .kilo
Benchmark.defaultConfiguration.metrics = [.cpuTotal, .wallClock, .mallocCountTotal, .throughput]

let string1 = "aaA" as CFString
let string2 = "AAà" as CFString
let range1 = CFRange(location: 0, length: CFStringGetLength(string1))
let nsLocales = Locale.availableIdentifiers.map {
NSLocale(localeIdentifier: $0)
}

Benchmark("CFStringCompareWithOptionsAndLocale", configuration: .init(scalingFactor: .mega)) { benchmark in
for nsLocale in nsLocales {
CFStringCompareWithOptionsAndLocale(string1, string2, range1, .init(rawValue: 0), nsLocale)
}
}
}
#endif
5 changes: 0 additions & 5 deletions Sources/FoundationEssentials/Locale/Locale.swift
Original file line number Diff line number Diff line change
Expand Up @@ -742,11 +742,6 @@ public struct Locale : Hashable, Equatable, Sendable {
/// - "nl": Dutch
/// - "el": Greek
/// For all other locales such as en_US, this is `false`.
internal var doesNotRequireSpecialCaseHandling: Bool {
// We call into the _LocaleBase because this value is cached in many cases. They will defer the initial calculation to the below helper function.
_locale.doesNotRequireSpecialCaseHandling
}

package static func identifierDoesNotRequireSpecialCaseHandling(_ identifier: String) -> Bool {
guard identifier.count >= 2 else { return true }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -252,10 +252,7 @@ internal final class _LocaleAutoupdating : _LocaleProtocol, @unchecked Sendable
var identifierCapturingPreferences: String {
LocaleCache.cache.current.identifierCapturingPreferences
}

var doesNotRequireSpecialCaseHandling: Bool {
LocaleCache.cache.current.doesNotRequireSpecialCaseHandling
}


#if FOUNDATION_FRAMEWORK
func pref(for key: String) -> Any? {
Expand Down
6 changes: 0 additions & 6 deletions Sources/FoundationEssentials/Locale/Locale_Protocol.swift
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,6 @@ package protocol _LocaleProtocol : AnyObject, Sendable, CustomDebugStringConvert

var identifierCapturingPreferences: String { get }

var doesNotRequireSpecialCaseHandling: Bool { get }

#if FOUNDATION_FRAMEWORK
func pref(for key: String) -> Any?

Expand All @@ -101,10 +99,6 @@ package protocol _LocaleProtocol : AnyObject, Sendable, CustomDebugStringConvert
}

extension _LocaleProtocol {
package var doesNotRequireSpecialCaseHandling: Bool {
// Some implementations may cache this value, but we can provide a default value here.
Locale.identifierDoesNotRequireSpecialCaseHandling(identifier)
}

package var regionCode: String? {
region?.identifier
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -316,10 +316,6 @@ internal final class _LocaleBridged: _LocaleProtocol, @unchecked Sendable {
func pref(for key: String) -> Any? {
nil
}

var doesNotRequireSpecialCaseHandling: Bool {
Locale.identifierDoesNotRequireSpecialCaseHandling(identifier)
}
}

#endif
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,6 @@ internal final class _LocaleICU: _LocaleProtocol, Sendable {
let identifierCapturingPreferences: String
let calendarIdentifier: Calendar.Identifier

let doesNotRequireSpecialCaseHandling: Bool

let prefs: LocalePreferences?

private let lock: LockedState<State>
Expand All @@ -157,7 +155,6 @@ internal final class _LocaleICU: _LocaleProtocol, Sendable {

required init(identifier: String, prefs: LocalePreferences? = nil) {
self.identifier = Locale._canonicalLocaleIdentifier(from: identifier)
doesNotRequireSpecialCaseHandling = Locale.identifierDoesNotRequireSpecialCaseHandling(self.identifier)
self.prefs = prefs
calendarIdentifier = Self._calendarIdentifier(forIdentifier: self.identifier)
identifierCapturingPreferences = Self._identifierCapturingPreferences(forIdentifier: self.identifier, calendarIdentifier: calendarIdentifier, preferences: prefs)
Expand All @@ -166,7 +163,6 @@ internal final class _LocaleICU: _LocaleProtocol, Sendable {

required init(components: Locale.Components) {
self.identifier = components.icuIdentifier
doesNotRequireSpecialCaseHandling = Locale.identifierDoesNotRequireSpecialCaseHandling(self.identifier)
prefs = nil
calendarIdentifier = Self._calendarIdentifier(forIdentifier: self.identifier)
identifierCapturingPreferences = Self._identifierCapturingPreferences(forIdentifier: self.identifier, calendarIdentifier: calendarIdentifier, preferences: prefs)
Expand Down Expand Up @@ -284,7 +280,6 @@ internal final class _LocaleICU: _LocaleProtocol, Sendable {
}

self.identifier = Locale._canonicalLocaleIdentifier(from: fixedIdent)
doesNotRequireSpecialCaseHandling = Locale.identifierDoesNotRequireSpecialCaseHandling(self.identifier)
self.prefs = prefs
calendarIdentifier = Self._calendarIdentifier(forIdentifier: self.identifier)
identifierCapturingPreferences = Self._identifierCapturingPreferences(forIdentifier: self.identifier, calendarIdentifier: calendarIdentifier, preferences: prefs)
Expand Down
16 changes: 13 additions & 3 deletions Sources/FoundationInternationalization/Locale/Locale_ObjC.swift
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ extension NSLocale {
@objc(_doesNotRequireSpecialCaseHandling)
func _doesNotRequireSpecialCaseHandling() -> Bool {
// Unable to use cached locale; create a new one. Subclass `_NSSwiftLocale` implements a better version
return Locale(identifier: localeIdentifier).doesNotRequireSpecialCaseHandling
Locale.identifierDoesNotRequireSpecialCaseHandling(localeIdentifier)
}
}

Expand All @@ -228,9 +228,13 @@ extension NSLocale {
@objc(_NSSwiftLocale)
internal class _NSSwiftLocale: _NSLocaleBridge, @unchecked Sendable {
var locale: Locale
var doesNotRequireSpecialHandling: Bool?

internal init(_ locale: Locale) {
self.locale = locale
// We cannot call `locale.identifier` to get the actual value here because that could trigger a recursive call into LocaleCache. If we enter from this `init` just lazily fetch the variable.
self.doesNotRequireSpecialHandling = nil

// The superclass does not care at all what the identifier is. Avoid a potentially recursive call into the Locale cache here by just using an empty string.
super.init(localeIdentifier: "")
}
Expand All @@ -244,9 +248,10 @@ internal class _NSSwiftLocale: _NSLocaleBridge, @unchecked Sendable {
return NSLocale.self
}
}

override init(localeIdentifier string: String) {
self.locale = Locale(identifier: string)
self.doesNotRequireSpecialHandling = Locale.identifierDoesNotRequireSpecialCaseHandling(string)
super.init(localeIdentifier: "")
}

Expand All @@ -266,6 +271,7 @@ internal class _NSSwiftLocale: _NSLocaleBridge, @unchecked Sendable {
}

locale = Locale(identifier: ident)
doesNotRequireSpecialHandling = Locale.identifierDoesNotRequireSpecialCaseHandling(ident)

// Must call a DI; this one does nothing so it's safe to call here.
super.init(localeIdentifier: "")
Expand Down Expand Up @@ -521,7 +527,11 @@ internal class _NSSwiftLocale: _NSLocaleBridge, @unchecked Sendable {
}

override func _doesNotRequireSpecialCaseHandling() -> Bool {
return locale.doesNotRequireSpecialCaseHandling
if let doesNotRequireSpecialHandling {
return doesNotRequireSpecialHandling
}

return Locale.identifierDoesNotRequireSpecialCaseHandling(locale.identifier)
}
}

Expand Down