Skip to content

Commit dcd7a97

Browse files
authored
Short-circuit logic to get locale that requires special case handling. (#937)
This fix targets at `__CFLocaleGetDoesNotRequireSpecialCaseHandling`. Currently, this function calls into `class _NSSwiftLocale`, then `struct Locale`, then `any LocaleProtocol` (`LocaleICU` here), which calls into a `static func` of `struct Locale`. All the hoop jumping contributes to a lot of retain and release calls. It turns out that this function is only used by this one call site (`_CFStrGetSpecialCaseHandlingLanguageIdentifierForLocale`). This change simplifies the calling chain so that we call the static function directly from `_NSSwiftLocale`, and the result is cached inside this class directly. I've verified that this change brings down the time spent in `CFStringCompareWithOptionsAndLocale` of the reproducable case in the radar down from 904ms to 397ms. Also added a BenchmarkLocale target. `CFStringCompareWithOptionsAndLocale` benchmark result **before** the change: ``` ╒═══════════════════════════╤═════════╤═════════╤═════════╤═════════╤═════════╤═════════╤═════════╤═════════╕ │ Metric │ p0 │ p25 │ p50 │ p75 │ p90 │ p99 │ p100 │ Samples │ ╞═══════════════════════════╪═════════╪═════════╪═════════╪═════════╪═════════╪═════════╪═════════╪═════════╡ │ Malloc (total) * │ 0 │ 0 │ 0 │ 0 │ 0 │ 0 │ 0 │ 448 │ ├───────────────────────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┤ │ Throughput (# / s) (M) │ 163 │ 153 │ 150 │ 146 │ 144 │ 129 │ 123 │ 448 │ ├───────────────────────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┤ │ Time (total CPU) (ns) * │ 6 │ 7 │ 7 │ 7 │ 7 │ 7 │ 8 │ 448 │ ├───────────────────────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┤ │ Time (wall clock) (ns) * │ 6 │ 7 │ 7 │ 7 │ 7 │ 8 │ 8 │ 448 │ ╘═══════════════════════════╧═════════╧═════════╧═════════╧═════════╧═════════╧═════════╧═════════╧═════════╛ ``` And **after** the change: ``` ╒═══════════════════════════╤═════════╤═════════╤═════════╤═════════╤═════════╤═════════╤═════════╤═════════╕ │ Metric │ p0 │ p25 │ p50 │ p75 │ p90 │ p99 │ p100 │ Samples │ ╞═══════════════════════════╪═════════╪═════════╪═════════╪═════════╪═════════╪═════════╪═════════╪═════════╡ │ Malloc (total) * │ 0 │ 0 │ 0 │ 0 │ 0 │ 0 │ 0 │ 476 │ ├───────────────────────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┤ │ Throughput (# / s) (M) │ 172 │ 161 │ 158 │ 156 │ 155 │ 153 │ 136 │ 476 │ ├───────────────────────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┤ │ Time (total CPU) (ns) * │ 6 │ 6 │ 6 │ 6 │ 6 │ 7 │ 7 │ 476 │ ├───────────────────────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┤ │ Time (wall clock) (ns) * │ 6 │ 6 │ 6 │ 6 │ 6 │ 7 │ 7 │ 476 │ ╘═══════════════════════════╧═════════╧═════════╧═════════╧═════════╧═════════╧═════════╧═════════╧═════════╛ ``` Resolves rdar://134912852
1 parent d1da0f7 commit dcd7a97

File tree

8 files changed

+52
-27
lines changed

8 files changed

+52
-27
lines changed
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
//===----------------------------------------------------------------------===//
2+
//
3+
// This source file is part of the Swift.org open source project
4+
//
5+
// Copyright (c) 2022-2023 Apple Inc. and the Swift project authors
6+
// Licensed under Apache License v2.0 with Runtime Library Exception
7+
//
8+
// See https://swift.org/LICENSE.txt for license information
9+
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
10+
//
11+
//===----------------------------------------------------------------------===//
12+
13+
import Benchmark
14+
import func Benchmark.blackHole
15+
16+
#if FOUNDATION_FRAMEWORK // This test uses CFString
17+
import Foundation
18+
19+
let benchmarks = {
20+
Benchmark.defaultConfiguration.maxIterations = 1_000
21+
Benchmark.defaultConfiguration.maxDuration = .seconds(3)
22+
Benchmark.defaultConfiguration.scalingFactor = .kilo
23+
Benchmark.defaultConfiguration.metrics = [.cpuTotal, .wallClock, .mallocCountTotal, .throughput]
24+
25+
let string1 = "aaA" as CFString
26+
let string2 = "AAà" as CFString
27+
let range1 = CFRange(location: 0, length: CFStringGetLength(string1))
28+
let nsLocales = Locale.availableIdentifiers.map {
29+
NSLocale(localeIdentifier: $0)
30+
}
31+
32+
Benchmark("CFStringCompareWithOptionsAndLocale", configuration: .init(scalingFactor: .mega)) { benchmark in
33+
for nsLocale in nsLocales {
34+
CFStringCompareWithOptionsAndLocale(string1, string2, range1, .init(rawValue: 0), nsLocale)
35+
}
36+
}
37+
}
38+
#endif

Sources/FoundationEssentials/Locale/Locale.swift

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -742,11 +742,6 @@ public struct Locale : Hashable, Equatable, Sendable {
742742
/// - "nl": Dutch
743743
/// - "el": Greek
744744
/// For all other locales such as en_US, this is `false`.
745-
internal var doesNotRequireSpecialCaseHandling: Bool {
746-
// We call into the _LocaleBase because this value is cached in many cases. They will defer the initial calculation to the below helper function.
747-
_locale.doesNotRequireSpecialCaseHandling
748-
}
749-
750745
package static func identifierDoesNotRequireSpecialCaseHandling(_ identifier: String) -> Bool {
751746
guard identifier.count >= 2 else { return true }
752747

Sources/FoundationEssentials/Locale/Locale_Autoupdating.swift

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -252,10 +252,7 @@ internal final class _LocaleAutoupdating : _LocaleProtocol, @unchecked Sendable
252252
var identifierCapturingPreferences: String {
253253
LocaleCache.cache.current.identifierCapturingPreferences
254254
}
255-
256-
var doesNotRequireSpecialCaseHandling: Bool {
257-
LocaleCache.cache.current.doesNotRequireSpecialCaseHandling
258-
}
255+
259256

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

Sources/FoundationEssentials/Locale/Locale_Protocol.swift

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -86,8 +86,6 @@ package protocol _LocaleProtocol : AnyObject, Sendable, CustomDebugStringConvert
8686

8787
var identifierCapturingPreferences: String { get }
8888

89-
var doesNotRequireSpecialCaseHandling: Bool { get }
90-
9189
#if FOUNDATION_FRAMEWORK
9290
func pref(for key: String) -> Any?
9391

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

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

109103
package var regionCode: String? {
110104
region?.identifier

Sources/FoundationInternationalization/Locale/Locale_Bridge.swift

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -316,10 +316,6 @@ internal final class _LocaleBridged: _LocaleProtocol, @unchecked Sendable {
316316
func pref(for key: String) -> Any? {
317317
nil
318318
}
319-
320-
var doesNotRequireSpecialCaseHandling: Bool {
321-
Locale.identifierDoesNotRequireSpecialCaseHandling(identifier)
322-
}
323319
}
324320

325321
#endif

Sources/FoundationInternationalization/Locale/Locale_ICU.swift

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -132,8 +132,6 @@ internal final class _LocaleICU: _LocaleProtocol, Sendable {
132132
let identifierCapturingPreferences: String
133133
let calendarIdentifier: Calendar.Identifier
134134

135-
let doesNotRequireSpecialCaseHandling: Bool
136-
137135
let prefs: LocalePreferences?
138136

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

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

167164
required init(components: Locale.Components) {
168165
self.identifier = components.icuIdentifier
169-
doesNotRequireSpecialCaseHandling = Locale.identifierDoesNotRequireSpecialCaseHandling(self.identifier)
170166
prefs = nil
171167
calendarIdentifier = Self._calendarIdentifier(forIdentifier: self.identifier)
172168
identifierCapturingPreferences = Self._identifierCapturingPreferences(forIdentifier: self.identifier, calendarIdentifier: calendarIdentifier, preferences: prefs)
@@ -284,7 +280,6 @@ internal final class _LocaleICU: _LocaleProtocol, Sendable {
284280
}
285281

286282
self.identifier = Locale._canonicalLocaleIdentifier(from: fixedIdent)
287-
doesNotRequireSpecialCaseHandling = Locale.identifierDoesNotRequireSpecialCaseHandling(self.identifier)
288283
self.prefs = prefs
289284
calendarIdentifier = Self._calendarIdentifier(forIdentifier: self.identifier)
290285
identifierCapturingPreferences = Self._identifierCapturingPreferences(forIdentifier: self.identifier, calendarIdentifier: calendarIdentifier, preferences: prefs)

Sources/FoundationInternationalization/Locale/Locale_ObjC.swift

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,7 @@ extension NSLocale {
219219
@objc(_doesNotRequireSpecialCaseHandling)
220220
func _doesNotRequireSpecialCaseHandling() -> Bool {
221221
// Unable to use cached locale; create a new one. Subclass `_NSSwiftLocale` implements a better version
222-
return Locale(identifier: localeIdentifier).doesNotRequireSpecialCaseHandling
222+
Locale.identifierDoesNotRequireSpecialCaseHandling(localeIdentifier)
223223
}
224224
}
225225

@@ -228,9 +228,13 @@ extension NSLocale {
228228
@objc(_NSSwiftLocale)
229229
internal class _NSSwiftLocale: _NSLocaleBridge, @unchecked Sendable {
230230
var locale: Locale
231+
var doesNotRequireSpecialHandling: Bool?
231232

232233
internal init(_ locale: Locale) {
233234
self.locale = locale
235+
// 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.
236+
self.doesNotRequireSpecialHandling = nil
237+
234238
// 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.
235239
super.init(localeIdentifier: "")
236240
}
@@ -244,9 +248,10 @@ internal class _NSSwiftLocale: _NSLocaleBridge, @unchecked Sendable {
244248
return NSLocale.self
245249
}
246250
}
247-
251+
248252
override init(localeIdentifier string: String) {
249253
self.locale = Locale(identifier: string)
254+
self.doesNotRequireSpecialHandling = Locale.identifierDoesNotRequireSpecialCaseHandling(string)
250255
super.init(localeIdentifier: "")
251256
}
252257

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

268273
locale = Locale(identifier: ident)
274+
doesNotRequireSpecialHandling = Locale.identifierDoesNotRequireSpecialCaseHandling(ident)
269275

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

523529
override func _doesNotRequireSpecialCaseHandling() -> Bool {
524-
return locale.doesNotRequireSpecialCaseHandling
530+
if let doesNotRequireSpecialHandling {
531+
return doesNotRequireSpecialHandling
532+
}
533+
534+
return Locale.identifierDoesNotRequireSpecialCaseHandling(locale.identifier)
525535
}
526536
}
527537

0 commit comments

Comments
 (0)