Skip to content

Commit 90109a4

Browse files
parkeraparkera
authored
Revert formatting performance improvements (#744)
There appears to be some kind of race or memory smash in ICU after these, and we need more time to investigate the full root cause.
1 parent 8db56a2 commit 90109a4

File tree

16 files changed

+492
-379
lines changed

16 files changed

+492
-379
lines changed

‎Benchmarks/Benchmarks/Formatting/BenchmarkFormatting.swift‎

Lines changed: 0 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,11 @@
1212

1313
import Benchmark
1414
import func Benchmark.blackHole
15-
import Dispatch
1615

1716
#if FOUNDATION_FRAMEWORK
1817
import Foundation
1918
#else
2019
import FoundationEssentials
21-
import FoundationInternationalization
2220
#endif
2321

2422
let benchmarks = {
@@ -59,35 +57,4 @@ let benchmarks = {
5957
}
6058
}
6159
}
62-
63-
Benchmark("parallel-number-formatting", configuration: .init(scalingFactor: .kilo)) { benchmark in
64-
for _ in benchmark.scaledIterations {
65-
DispatchQueue.concurrentPerform(iterations: 1000) { _ in
66-
let result = 10.123.formatted()
67-
blackHole(result)
68-
}
69-
}
70-
}
71-
72-
Benchmark("parallel-and-serialized-number-formatting", configuration: .init(scalingFactor: .kilo)) { benchmark in
73-
for _ in benchmark.scaledIterations {
74-
DispatchQueue.concurrentPerform(iterations: 10) { _ in
75-
// Reuse the values on this thread a bunch
76-
for _ in 0..<100 {
77-
let result = 10.123.formatted()
78-
blackHole(result)
79-
}
80-
}
81-
}
82-
}
83-
84-
Benchmark("serialized-number-formatting", configuration: .init(scalingFactor: .kilo)) { benchmark in
85-
for _ in benchmark.scaledIterations {
86-
for _ in 0..<1000 {
87-
let result = 10.123.formatted()
88-
blackHole(result)
89-
}
90-
}
91-
}
92-
9360
}

‎Sources/FoundationEssentials/Calendar/Calendar.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -319,7 +319,7 @@ public struct Calendar : Hashable, Equatable, Sendable {
319319
///
320320
/// - note: The autoupdating Calendar will only compare equal to another autoupdating Calendar.
321321
public static var autoupdatingCurrent : Calendar {
322-
Calendar(inner: CalendarCache.autoupdatingCurrent)
322+
Calendar(inner: CalendarCache.cache.autoupdatingCurrent)
323323
}
324324

325325
// MARK: -

‎Sources/FoundationEssentials/Calendar/Calendar_Cache.swift‎

Lines changed: 85 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import CoreFoundation
1616
#endif
1717

1818
/// Singleton which listens for notifications about preference changes for Calendar and holds cached singletons for the current locale, calendar, and time zone.
19-
struct CalendarCache : Sendable, ~Copyable {
19+
struct CalendarCache : Sendable {
2020

2121
// MARK: - Concrete Classes
2222

@@ -38,71 +38,103 @@ struct CalendarCache : Sendable, ~Copyable {
3838
}
3939
#endif
4040
}
41+
42+
// MARK: - State
4143

42-
static let cache = CalendarCache()
43-
44-
// The values stored in these two locks do not depend upon each other, so it is safe to access them with separate locks. This helps avoids contention on a single lock.
45-
46-
private let _current = LockedState<(any _CalendarProtocol)?>(initialState: nil)
47-
private let _fixed = LockedState<[Calendar.Identifier: any _CalendarProtocol]>(initialState: [:])
48-
49-
fileprivate init() {
50-
}
51-
52-
var current: any _CalendarProtocol {
53-
if let result = _current.withLock({ $0 }) {
54-
return result
44+
struct State : Sendable {
45+
// If nil, the calendar has been invalidated and will be created next time State.current() is called
46+
private var currentCalendar: (any _CalendarProtocol)?
47+
private var autoupdatingCurrentCalendar: _CalendarAutoupdating?
48+
private var fixedCalendars: [Calendar.Identifier: any _CalendarProtocol] = [:]
49+
50+
private var noteCount = -1
51+
private var wasResetManually = false
52+
53+
mutating func check() {
54+
#if FOUNDATION_FRAMEWORK
55+
// On Darwin we listen for certain distributed notifications to reset the current Calendar.
56+
let newNoteCount = _CFLocaleGetNoteCount() + _CFTimeZoneGetNoteCount() + Int(_CFCalendarGetMidnightNoteCount())
57+
#else
58+
let newNoteCount = 1
59+
#endif
60+
if newNoteCount != noteCount || wasResetManually {
61+
// rdar://102017659
62+
// Don't create `currentCalendar` here to avoid deadlocking when retrieving a fixed
63+
// calendar. Creating the current calendar gets the current locale, decodes a plist
64+
// from CFPreferences, and may call +[NSDate initialize] on a separate thread. This
65+
// leads to a deadlock if we are also initializing a class on the current thread
66+
currentCalendar = nil
67+
fixedCalendars = [:]
68+
69+
noteCount = newNoteCount
70+
wasResetManually = false
71+
}
72+
}
73+
74+
mutating func current() -> any _CalendarProtocol {
75+
check()
76+
if let currentCalendar {
77+
return currentCalendar
78+
} else {
79+
let id = Locale.current._calendarIdentifier
80+
// If we cannot create the right kind of class, we fail immediately here
81+
let calendarClass = CalendarCache.calendarICUClass(identifier: id, useGregorian: true)!
82+
let calendar = calendarClass.init(identifier: id, timeZone: nil, locale: Locale.current, firstWeekday: nil, minimumDaysInFirstWeek: nil, gregorianStartDate: nil)
83+
currentCalendar = calendar
84+
return calendar
85+
}
5586
}
56-
57-
let id = Locale.current._calendarIdentifier
58-
// If we cannot create the right kind of class, we fail immediately here
59-
let calendarClass = CalendarCache.calendarICUClass(identifier: id, useGregorian: true)!
60-
let calendar = calendarClass.init(identifier: id, timeZone: nil, locale: Locale.current, firstWeekday: nil, minimumDaysInFirstWeek: nil, gregorianStartDate: nil)
6187

62-
return _current.withLock {
63-
if let current = $0 {
64-
// Someone beat us to setting it - use the existing one
65-
return current
88+
mutating func autoupdatingCurrent() -> any _CalendarProtocol {
89+
if let autoupdatingCurrentCalendar {
90+
return autoupdatingCurrentCalendar
6691
} else {
67-
$0 = calendar
92+
let calendar = _CalendarAutoupdating()
93+
autoupdatingCurrentCalendar = calendar
6894
return calendar
6995
}
7096
}
97+
98+
mutating func fixed(_ id: Calendar.Identifier) -> any _CalendarProtocol {
99+
check()
100+
if let cached = fixedCalendars[id] {
101+
return cached
102+
} else {
103+
// If we cannot create the right kind of class, we fail immediately here
104+
let calendarClass = CalendarCache.calendarICUClass(identifier: id, useGregorian: true)!
105+
let new = calendarClass.init(identifier: id, timeZone: nil, locale: nil, firstWeekday: nil, minimumDaysInFirstWeek: nil, gregorianStartDate: nil)
106+
fixedCalendars[id] = new
107+
return new
108+
}
109+
}
110+
111+
mutating func reset() {
112+
wasResetManually = true
113+
}
71114
}
72-
115+
116+
let lock: LockedState<State>
117+
118+
static let cache = CalendarCache()
119+
120+
fileprivate init() {
121+
lock = LockedState(initialState: State())
122+
}
123+
73124
func reset() {
74-
// rdar://102017659
75-
// Don't create `currentCalendar` here to avoid deadlocking when retrieving a fixed
76-
// calendar. Creating the current calendar gets the current locale, decodes a plist
77-
// from CFPreferences, and may call +[NSDate initialize] on a separate thread. This
78-
// leads to a deadlock if we are also initializing a class on the current thread
79-
_current.withLock { $0 = nil }
80-
_fixed.withLock { $0 = [:] }
125+
lock.withLock { $0.reset() }
126+
}
127+
128+
var current: any _CalendarProtocol {
129+
lock.withLock { $0.current() }
81130
}
82131

83-
// MARK: Singletons
84-
85-
static let autoupdatingCurrent = _CalendarAutoupdating()
86-
87-
// MARK: -
132+
var autoupdatingCurrent: any _CalendarProtocol {
133+
lock.withLock { $0.autoupdatingCurrent() }
134+
}
88135

89136
func fixed(_ id: Calendar.Identifier) -> any _CalendarProtocol {
90-
if let existing = _fixed.withLock({ $0[id] }) {
91-
return existing
92-
}
93-
94-
// If we cannot create the right kind of class, we fail immediately here
95-
let calendarClass = CalendarCache.calendarICUClass(identifier: id, useGregorian: true)!
96-
let new = calendarClass.init(identifier: id, timeZone: nil, locale: nil, firstWeekday: nil, minimumDaysInFirstWeek: nil, gregorianStartDate: nil)
97-
98-
return _fixed.withLock {
99-
if let existing = $0[id] {
100-
return existing
101-
} else {
102-
$0[id] = new
103-
return new
104-
}
105-
}
137+
lock.withLock { $0.fixed(id) }
106138
}
107139

108140
func fixed(identifier: Calendar.Identifier, locale: Locale?, timeZone: TimeZone?, firstWeekday: Int?, minimumDaysInFirstWeek: Int?, gregorianStartDate: Date?) -> any _CalendarProtocol {

‎Sources/FoundationEssentials/Formatting/FormatterCache.swift‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
//===----------------------------------------------------------------------===//
1212

1313
package struct FormatterCache<Format : Hashable & Sendable, FormattingType: Sendable>: Sendable {
14+
1415
let countLimit = 100
1516

1617
private let _lock: LockedState<[Format: FormattingType]>

‎Sources/FoundationEssentials/Locale/Locale.swift‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ public struct Locale : Hashable, Equatable, Sendable {
6565
///
6666
/// - note: The autoupdating Locale will only compare equal to another autoupdating Locale.
6767
public static var autoupdatingCurrent : Locale {
68-
Locale(inner: LocaleCache.autoupdatingCurrent)
68+
Locale(inner: LocaleCache.cache.autoupdatingCurrent)
6969
}
7070

7171
/// Returns the user's current locale.
@@ -75,12 +75,12 @@ public struct Locale : Hashable, Equatable, Sendable {
7575

7676
/// System locale.
7777
internal static var system : Locale {
78-
Locale(inner: LocaleCache.system)
78+
Locale(inner: LocaleCache.cache.system)
7979
}
8080

8181
/// Unlocalized locale (`en_001`).
8282
internal static var unlocalized : Locale {
83-
Locale(inner: LocaleCache.unlocalized)
83+
Locale(inner: LocaleCache.cache.unlocalized)
8484
}
8585

8686
#if FOUNDATION_FRAMEWORK && canImport(_FoundationICU)

‎Sources/FoundationEssentials/Locale/Locale_Autoupdating.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -263,7 +263,7 @@ internal final class _LocaleAutoupdating : _LocaleProtocol, @unchecked Sendable
263263
}
264264

265265
func bridgeToNSLocale() -> NSLocale {
266-
LocaleCache.autoupdatingCurrentNSLocale
266+
LocaleCache.cache.autoupdatingCurrentNSLocale()
267267
}
268268
#endif
269269

0 commit comments

Comments
 (0)