Skip to content

Commit b4d43d0

Browse files
asfernandesasfernandes
committed
Fix #7854 - Performance issue with time zones. (#7859)
1 parent 42238f1 commit b4d43d0

File tree

2 files changed

+76
-41
lines changed

2 files changed

+76
-41
lines changed

‎src/common/TimeZoneUtil.cpp‎

Lines changed: 28 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -45,10 +45,20 @@ namespace
4545
public:
4646
TimeZoneDesc(MemoryPool& pool)
4747
: asciiName(pool),
48-
unicodeName(pool)
48+
unicodeName(pool),
49+
icuCachedCalendar(nullptr)
4950
{
5051
}
5152

53+
~TimeZoneDesc()
54+
{
55+
if (const auto calendar = icuCachedCalendar.exchange(nullptr))
56+
{
57+
auto& icuLib = Jrd::UnicodeUtil::getConversionICU();
58+
icuLib.ucalClose(calendar);
59+
}
60+
}
61+
5262
public:
5363
void setName(const char* name)
5464
{
@@ -70,9 +80,21 @@ namespace
7080
return unicodeName.begin();
7181
}
7282

83+
IcuCalendarWrapper getCalendar(const Jrd::UnicodeUtil::ConversionICU& icuLib, UErrorCode* err = nullptr) const
84+
{
85+
auto calendar = icuCachedCalendar.exchange(nullptr);
86+
UErrorCode internalErr = U_ZERO_ERROR;
87+
88+
if (!calendar)
89+
calendar = icuLib.ucalOpen(getUnicodeName(), -1, nullptr, UCAL_GREGORIAN, (err ? err : &internalErr));
90+
91+
return IcuCalendarWrapper(calendar, &icuCachedCalendar);
92+
}
93+
7394
private:
7495
string asciiName;
7596
Array<UChar> unicodeName;
97+
mutable std::atomic<UCalendar*> icuCachedCalendar;
7698
};
7799
}
78100

@@ -594,30 +616,21 @@ void TimeZoneUtil::extractOffset(const ISC_TIMESTAMP_TZ& timeStampTz, SSHORT* of
594616

595617
Jrd::UnicodeUtil::ConversionICU& icuLib = Jrd::UnicodeUtil::getConversionICU();
596618

597-
UCalendar* icuCalendar = icuLib.ucalOpen(
598-
getDesc(timeStampTz.time_zone)->getUnicodeName(), -1, NULL, UCAL_GREGORIAN, &icuErrorCode);
619+
auto icuCalendar = getDesc(timeStampTz.time_zone)->getCalendar(icuLib, &icuErrorCode);
599620

600621
if (!icuCalendar)
601622
status_exception::raise(Arg::Gds(isc_random) << "Error calling ICU's ucal_open.");
602623

603624
icuLib.ucalSetMillis(icuCalendar, timeStampToIcuDate(timeStampTz.utc_timestamp), &icuErrorCode);
604625

605626
if (U_FAILURE(icuErrorCode))
606-
{
607-
icuLib.ucalClose(icuCalendar);
608627
status_exception::raise(Arg::Gds(isc_random) << "Error calling ICU's ucal_setMillis.");
609-
}
610628

611629
displacement = (icuLib.ucalGet(icuCalendar, UCAL_ZONE_OFFSET, &icuErrorCode) +
612630
icuLib.ucalGet(icuCalendar, UCAL_DST_OFFSET, &icuErrorCode)) / U_MILLIS_PER_MINUTE;
613631

614632
if (U_FAILURE(icuErrorCode))
615-
{
616-
icuLib.ucalClose(icuCalendar);
617633
status_exception::raise(Arg::Gds(isc_random) << "Error calling ICU's ucal_get.");
618-
}
619-
620-
icuLib.ucalClose(icuCalendar);
621634
}
622635

623636
*offset = displacement;
@@ -689,8 +702,7 @@ void TimeZoneUtil::localTimeStampToUtc(ISC_TIMESTAMP_TZ& timeStampTz)
689702

690703
Jrd::UnicodeUtil::ConversionICU& icuLib = Jrd::UnicodeUtil::getConversionICU();
691704

692-
UCalendar* icuCalendar = icuLib.ucalOpen(
693-
getDesc(timeStampTz.time_zone)->getUnicodeName(), -1, NULL, UCAL_GREGORIAN, &icuErrorCode);
705+
auto icuCalendar = getDesc(timeStampTz.time_zone)->getCalendar(icuLib, &icuErrorCode);
694706

695707
if (!icuCalendar)
696708
status_exception::raise(Arg::Gds(isc_random) << "Error calling ICU's ucal_open.");
@@ -702,21 +714,13 @@ void TimeZoneUtil::localTimeStampToUtc(ISC_TIMESTAMP_TZ& timeStampTz)
702714
times.tm_hour, times.tm_min, times.tm_sec, &icuErrorCode);
703715

704716
if (U_FAILURE(icuErrorCode))
705-
{
706-
icuLib.ucalClose(icuCalendar);
707717
status_exception::raise(Arg::Gds(isc_random) << "Error calling ICU's ucal_setDateTime.");
708-
}
709718

710719
displacement = (icuLib.ucalGet(icuCalendar, UCAL_ZONE_OFFSET, &icuErrorCode) +
711720
icuLib.ucalGet(icuCalendar, UCAL_DST_OFFSET, &icuErrorCode)) / U_MILLIS_PER_MINUTE;
712721

713722
if (U_FAILURE(icuErrorCode))
714-
{
715-
icuLib.ucalClose(icuCalendar);
716723
status_exception::raise(Arg::Gds(isc_random) << "Error calling ICU's ucal_get.");
717-
}
718-
719-
icuLib.ucalClose(icuCalendar);
720724
}
721725

722726
const auto ticks = TimeStamp::timeStampToTicks(timeStampTz.utc_timestamp) -
@@ -758,30 +762,21 @@ bool TimeZoneUtil::decodeTimeStamp(const ISC_TIMESTAMP_TZ& timeStampTz, bool gmt
758762
#endif
759763
Jrd::UnicodeUtil::ConversionICU& icuLib = Jrd::UnicodeUtil::getConversionICU();
760764

761-
UCalendar* icuCalendar = icuLib.ucalOpen(
762-
getDesc(timeStampTz.time_zone)->getUnicodeName(), -1, NULL, UCAL_GREGORIAN, &icuErrorCode);
765+
auto icuCalendar = getDesc(timeStampTz.time_zone)->getCalendar(icuLib, &icuErrorCode);
763766

764767
if (!icuCalendar)
765768
status_exception::raise(Arg::Gds(isc_random) << "Error calling ICU's ucal_open.");
766769

767770
icuLib.ucalSetMillis(icuCalendar, timeStampToIcuDate(timeStampTz.utc_timestamp), &icuErrorCode);
768771

769772
if (U_FAILURE(icuErrorCode))
770-
{
771-
icuLib.ucalClose(icuCalendar);
772773
status_exception::raise(Arg::Gds(isc_random) << "Error calling ICU's ucal_setMillis.");
773-
}
774774

775775
displacement = (icuLib.ucalGet(icuCalendar, UCAL_ZONE_OFFSET, &icuErrorCode) +
776776
icuLib.ucalGet(icuCalendar, UCAL_DST_OFFSET, &icuErrorCode)) / U_MILLIS_PER_MINUTE;
777777

778778
if (U_FAILURE(icuErrorCode))
779-
{
780-
icuLib.ucalClose(icuCalendar);
781779
status_exception::raise(Arg::Gds(isc_random) << "Error calling ICU's ucal_get.");
782-
}
783-
784-
icuLib.ucalClose(icuCalendar);
785780
}
786781
catch (const Exception&)
787782
{
@@ -1050,12 +1045,11 @@ ISC_TIMESTAMP_TZ TimeZoneUtil::dateToTimeStampTz(const ISC_DATE& date, Callbacks
10501045
TimeZoneRuleIterator::TimeZoneRuleIterator(USHORT aId, const ISC_TIMESTAMP_TZ& aFrom, const ISC_TIMESTAMP_TZ& aTo)
10511046
: id(aId),
10521047
icuLib(Jrd::UnicodeUtil::getConversionICU()),
1053-
toTicks(TimeStamp::timeStampToTicks(aTo.utc_timestamp))
1048+
toTicks(TimeStamp::timeStampToTicks(aTo.utc_timestamp)),
1049+
icuCalendar(getDesc(aId)->getCalendar(icuLib))
10541050
{
10551051
UErrorCode icuErrorCode = U_ZERO_ERROR;
10561052

1057-
icuCalendar = icuLib.ucalOpen(getDesc(id)->getUnicodeName(), -1, NULL, UCAL_GREGORIAN, &icuErrorCode);
1058-
10591053
if (!icuCalendar)
10601054
status_exception::raise(Arg::Gds(isc_random) << "Error calling ICU's ucal_open.");
10611055

@@ -1092,11 +1086,6 @@ TimeZoneRuleIterator::TimeZoneRuleIterator(USHORT aId, const ISC_TIMESTAMP_TZ& a
10921086
startTicks = TimeStamp::timeStampToTicks(TimeZoneUtil::icuDateToTimeStamp(icuDate));
10931087
}
10941088

1095-
TimeZoneRuleIterator::~TimeZoneRuleIterator()
1096-
{
1097-
icuLib.ucalClose(icuCalendar);
1098-
}
1099-
11001089
bool TimeZoneRuleIterator::next()
11011090
{
11021091
if (startTicks > toTicks)

‎src/common/TimeZoneUtil.h‎

Lines changed: 48 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
#ifndef COMMON_TIME_ZONE_UTIL_H
2828
#define COMMON_TIME_ZONE_UTIL_H
2929

30+
#include <atomic>
3031
#include <functional>
3132
#include "../common/classes/fb_string.h"
3233
#include "../common/classes/timestamp.h"
@@ -140,11 +141,56 @@ class TimeZoneUtil
140141
static ISC_TIMESTAMP_TZ dateToTimeStampTz(const ISC_DATE& date, Callbacks* cb);
141142
};
142143

144+
class IcuCalendarWrapper
145+
{
146+
public:
147+
IcuCalendarWrapper(UCalendar* aWrapped, std::atomic<UCalendar*>* aCachePtr)
148+
: wrapped(aWrapped),
149+
cachePtr(aCachePtr)
150+
{}
151+
152+
~IcuCalendarWrapper()
153+
{
154+
if (wrapped)
155+
{
156+
auto newCached = cachePtr->exchange(wrapped);
157+
158+
if (newCached)
159+
{
160+
auto& icuLib = Jrd::UnicodeUtil::getConversionICU();
161+
icuLib.ucalClose(newCached);
162+
}
163+
}
164+
}
165+
166+
IcuCalendarWrapper(const IcuCalendarWrapper&) = delete;
167+
IcuCalendarWrapper& operator=(const IcuCalendarWrapper&) = delete;
168+
169+
public:
170+
UCalendar* operator->()
171+
{
172+
return wrapped;
173+
}
174+
175+
operator UCalendar*()
176+
{
177+
return wrapped;
178+
}
179+
180+
bool operator!() const
181+
{
182+
return !wrapped;
183+
}
184+
185+
private:
186+
UCalendar* wrapped;
187+
std::atomic<UCalendar*>* cachePtr;
188+
};
189+
143190
class TimeZoneRuleIterator
144191
{
145192
public:
146193
TimeZoneRuleIterator(USHORT aId, const ISC_TIMESTAMP_TZ& aFrom, const ISC_TIMESTAMP_TZ& aTo);
147-
~TimeZoneRuleIterator();
148194

149195
public:
150196
bool next();
@@ -160,7 +206,7 @@ class TimeZoneRuleIterator
160206
Jrd::UnicodeUtil::ConversionICU& icuLib;
161207
SINT64 startTicks;
162208
SINT64 toTicks;
163-
UCalendar* icuCalendar;
209+
IcuCalendarWrapper icuCalendar;
164210
UDate icuDate;
165211
};
166212

0 commit comments

Comments
 (0)