From 40520b8606111adeab272cb9d2fa4c7acca3e406 Mon Sep 17 00:00:00 2001 From: sebres Date: Tue, 10 Jan 2017 22:30:24 +0000 Subject: improve LocalizeFormat, internal caching of localized formats inside msgcat for locale and format objects smart reference introduced in dict (smart pointer with 0 object reference but increase dict-reference, provide changeable locale dict) code review --- generic/tclClock.c | 98 +------------------- generic/tclClockFmt.c | 229 ++++++++++++++++++++++++++++++---------------- generic/tclDate.h | 52 ++++++++++- library/clock.tcl | 19 ++-- tests-perf/clock.perf.tcl | 6 +- 5 files changed, 214 insertions(+), 190 deletions(-) diff --git a/generic/tclClock.c b/generic/tclClock.c index e066812..08bc6ef 100644 --- a/generic/tclClock.c +++ b/generic/tclClock.c @@ -42,53 +42,7 @@ static const int daysInPriorMonths[2][13] = { * Enumeration of the string literals used in [clock] */ -typedef enum ClockLiteral { - LIT__NIL, - LIT__DEFAULT_FORMAT, - LIT_BCE, LIT_C, - LIT_CANNOT_USE_GMT_AND_TIMEZONE, - LIT_CE, - LIT_DAYOFMONTH, LIT_DAYOFWEEK, LIT_DAYOFYEAR, - LIT_ERA, LIT_GMT, LIT_GREGORIAN, - LIT_INTEGER_VALUE_TOO_LARGE, - LIT_ISO8601WEEK, LIT_ISO8601YEAR, - LIT_JULIANDAY, LIT_LOCALSECONDS, - LIT_MONTH, - LIT_SECONDS, LIT_TZNAME, LIT_TZOFFSET, - LIT_YEAR, - LIT_TZDATA, - LIT_GETSYSTEMTIMEZONE, - LIT_SETUPTIMEZONE, - LIT_MCGET, LIT_TCL_CLOCK, - LIT_LOCALIZE_FORMAT, -#if 0 - LIT_FREESCAN, -#endif - LIT__END -} ClockLiteral; -static const char *const Literals[] = { - "", - "%a %b %d %H:%M:%S %Z %Y", - "BCE", "C", - "cannot use -gmt and -timezone in same call", - "CE", - "dayOfMonth", "dayOfWeek", "dayOfYear", - "era", ":GMT", "gregorian", - "integer value too large to represent", - "iso8601Week", "iso8601Year", - "julianDay", "localSeconds", - "month", - "seconds", "tzName", "tzOffset", - "year", - "::tcl::clock::TZData", - "::tcl::clock::GetSystemTimeZone", - "::tcl::clock::SetupTimeZone", - "::msgcat::mcget", "::tcl::clock", - "::tcl::clock::LocalizeFormat" -#if 0 - "::tcl::clock::FreeScan" -#endif -}; +CLOCK_LITERAL_ARRAY(Literals); /* Msgcat literals for exact match (mcKey) */ CLOCK_LOCALE_LITERAL_ARRAY(MsgCtLiterals, ""); @@ -634,56 +588,6 @@ ClockMCGetListIdxDict( return valObj; } -MODULE_SCOPE Tcl_Obj * -ClockLocalizeFormat( - ClockFmtScnCmdArgs *opts) -{ - ClockClientData *dataPtr = opts->clientData; - Tcl_Obj *valObj, *keyObj; - - keyObj = ClockFrmObjGetLocFmtKey(opts->interp, opts->formatObj); - - if (opts->mcDictObj == NULL) { - ClockMCDict(opts); - if (opts->mcDictObj == NULL) - return NULL; - } - - /* try to find in cache within mc-catalog */ - if (Tcl_DictObjGet(NULL, opts->mcDictObj, - keyObj, &valObj) != TCL_OK) { - return NULL; - } - if (valObj == NULL) { - Tcl_Obj *callargs[4]; - /* call LocalizeFormat locale format fmtkey */ - callargs[0] = dataPtr->literals[LIT_LOCALIZE_FORMAT]; - callargs[1] = opts->localeObj; - callargs[2] = opts->formatObj; - callargs[3] = keyObj; - Tcl_IncrRefCount(keyObj); - if (Tcl_EvalObjv(opts->interp, 4, callargs, 0) != TCL_OK - ) { - Tcl_DecrRefCount(keyObj); - return NULL; - } - - valObj = Tcl_GetObjResult(opts->interp); - - if (Tcl_DictObjPut(opts->interp, opts->mcDictObj, - keyObj, valObj) != TCL_OK - ) { - Tcl_DecrRefCount(keyObj); - return NULL; - } - - Tcl_DecrRefCount(keyObj); - Tcl_ResetResult(opts->interp); - } - - return (opts->formatObj = valObj); -} - /* *---------------------------------------------------------------------- */ diff --git a/generic/tclClockFmt.c b/generic/tclClockFmt.c index 184b52a..daedb26 100644 --- a/generic/tclClockFmt.c +++ b/generic/tclClockFmt.c @@ -233,65 +233,6 @@ static Tcl_HashKeyType ClockFmtScnStorageHashKeyType; /* - *---------------------------------------------------------------------- - */ - -static ClockFmtScnStorage * -FindOrCreateFmtScnStorage( - Tcl_Interp *interp, - const char *strFmt) -{ - ClockFmtScnStorage *fss = NULL; - int new; - Tcl_HashEntry *hPtr; - - Tcl_MutexLock(&ClockFmtMutex); - - /* if not yet initialized */ - if (!initialized) { - /* initialize type */ - memcpy(&ClockFmtScnStorageHashKeyType, &tclStringHashKeyType, sizeof(tclStringHashKeyType)); - ClockFmtScnStorageHashKeyType.allocEntryProc = ClockFmtScnStorageAllocProc; - ClockFmtScnStorageHashKeyType.freeEntryProc = ClockFmtScnStorageFreeProc; - - /* initialize hash table */ - Tcl_InitCustomHashTable(&FmtScnHashTable, TCL_CUSTOM_TYPE_KEYS, - &ClockFmtScnStorageHashKeyType); - - initialized = 1; - Tcl_CreateExitHandler(ClockFrmScnFinalize, NULL); - } - - /* get or create entry (and alocate storage) */ - hPtr = Tcl_CreateHashEntry(&FmtScnHashTable, strFmt, &new); - if (hPtr != NULL) { - - fss = FmtScn4HashEntry(hPtr); - - #if CLOCK_FMT_SCN_STORAGE_GC_SIZE > 0 - /* unlink if it is currently in GC */ - if (new == 0 && fss->objRefCount == 0) { - ClockFmtScnStorage_GC_Out(fss); - } - #endif - - /* new reference, so increment in lock right now */ - fss->objRefCount++; - } - - Tcl_MutexUnlock(&ClockFmtMutex); - - if (fss == NULL && interp != NULL) { - Tcl_AppendResult(interp, "retrieve clock format failed \"", - strFmt ? strFmt : "", "\"", NULL); - Tcl_SetErrorCode(interp, "TCL", "EINVAL", NULL); - } - - return fss; -} - - -/* * Type definition. */ @@ -303,20 +244,11 @@ Tcl_ObjType ClockFmtObjType = { ClockFmtObj_SetFromAny /* setFromAnyProc */ }; -#define SetObjClockFmtScn(objPtr, fss) \ - objPtr->internalRep.twoPtrValue.ptr1 = fss #define ObjClockFmtScn(objPtr) \ - (ClockFmtScnStorage *)objPtr->internalRep.twoPtrValue.ptr1; + (*((ClockFmtScnStorage **)&(objPtr)->internalRep.twoPtrValue.ptr1)) -#define SetObjLocFmtKey(objPtr, key) \ - Tcl_InitObjRef((Tcl_Obj *)objPtr->internalRep.twoPtrValue.ptr2, key) #define ObjLocFmtKey(objPtr) \ - ((Tcl_Obj *)objPtr->internalRep.twoPtrValue.ptr2) - -#define ClockFmtObj_SetObjIntRep(objPtr, fss, key) \ - objPtr->internalRep.twoPtrValue.ptr1 = fss; \ - Tcl_InitObjRef((Tcl_Obj *)objPtr->internalRep.twoPtrValue.ptr2, key); \ - objPtr->typePtr = &ClockFmtObjType; + (*((Tcl_Obj **)&(objPtr)->internalRep.twoPtrValue.ptr2)) /* *---------------------------------------------------------------------- @@ -334,7 +266,15 @@ ClockFmtObj_DupInternalRep(srcPtr, copyPtr) Tcl_MutexUnlock(&ClockFmtMutex); } - ClockFmtObj_SetObjIntRep(copyPtr, fss, ObjLocFmtKey(srcPtr)); + ObjClockFmtScn(copyPtr) = fss; + /* regards special case - format not localizable */ + if (ObjLocFmtKey(srcPtr) != srcPtr) { + Tcl_InitObjRef(ObjLocFmtKey(copyPtr), ObjLocFmtKey(srcPtr)); + } else { + ObjLocFmtKey(copyPtr) = copyPtr; + } + copyPtr->typePtr = &ClockFmtObjType; + /* if no format representation, dup string representation */ if (fss == NULL) { @@ -365,8 +305,12 @@ ClockFmtObj_FreeInternalRep(objPtr) } Tcl_MutexUnlock(&ClockFmtMutex); } - SetObjClockFmtScn(objPtr, NULL); - Tcl_UnsetObjRef(ObjLocFmtKey(objPtr)); + ObjClockFmtScn(objPtr) = NULL; + if (ObjLocFmtKey(objPtr) != objPtr) { + Tcl_UnsetObjRef(ObjLocFmtKey(objPtr)); + } else { + ObjLocFmtKey(objPtr) = NULL; + } objPtr->typePtr = NULL; }; /* @@ -385,8 +329,8 @@ ClockFmtObj_SetFromAny(interp, objPtr) objPtr->typePtr->freeIntRepProc(objPtr); /* initial state of format object */ - objPtr->internalRep.twoPtrValue.ptr1 = NULL; - objPtr->internalRep.twoPtrValue.ptr2 = NULL; + ObjClockFmtScn(objPtr) = NULL; + ObjLocFmtKey(objPtr) = NULL; objPtr->typePtr = &ClockFmtObjType; return TCL_OK; @@ -435,13 +379,74 @@ ClockFrmObjGetLocFmtKey( } keyObj = Tcl_ObjPrintf("FMT_%s", TclGetString(objPtr)); - SetObjLocFmtKey(objPtr, keyObj); + Tcl_InitObjRef(ObjLocFmtKey(objPtr), keyObj); return keyObj; } /* *---------------------------------------------------------------------- + */ + +static ClockFmtScnStorage * +FindOrCreateFmtScnStorage( + Tcl_Interp *interp, + Tcl_Obj *objPtr) +{ + const char *strFmt = TclGetString(objPtr); + ClockFmtScnStorage *fss = NULL; + int new; + Tcl_HashEntry *hPtr; + + Tcl_MutexLock(&ClockFmtMutex); + + /* if not yet initialized */ + if (!initialized) { + /* initialize type */ + memcpy(&ClockFmtScnStorageHashKeyType, &tclStringHashKeyType, sizeof(tclStringHashKeyType)); + ClockFmtScnStorageHashKeyType.allocEntryProc = ClockFmtScnStorageAllocProc; + ClockFmtScnStorageHashKeyType.freeEntryProc = ClockFmtScnStorageFreeProc; + + /* initialize hash table */ + Tcl_InitCustomHashTable(&FmtScnHashTable, TCL_CUSTOM_TYPE_KEYS, + &ClockFmtScnStorageHashKeyType); + + initialized = 1; + Tcl_CreateExitHandler(ClockFrmScnFinalize, NULL); + } + + /* get or create entry (and alocate storage) */ + hPtr = Tcl_CreateHashEntry(&FmtScnHashTable, strFmt, &new); + if (hPtr != NULL) { + + fss = FmtScn4HashEntry(hPtr); + + #if CLOCK_FMT_SCN_STORAGE_GC_SIZE > 0 + /* unlink if it is currently in GC */ + if (new == 0 && fss->objRefCount == 0) { + ClockFmtScnStorage_GC_Out(fss); + } + #endif + + /* new reference, so increment in lock right now */ + fss->objRefCount++; + + ObjClockFmtScn(objPtr) = fss; + } + + Tcl_MutexUnlock(&ClockFmtMutex); + + if (fss == NULL && interp != NULL) { + Tcl_AppendResult(interp, "retrieve clock format failed \"", + strFmt ? strFmt : "", "\"", NULL); + Tcl_SetErrorCode(interp, "TCL", "EINVAL", NULL); + } + + return fss; +} + +/* + *---------------------------------------------------------------------- * * Tcl_GetClockFrmScnFromObj -- * @@ -475,8 +480,7 @@ Tcl_GetClockFrmScnFromObj( fss = ObjClockFmtScn(objPtr); if (fss == NULL) { - const char *strFmt = TclGetString(objPtr); - fss = FindOrCreateFmtScnStorage(interp, strFmt); + fss = FindOrCreateFmtScnStorage(interp, objPtr); } return fss; @@ -772,7 +776,7 @@ ClockGetOrParseScanFormat( fss = ObjClockFmtScn(formatObj); if (fss == NULL) { - fss = FindOrCreateFmtScnStorage(interp, TclGetString(formatObj)); + fss = FindOrCreateFmtScnStorage(interp, formatObj); if (fss == NULL) { return NULL; } @@ -898,6 +902,72 @@ done: return fss->scnTok; } +MODULE_SCOPE Tcl_Obj * +ClockLocalizeFormat( + ClockFmtScnCmdArgs *opts) +{ + ClockClientData *dataPtr = opts->clientData; + Tcl_Obj *valObj = NULL, *keyObj; + + keyObj = ClockFrmObjGetLocFmtKey(opts->interp, opts->formatObj); + + /* special case - format object is not localizable */ + if (keyObj == opts->formatObj) { + return opts->formatObj; + } + + if (opts->mcDictObj == NULL) { + ClockMCDict(opts); + if (opts->mcDictObj == NULL) + return NULL; + } + + /* try to find in cache within locale mc-catalog */ + if (Tcl_DictObjGet(NULL, opts->mcDictObj, + keyObj, &valObj) != TCL_OK) { + return NULL; + } + + /* call LocalizeFormat locale format fmtkey */ + if (valObj == NULL) { + Tcl_Obj *callargs[4]; + callargs[0] = dataPtr->literals[LIT_LOCALIZE_FORMAT]; + callargs[1] = opts->localeObj; + callargs[2] = opts->formatObj; + callargs[3] = keyObj; + Tcl_IncrRefCount(keyObj); + if (Tcl_EvalObjv(opts->interp, 4, callargs, 0) != TCL_OK + ) { + goto clean; + } + + valObj = Tcl_GetObjResult(opts->interp); + + /* cache it inside mc-dictionary (this incr. ref count of keyObj/valObj) */ + if (Tcl_DictObjPut(opts->interp, opts->mcDictObj, + keyObj, valObj) != TCL_OK + ) { + valObj = NULL; + goto clean; + } + + /* check special case - format object is not localizable */ + if (valObj == opts->formatObj) { + /* mark it as unlocalizable, by setting self as key (without refcount incr) */ + if (opts->formatObj->typePtr == &ClockFmtObjType) { + Tcl_UnsetObjRef(ObjLocFmtKey(opts->formatObj)); + ObjLocFmtKey(opts->formatObj) = opts->formatObj; + } + } +clean: + + Tcl_UnsetObjRef(keyObj); + Tcl_ResetResult(opts->interp); + } + + return (opts->formatObj = valObj); +} + /* *---------------------------------------------------------------------- */ @@ -917,7 +987,6 @@ ClockScan( int ret = TCL_ERROR; /* get localized format */ - if (ClockLocalizeFormat(opts) == NULL) { return TCL_ERROR; } diff --git a/generic/tclDate.h b/generic/tclDate.h index 62fa693..9f65b1b 100644 --- a/generic/tclDate.h +++ b/generic/tclDate.h @@ -37,6 +37,53 @@ /* + * Enumeration of the string literals used in [clock] + */ + +typedef enum ClockLiteral { + LIT__NIL, + LIT__DEFAULT_FORMAT, + LIT_BCE, LIT_C, + LIT_CANNOT_USE_GMT_AND_TIMEZONE, + LIT_CE, + LIT_DAYOFMONTH, LIT_DAYOFWEEK, LIT_DAYOFYEAR, + LIT_ERA, LIT_GMT, LIT_GREGORIAN, + LIT_INTEGER_VALUE_TOO_LARGE, + LIT_ISO8601WEEK, LIT_ISO8601YEAR, + LIT_JULIANDAY, LIT_LOCALSECONDS, + LIT_MONTH, + LIT_SECONDS, LIT_TZNAME, LIT_TZOFFSET, + LIT_YEAR, + LIT_TZDATA, + LIT_GETSYSTEMTIMEZONE, + LIT_SETUPTIMEZONE, + LIT_MCGET, LIT_TCL_CLOCK, + LIT_LOCALIZE_FORMAT, + LIT__END +} ClockLiteral; + +#define CLOCK_LITERAL_ARRAY(litarr) static const char *const litarr[] = { \ + "", \ + "%a %b %d %H:%M:%S %Z %Y", \ + "BCE", "C", \ + "cannot use -gmt and -timezone in same call", \ + "CE", \ + "dayOfMonth", "dayOfWeek", "dayOfYear", \ + "era", ":GMT", "gregorian", \ + "integer value too large to represent", \ + "iso8601Week", "iso8601Year", \ + "julianDay", "localSeconds", \ + "month", \ + "seconds", "tzName", "tzOffset", \ + "year", \ + "::tcl::clock::TZData", \ + "::tcl::clock::GetSystemTimeZone", \ + "::tcl::clock::SetupTimeZone", \ + "::msgcat::mcget", "::tcl::clock", \ + "::tcl::clock::LocalizeFormat" \ +} + +/* * Enumeration of the msgcat literals used in [clock] */ @@ -362,8 +409,6 @@ MODULE_SCOPE Tcl_Obj * ClockMCGet(ClockFmtScnCmdArgs *opts, int mcKey); MODULE_SCOPE Tcl_Obj * ClockMCGetListIdxDict(ClockFmtScnCmdArgs *opts, int mcKey); -MODULE_SCOPE Tcl_Obj * - ClockLocalizeFormat(ClockFmtScnCmdArgs *opts); /* tclClockFmt.c module declarations */ @@ -374,7 +419,8 @@ MODULE_SCOPE Tcl_Obj* MODULE_SCOPE ClockFmtScnStorage * Tcl_GetClockFrmScnFromObj(Tcl_Interp *interp, Tcl_Obj *objPtr); - +MODULE_SCOPE Tcl_Obj * + ClockLocalizeFormat(ClockFmtScnCmdArgs *opts); MODULE_SCOPE int ClockScan(ClientData clientData, Tcl_Interp *interp, register DateInfo *info, Tcl_Obj *strObj, ClockFmtScnCmdArgs *opts); diff --git a/library/clock.tcl b/library/clock.tcl index 4173174..a532c0d 100755 --- a/library/clock.tcl +++ b/library/clock.tcl @@ -2365,18 +2365,23 @@ proc ::tcl::clock::LocalizeFormat { locale format {fmtkey {}} } { dict set LocaleFormats $locale MLST $mlst } - # translate: - set locfmt [string map $mlst $format] - - # Save original format as long as possible, because of internal representation (performance) - if {$locfmt eq $format} { - set locfmt $format - } + # translate copy of format (don't use format object here, because otherwise + # it can lose its internal representation (string map - convert to unicode) + set locfmt [string map $mlst [string range " $format" 1 end]] # cache it: dict set LocaleFormats $locale $fmtkey $locfmt } + # Save original format as long as possible, because of internal + # representation (performance). + # Note that in this case such format will be never localized (also + # using another locales). To prevent this return a duplicate (but + # it may be slower). + if {$locfmt eq $format} { + set locfmt $format + } + return $locfmt } diff --git a/tests-perf/clock.perf.tcl b/tests-perf/clock.perf.tcl index a005648..3c69414 100644 --- a/tests-perf/clock.perf.tcl +++ b/tests-perf/clock.perf.tcl @@ -132,9 +132,6 @@ proc test-scan {{reptime 1000}} { # Scan : date-time (system time zone without base) {clock scan "25.11.2015 10:35:55" -format "%d.%m.%Y %H:%M:%S"} - # Scan : dynamic format (cacheable) - {clock scan "25.11.2015 10:35:55" -format [string trim "%d.%m.%Y %H:%M:%S "] -base 0 -gmt 1} - # Scan : julian day in gmt {clock scan 2451545 -format %J -gmt 1} # Scan : julian day in system TZ @@ -153,6 +150,9 @@ proc test-scan {{reptime 1000}} { # Scan : century, lookup table month and day (list scan: entries with position 12 / 31) {clock scan {2016 Dec 31} -format {%C%y %b %Od} -locale en -gmt 1} + # Scan : dynamic format (cacheable) + {clock scan "25.11.2015 10:35:55" -format [string trim "%d.%m.%Y %H:%M:%S "] -base 0 -gmt 1} + break # Scan : zone only -- cgit v0.12