Skip to content

Commit c390a29

Browse files
asfernandesasfernandes
committed
Fix #7854 - Performance issue with time zones.
1 parent a0ff6b9 commit c390a29

2 files changed

Lines changed: 76 additions & 41 deletions

File tree

‎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

@@ -593,30 +615,21 @@ void TimeZoneUtil::extractOffset(const ISC_TIMESTAMP_TZ& timeStampTz, SSHORT* of
593615

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

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

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

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

604625
if (U_FAILURE(icuErrorCode))
605-
{
606-
icuLib.ucalClose(icuCalendar);
607626
status_exception::raise(Arg::Gds(isc_random) << "Error calling ICU's ucal_setMillis.");
608-
}
609627

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

613631
if (U_FAILURE(icuErrorCode))
614-
{
615-
icuLib.ucalClose(icuCalendar);
616632
status_exception::raise(Arg::Gds(isc_random) << "Error calling ICU's ucal_get.");
617-
}
618-
619-
icuLib.ucalClose(icuCalendar);
620633
}
621634

622635
*offset = displacement;
@@ -702,8 +715,7 @@ void TimeZoneUtil::localTimeStampToUtc(ISC_TIMESTAMP_TZ& timeStampTz)
702715

703716
Jrd::UnicodeUtil::ConversionICU& icuLib = Jrd::UnicodeUtil::getConversionICU();
704717

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

708720
if (!icuCalendar)
709721
status_exception::raise(Arg::Gds(isc_random) << "Error calling ICU's ucal_open.");
@@ -715,21 +727,13 @@ void TimeZoneUtil::localTimeStampToUtc(ISC_TIMESTAMP_TZ& timeStampTz)
715727
times.tm_hour, times.tm_min, times.tm_sec, &icuErrorCode);
716728

717729
if (U_FAILURE(icuErrorCode))
718-
{
719-
icuLib.ucalClose(icuCalendar);
720730
status_exception::raise(Arg::Gds(isc_random) << "Error calling ICU's ucal_setDateTime.");
721-
}
722731

723732
displacement = (icuLib.ucalGet(icuCalendar, UCAL_ZONE_OFFSET, &icuErrorCode) +
724733
icuLib.ucalGet(icuCalendar, UCAL_DST_OFFSET, &icuErrorCode)) / U_MILLIS_PER_MINUTE;
725734

726735
if (U_FAILURE(icuErrorCode))
727-
{
728-
icuLib.ucalClose(icuCalendar);
729736
status_exception::raise(Arg::Gds(isc_random) << "Error calling ICU's ucal_get.");
730-
}
731-
732-
icuLib.ucalClose(icuCalendar);
733737
}
734738

735739
const auto ticks = TimeStamp::timeStampToTicks(timeStampTz.utc_timestamp) -
@@ -771,30 +775,21 @@ bool TimeZoneUtil::decodeTimeStamp(const ISC_TIMESTAMP_TZ& timeStampTz, bool gmt
771775
#endif
772776
Jrd::UnicodeUtil::ConversionICU& icuLib = Jrd::UnicodeUtil::getConversionICU();
773777

774-
UCalendar* icuCalendar = icuLib.ucalOpen(
775-
getDesc(timeStampTz.time_zone)->getUnicodeName(), -1, NULL, UCAL_GREGORIAN, &icuErrorCode);
778+
auto icuCalendar = getDesc(timeStampTz.time_zone)->getCalendar(icuLib, &icuErrorCode);
776779

777780
if (!icuCalendar)
778781
status_exception::raise(Arg::Gds(isc_random) << "Error calling ICU's ucal_open.");
779782

780783
icuLib.ucalSetMillis(icuCalendar, timeStampToIcuDate(timeStampTz.utc_timestamp), &icuErrorCode);
781784

782785
if (U_FAILURE(icuErrorCode))
783-
{
784-
icuLib.ucalClose(icuCalendar);
785786
status_exception::raise(Arg::Gds(isc_random) << "Error calling ICU's ucal_setMillis.");
786-
}
787787

788788
displacement = (icuLib.ucalGet(icuCalendar, UCAL_ZONE_OFFSET, &icuErrorCode) +
789789
icuLib.ucalGet(icuCalendar, UCAL_DST_OFFSET, &icuErrorCode)) / U_MILLIS_PER_MINUTE;
790790

791791
if (U_FAILURE(icuErrorCode))
792-
{
793-
icuLib.ucalClose(icuCalendar);
794792
status_exception::raise(Arg::Gds(isc_random) << "Error calling ICU's ucal_get.");
795-
}
796-
797-
icuLib.ucalClose(icuCalendar);
798793
}
799794
catch (const Exception&)
800795
{
@@ -1063,12 +1058,11 @@ ISC_TIMESTAMP_TZ TimeZoneUtil::dateToTimeStampTz(const ISC_DATE& date, Callbacks
10631058
TimeZoneRuleIterator::TimeZoneRuleIterator(USHORT aId, const ISC_TIMESTAMP_TZ& aFrom, const ISC_TIMESTAMP_TZ& aTo)
10641059
: id(aId),
10651060
icuLib(Jrd::UnicodeUtil::getConversionICU()),
1066-
toTicks(TimeStamp::timeStampToTicks(aTo.utc_timestamp))
1061+
toTicks(TimeStamp::timeStampToTicks(aTo.utc_timestamp)),
1062+
icuCalendar(getDesc(aId)->getCalendar(icuLib))
10671063
{
10681064
UErrorCode icuErrorCode = U_ZERO_ERROR;
10691065

1070-
icuCalendar = icuLib.ucalOpen(getDesc(id)->getUnicodeName(), -1, NULL, UCAL_GREGORIAN, &icuErrorCode);
1071-
10721066
if (!icuCalendar)
10731067
status_exception::raise(Arg::Gds(isc_random) << "Error calling ICU's ucal_open.");
10741068

@@ -1105,11 +1099,6 @@ TimeZoneRuleIterator::TimeZoneRuleIterator(USHORT aId, const ISC_TIMESTAMP_TZ& a
11051099
startTicks = TimeStamp::timeStampToTicks(TimeZoneUtil::icuDateToTimeStamp(icuDate));
11061100
}
11071101

1108-
TimeZoneRuleIterator::~TimeZoneRuleIterator()
1109-
{
1110-
icuLib.ucalClose(icuCalendar);
1111-
}
1112-
11131102
bool TimeZoneRuleIterator::next()
11141103
{
11151104
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"
@@ -142,11 +143,56 @@ class TimeZoneUtil
142143
static ISC_TIMESTAMP_TZ dateToTimeStampTz(const ISC_DATE& date, Callbacks* cb);
143144
};
144145

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

151197
public:
152198
bool next();
@@ -162,7 +208,7 @@ class TimeZoneRuleIterator
162208
Jrd::UnicodeUtil::ConversionICU& icuLib;
163209
SINT64 startTicks;
164210
SINT64 toTicks;
165-
UCalendar* icuCalendar;
211+
IcuCalendarWrapper icuCalendar;
166212
UDate icuDate;
167213
};
168214

0 commit comments

Comments
 (0)