Skip to content

Commit e553406

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

File tree

2 files changed

+83
-41
lines changed

2 files changed

+83
-41
lines changed

‎src/common/TimeZoneUtil.cpp

+28-39
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

@@ -588,30 +610,21 @@ void TimeZoneUtil::extractOffset(const ISC_TIMESTAMP_TZ& timeStampTz, SSHORT* of
588610

589611
Jrd::UnicodeUtil::ConversionICU& icuLib = Jrd::UnicodeUtil::getConversionICU();
590612

591-
UCalendar* icuCalendar = icuLib.ucalOpen(
592-
getDesc(timeStampTz.time_zone)->getUnicodeName(), -1, NULL, UCAL_GREGORIAN, &icuErrorCode);
613+
auto icuCalendar = getDesc(timeStampTz.time_zone)->getCalendar(icuLib, &icuErrorCode);
593614

594615
if (!icuCalendar)
595616
status_exception::raise(Arg::Gds(isc_random) << "Error calling ICU's ucal_open.");
596617

597618
icuLib.ucalSetMillis(icuCalendar, timeStampToIcuDate(timeStampTz.utc_timestamp), &icuErrorCode);
598619

599620
if (U_FAILURE(icuErrorCode))
600-
{
601-
icuLib.ucalClose(icuCalendar);
602621
status_exception::raise(Arg::Gds(isc_random) << "Error calling ICU's ucal_setMillis.");
603-
}
604622

605623
displacement = (icuLib.ucalGet(icuCalendar, UCAL_ZONE_OFFSET, &icuErrorCode) +
606624
icuLib.ucalGet(icuCalendar, UCAL_DST_OFFSET, &icuErrorCode)) / U_MILLIS_PER_MINUTE;
607625

608626
if (U_FAILURE(icuErrorCode))
609-
{
610-
icuLib.ucalClose(icuCalendar);
611627
status_exception::raise(Arg::Gds(isc_random) << "Error calling ICU's ucal_get.");
612-
}
613-
614-
icuLib.ucalClose(icuCalendar);
615628
}
616629

617630
*offset = displacement;
@@ -683,8 +696,7 @@ void TimeZoneUtil::localTimeStampToUtc(ISC_TIMESTAMP_TZ& timeStampTz)
683696

684697
Jrd::UnicodeUtil::ConversionICU& icuLib = Jrd::UnicodeUtil::getConversionICU();
685698

686-
UCalendar* icuCalendar = icuLib.ucalOpen(
687-
getDesc(timeStampTz.time_zone)->getUnicodeName(), -1, NULL, UCAL_GREGORIAN, &icuErrorCode);
699+
auto icuCalendar = getDesc(timeStampTz.time_zone)->getCalendar(icuLib, &icuErrorCode);
688700

689701
if (!icuCalendar)
690702
status_exception::raise(Arg::Gds(isc_random) << "Error calling ICU's ucal_open.");
@@ -696,21 +708,13 @@ void TimeZoneUtil::localTimeStampToUtc(ISC_TIMESTAMP_TZ& timeStampTz)
696708
times.tm_hour, times.tm_min, times.tm_sec, &icuErrorCode);
697709

698710
if (U_FAILURE(icuErrorCode))
699-
{
700-
icuLib.ucalClose(icuCalendar);
701711
status_exception::raise(Arg::Gds(isc_random) << "Error calling ICU's ucal_setDateTime.");
702-
}
703712

704713
displacement = (icuLib.ucalGet(icuCalendar, UCAL_ZONE_OFFSET, &icuErrorCode) +
705714
icuLib.ucalGet(icuCalendar, UCAL_DST_OFFSET, &icuErrorCode)) / U_MILLIS_PER_MINUTE;
706715

707716
if (U_FAILURE(icuErrorCode))
708-
{
709-
icuLib.ucalClose(icuCalendar);
710717
status_exception::raise(Arg::Gds(isc_random) << "Error calling ICU's ucal_get.");
711-
}
712-
713-
icuLib.ucalClose(icuCalendar);
714718
}
715719

716720
const auto ticks = TimeStamp::timeStampToTicks(timeStampTz.utc_timestamp) -
@@ -752,30 +756,21 @@ bool TimeZoneUtil::decodeTimeStamp(const ISC_TIMESTAMP_TZ& timeStampTz, bool gmt
752756
#endif
753757
Jrd::UnicodeUtil::ConversionICU& icuLib = Jrd::UnicodeUtil::getConversionICU();
754758

755-
UCalendar* icuCalendar = icuLib.ucalOpen(
756-
getDesc(timeStampTz.time_zone)->getUnicodeName(), -1, NULL, UCAL_GREGORIAN, &icuErrorCode);
759+
auto icuCalendar = getDesc(timeStampTz.time_zone)->getCalendar(icuLib, &icuErrorCode);
757760

758761
if (!icuCalendar)
759762
status_exception::raise(Arg::Gds(isc_random) << "Error calling ICU's ucal_open.");
760763

761764
icuLib.ucalSetMillis(icuCalendar, timeStampToIcuDate(timeStampTz.utc_timestamp), &icuErrorCode);
762765

763766
if (U_FAILURE(icuErrorCode))
764-
{
765-
icuLib.ucalClose(icuCalendar);
766767
status_exception::raise(Arg::Gds(isc_random) << "Error calling ICU's ucal_setMillis.");
767-
}
768768

769769
displacement = (icuLib.ucalGet(icuCalendar, UCAL_ZONE_OFFSET, &icuErrorCode) +
770770
icuLib.ucalGet(icuCalendar, UCAL_DST_OFFSET, &icuErrorCode)) / U_MILLIS_PER_MINUTE;
771771

772772
if (U_FAILURE(icuErrorCode))
773-
{
774-
icuLib.ucalClose(icuCalendar);
775773
status_exception::raise(Arg::Gds(isc_random) << "Error calling ICU's ucal_get.");
776-
}
777-
778-
icuLib.ucalClose(icuCalendar);
779774
}
780775
catch (const Exception&)
781776
{
@@ -1044,12 +1039,11 @@ ISC_TIMESTAMP_TZ TimeZoneUtil::dateToTimeStampTz(const ISC_DATE& date, Callbacks
10441039
TimeZoneRuleIterator::TimeZoneRuleIterator(USHORT aId, const ISC_TIMESTAMP_TZ& aFrom, const ISC_TIMESTAMP_TZ& aTo)
10451040
: id(aId),
10461041
icuLib(Jrd::UnicodeUtil::getConversionICU()),
1047-
toTicks(TimeStamp::timeStampToTicks(aTo.utc_timestamp))
1042+
toTicks(TimeStamp::timeStampToTicks(aTo.utc_timestamp)),
1043+
icuCalendar(getDesc(aId)->getCalendar(icuLib))
10481044
{
10491045
UErrorCode icuErrorCode = U_ZERO_ERROR;
10501046

1051-
icuCalendar = icuLib.ucalOpen(getDesc(id)->getUnicodeName(), -1, NULL, UCAL_GREGORIAN, &icuErrorCode);
1052-
10531047
if (!icuCalendar)
10541048
status_exception::raise(Arg::Gds(isc_random) << "Error calling ICU's ucal_open.");
10551049

@@ -1086,11 +1080,6 @@ TimeZoneRuleIterator::TimeZoneRuleIterator(USHORT aId, const ISC_TIMESTAMP_TZ& a
10861080
startTicks = TimeStamp::timeStampToTicks(TimeZoneUtil::icuDateToTimeStamp(icuDate));
10871081
}
10881082

1089-
TimeZoneRuleIterator::~TimeZoneRuleIterator()
1090-
{
1091-
icuLib.ucalClose(icuCalendar);
1092-
}
1093-
10941083
bool TimeZoneRuleIterator::next()
10951084
{
10961085
if (startTicks > toTicks)

‎src/common/TimeZoneUtil.h

+55-2
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"
@@ -133,11 +134,63 @@ class TimeZoneUtil
133134
static ISC_TIMESTAMP_TZ dateToTimeStampTz(const ISC_DATE& date, Callbacks* cb);
134135
};
135136

137+
class IcuCalendarWrapper
138+
{
139+
public:
140+
IcuCalendarWrapper(UCalendar* aWrapped, std::atomic<UCalendar*>* aCachePtr)
141+
: wrapped(aWrapped),
142+
cachePtr(aCachePtr)
143+
{}
144+
145+
IcuCalendarWrapper(IcuCalendarWrapper&& o)
146+
: wrapped(o.wrapped),
147+
cachePtr(o.cachePtr)
148+
{
149+
o.wrapped = nullptr;
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+
136190
class TimeZoneRuleIterator
137191
{
138192
public:
139193
TimeZoneRuleIterator(USHORT aId, const ISC_TIMESTAMP_TZ& aFrom, const ISC_TIMESTAMP_TZ& aTo);
140-
~TimeZoneRuleIterator();
141194

142195
public:
143196
bool next();
@@ -153,7 +206,7 @@ class TimeZoneRuleIterator
153206
Jrd::UnicodeUtil::ConversionICU& icuLib;
154207
SINT64 startTicks;
155208
SINT64 toTicks;
156-
UCalendar* icuCalendar;
209+
IcuCalendarWrapper icuCalendar;
157210
UDate icuDate;
158211
};
159212

0 commit comments

Comments
 (0)