From 397413cd716cc73ca6291b8ba67c1c1950624fd2 Mon Sep 17 00:00:00 2001 From: dgp Date: Tue, 29 Jul 2014 00:22:32 +0000 Subject: [3757cdf808] Refactoring seems to reduce (maybe somewhat kinda sorta) a bizarre performance regression. Still not what it was. Damn Heisenbugs. --- generic/tclClock.c | 54 ++++++++++++++++++++++++++++++++---------------------- 1 file changed, 32 insertions(+), 22 deletions(-) diff --git a/generic/tclClock.c b/generic/tclClock.c index 7e917f6..524a9e8 100644 --- a/generic/tclClock.c +++ b/generic/tclClock.c @@ -522,6 +522,26 @@ ClockGetdatefieldsObjCmd( */ static int +FetchIntField( + Tcl_Interp *interp, + Tcl_Obj *dict, + Tcl_Obj *key, + int *storePtr) +{ + Tcl_Obj *value = NULL; + + if (Tcl_DictObjGet(interp, dict, key, &value) != TCL_OK) { + return TCL_ERROR; + } + if (value == NULL) { + Tcl_SetObjResult(interp, Tcl_NewStringObj( + "expected key(s) not found in dictionary", -1)); + return TCL_ERROR; + } + return TclGetIntFromObj(interp, value, storePtr); +} + +static int ClockGetjuliandayfromerayearmonthdayObjCmd( ClientData clientData, /* Opaque pointer to literal pool, etc. */ Tcl_Interp *interp, /* Tcl interpreter */ @@ -551,18 +571,13 @@ ClockGetjuliandayfromerayearmonthdayObjCmd( || fieldPtr == NULL || Tcl_GetIndexFromObj(interp, fieldPtr, eras, "era", TCL_EXACT, &era) != TCL_OK - || Tcl_DictObjGet(interp, dict, literals[LIT_YEAR], &fieldPtr) != TCL_OK - || fieldPtr == NULL - || TclGetIntFromObj(interp, fieldPtr, &fields.year) != TCL_OK - || Tcl_DictObjGet(interp, dict, literals[LIT_MONTH], &fieldPtr) != TCL_OK - || fieldPtr == NULL - || TclGetIntFromObj(interp, fieldPtr, &fields.month) != TCL_OK - || Tcl_DictObjGet(interp, dict, literals[LIT_DAYOFMONTH], &fieldPtr) != TCL_OK - || fieldPtr == NULL - || TclGetIntFromObj(interp, fieldPtr, &fields.dayOfMonth)!=TCL_OK + || FetchIntField(interp, dict, literals[LIT_YEAR], &fields.year) + != TCL_OK + || FetchIntField(interp, dict, literals[LIT_MONTH], &fields.month) + != TCL_OK + || FetchIntField(interp, dict, literals[LIT_DAYOFMONTH], + &fields.dayOfMonth) != TCL_OK || TclGetIntFromObj(interp, objv[2], &changeover) != TCL_OK) { - if (fieldPtr == NULL) - Tcl_SetObjResult(interp, Tcl_NewStringObj("expected key(s) not found in dictionary", -1)); return TCL_ERROR; } fields.era = era; @@ -644,18 +659,13 @@ ClockGetjuliandayfromerayearweekdayObjCmd( || fieldPtr == NULL || Tcl_GetIndexFromObj(interp, fieldPtr, eras, "era", TCL_EXACT, &era) != TCL_OK - || Tcl_DictObjGet(interp, dict, literals[LIT_ISO8601YEAR], &fieldPtr) != TCL_OK - || fieldPtr == NULL - || TclGetIntFromObj(interp, fieldPtr, &(fields.iso8601Year)) != TCL_OK - || Tcl_DictObjGet(interp, dict, literals[LIT_ISO8601WEEK], &fieldPtr) != TCL_OK - || fieldPtr == NULL - || TclGetIntFromObj(interp, fieldPtr, &(fields.iso8601Week)) != TCL_OK - || Tcl_DictObjGet(interp, dict, literals[LIT_DAYOFWEEK], &fieldPtr) != TCL_OK - || fieldPtr == NULL - || TclGetIntFromObj(interp, fieldPtr, &(fields.dayOfWeek)) != TCL_OK + || FetchIntField(interp, dict, literals[LIT_ISO8601YEAR], + &fields.iso8601Year) != TCL_OK + || FetchIntField(interp, dict, literals[LIT_ISO8601WEEK], + &fields.iso8601Week) != TCL_OK + || FetchIntField(interp, dict, literals[LIT_DAYOFWEEK], + &fields.dayOfWeek) != TCL_OK || TclGetIntFromObj(interp, objv[2], &changeover) != TCL_OK) { - if (fieldPtr == NULL) - Tcl_SetObjResult(interp, Tcl_NewStringObj("expected key(s) not found in dictionary", -1)); return TCL_ERROR; } fields.era = era; -- cgit v0.12 From 2e1ca1d9e14de68527cb660bb11fb459fdb8f01e Mon Sep 17 00:00:00 2001 From: dgp Date: Wed, 30 Jul 2014 00:22:33 +0000 Subject: Reduce the cost of Preserve/Release on channels --- generic/tclIO.c | 77 +++++++++++++++++++++++++++++++++++----------------- generic/tclIO.h | 2 ++ generic/tclIOCmd.c | 32 +++++++++++----------- generic/tclIORChan.c | 4 +-- generic/tclInt.h | 2 ++ 5 files changed, 74 insertions(+), 43 deletions(-) diff --git a/generic/tclIO.c b/generic/tclIO.c index d9d37a2..36ec903 100644 --- a/generic/tclIO.c +++ b/generic/tclIO.c @@ -1652,6 +1652,7 @@ Tcl_CreateChannel( chanPtr->upChanPtr = NULL; chanPtr->inQueueHead = NULL; chanPtr->inQueueTail = NULL; + chanPtr->refCount = 0; /* * TIP #219, Tcl Channel Reflection API @@ -1872,6 +1873,7 @@ Tcl_StackChannel( chanPtr->upChanPtr = NULL; chanPtr->inQueueHead = NULL; chanPtr->inQueueTail = NULL; + chanPtr->refCount = 0; /* * Place new block at the head of a possibly existing list of previously @@ -1896,6 +1898,31 @@ Tcl_StackChannel( return (Tcl_Channel) chanPtr; } + +void +TclChannelPreserve( + Tcl_Channel chan) +{ + Channel *chanPtr = (Channel *) chan; + + if (chanPtr->refCount == 0) { + Tcl_Preserve(chan); + } + chanPtr->refCount++; +} + +void +TclChannelRelease( + Tcl_Channel chan) +{ + Channel *chanPtr = (Channel *) chan; + + if (--chanPtr->refCount) { + return; + } + Tcl_Release(chan); +} + /* *---------------------------------------------------------------------- @@ -2642,7 +2669,7 @@ FlushChannel( * of the queued output to the channel. */ - Tcl_Preserve(chanPtr); + TclChannelPreserve((Tcl_Channel)chanPtr); while (statePtr->outQueueHead) { bufPtr = statePtr->outQueueHead; @@ -2833,7 +2860,7 @@ FlushChannel( } done: - Tcl_Release(chanPtr); + TclChannelRelease((Tcl_Channel)chanPtr); return errorCode; } @@ -4391,7 +4418,7 @@ Tcl_GetsObj( */ chanPtr = statePtr->topChanPtr; - Tcl_Preserve(chanPtr); + TclChannelPreserve((Tcl_Channel)chanPtr); bufPtr = statePtr->inQueueHead; encoding = statePtr->encoding; @@ -4627,9 +4654,9 @@ Tcl_GetsObj( */ if (chanPtr != statePtr->topChanPtr) { - Tcl_Release(chanPtr); + TclChannelRelease((Tcl_Channel)chanPtr); chanPtr = statePtr->topChanPtr; - Tcl_Preserve(chanPtr); + TclChannelPreserve((Tcl_Channel)chanPtr); } bufPtr = gs.bufPtr; @@ -4665,9 +4692,9 @@ Tcl_GetsObj( * self-modifying reflected transforms. */ if (chanPtr != statePtr->topChanPtr) { - Tcl_Release(chanPtr); + TclChannelRelease((Tcl_Channel)chanPtr); chanPtr = statePtr->topChanPtr; - Tcl_Preserve(chanPtr); + TclChannelPreserve((Tcl_Channel)chanPtr); } bufPtr = statePtr->inQueueHead; if (bufPtr != NULL) { @@ -4709,12 +4736,12 @@ Tcl_GetsObj( * self-modifying reflected transforms. */ if (chanPtr != statePtr->topChanPtr) { - Tcl_Release(chanPtr); + TclChannelRelease((Tcl_Channel)chanPtr); chanPtr = statePtr->topChanPtr; - Tcl_Preserve(chanPtr); + TclChannelPreserve((Tcl_Channel)chanPtr); } UpdateInterest(chanPtr); - Tcl_Release(chanPtr); + TclChannelRelease((Tcl_Channel)chanPtr); return copiedTotal; } @@ -4760,7 +4787,7 @@ TclGetsObjBinary( */ chanPtr = statePtr->topChanPtr; - Tcl_Preserve(chanPtr); + TclChannelPreserve((Tcl_Channel)chanPtr); bufPtr = statePtr->inQueueHead; @@ -4965,7 +4992,7 @@ TclGetsObjBinary( done: UpdateInterest(chanPtr); - Tcl_Release(chanPtr); + TclChannelRelease((Tcl_Channel)chanPtr); return copiedTotal; } @@ -5589,7 +5616,7 @@ DoReadChars( chanPtr = statePtr->topChanPtr; encoding = statePtr->encoding; factor = UTF_EXPANSION_FACTOR; - Tcl_Preserve(chanPtr); + TclChannelPreserve((Tcl_Channel)chanPtr); binaryMode = (encoding == NULL) && (statePtr->inputTranslation == TCL_TRANSLATE_LF) @@ -5650,9 +5677,9 @@ DoReadChars( } result = GetInput(chanPtr); if (chanPtr != statePtr->topChanPtr) { - Tcl_Release(chanPtr); + TclChannelRelease((Tcl_Channel)chanPtr); chanPtr = statePtr->topChanPtr; - Tcl_Preserve(chanPtr); + TclChannelPreserve((Tcl_Channel)chanPtr); } if (result != 0) { if (!GotFlag(statePtr, CHANNEL_BLOCKED)) { @@ -5680,9 +5707,9 @@ DoReadChars( * self-modifying reflected transforms. */ if (chanPtr != statePtr->topChanPtr) { - Tcl_Release(chanPtr); + TclChannelRelease((Tcl_Channel)chanPtr); chanPtr = statePtr->topChanPtr; - Tcl_Preserve(chanPtr); + TclChannelPreserve((Tcl_Channel)chanPtr); } /* @@ -5690,7 +5717,7 @@ DoReadChars( * in the buffers. */ UpdateInterest(chanPtr); - Tcl_Release(chanPtr); + TclChannelRelease((Tcl_Channel)chanPtr); return copied; } @@ -8062,7 +8089,7 @@ Tcl_NotifyChannel( * Preserve the channel struct in case the script closes it. */ - Tcl_Preserve(channel); + TclChannelPreserve((Tcl_Channel)channel); Tcl_Preserve(statePtr); /* @@ -8112,7 +8139,7 @@ Tcl_NotifyChannel( } Tcl_Release(statePtr); - Tcl_Release(channel); + TclChannelRelease(channel); tsdPtr->nestedHandlerPtr = nh.nestedHandlerPtr; } @@ -8594,7 +8621,7 @@ TclChannelEventScriptInvoker( */ Tcl_Preserve(interp); - Tcl_Preserve(chanPtr); + TclChannelPreserve((Tcl_Channel)chanPtr); result = Tcl_EvalObjEx(interp, esPtr->scriptPtr, TCL_EVAL_GLOBAL); /* @@ -8611,7 +8638,7 @@ TclChannelEventScriptInvoker( } Tcl_BackgroundException(interp, result); } - Tcl_Release(chanPtr); + TclChannelRelease((Tcl_Channel)chanPtr); Tcl_Release(interp); } @@ -9464,7 +9491,7 @@ DoRead( ChannelState *statePtr = chanPtr->state; char *p = dst; - Tcl_Preserve(chanPtr); + TclChannelPreserve((Tcl_Channel)chanPtr); while (bytesToRead) { /* * Each pass through the loop is intended to process up to @@ -9503,7 +9530,7 @@ DoRead( if (code) { /* Read error */ UpdateInterest(chanPtr); - Tcl_Release(chanPtr); + TclChannelRelease((Tcl_Channel)chanPtr); return -1; } @@ -9601,7 +9628,7 @@ DoRead( ResetFlag(statePtr, CHANNEL_BLOCKED); } - Tcl_Release(chanPtr); + TclChannelRelease((Tcl_Channel)chanPtr); return (int)(p - dst); } diff --git a/generic/tclIO.h b/generic/tclIO.h index 097cd61..ca74c3e 100644 --- a/generic/tclIO.h +++ b/generic/tclIO.h @@ -112,6 +112,8 @@ typedef struct Channel { ChannelBuffer *inQueueHead; /* Points at first buffer in input queue. */ ChannelBuffer *inQueueTail; /* Points at last buffer in input queue. */ + + int refCount; } Channel; /* diff --git a/generic/tclIOCmd.c b/generic/tclIOCmd.c index 3368a76..834f225 100644 --- a/generic/tclIOCmd.c +++ b/generic/tclIOCmd.c @@ -181,7 +181,7 @@ Tcl_PutsObjCmd( return TCL_ERROR; } - Tcl_Preserve(chan); + TclChannelPreserve(chan); result = Tcl_WriteObj(chan, string); if (result < 0) { goto error; @@ -192,7 +192,7 @@ Tcl_PutsObjCmd( goto error; } } - Tcl_Release(chan); + TclChannelRelease(chan); return TCL_OK; /* @@ -207,7 +207,7 @@ Tcl_PutsObjCmd( Tcl_SetObjResult(interp, Tcl_ObjPrintf("error writing \"%s\": %s", TclGetString(chanObjPtr), Tcl_PosixError(interp))); } - Tcl_Release(chan); + TclChannelRelease(chan); return TCL_ERROR; } @@ -255,7 +255,7 @@ Tcl_FlushObjCmd( return TCL_ERROR; } - Tcl_Preserve(chan); + TclChannelPreserve(chan); if (Tcl_Flush(chan) != TCL_OK) { /* * TIP #219. @@ -269,10 +269,10 @@ Tcl_FlushObjCmd( "error flushing \"%s\": %s", TclGetString(chanObjPtr), Tcl_PosixError(interp))); } - Tcl_Release(chan); + TclChannelRelease(chan); return TCL_ERROR; } - Tcl_Release(chan); + TclChannelRelease(chan); return TCL_OK; } @@ -322,7 +322,7 @@ Tcl_GetsObjCmd( return TCL_ERROR; } - Tcl_Preserve(chan); + TclChannelPreserve(chan); linePtr = Tcl_NewObj(); lineLen = Tcl_GetsObj(chan, linePtr); if (lineLen < 0) { @@ -357,7 +357,7 @@ Tcl_GetsObjCmd( Tcl_SetObjResult(interp, linePtr); } done: - Tcl_Release(chan); + TclChannelRelease(chan); return code; } @@ -465,7 +465,7 @@ Tcl_ReadObjCmd( resultPtr = Tcl_NewObj(); Tcl_IncrRefCount(resultPtr); - Tcl_Preserve(chan); + TclChannelPreserve(chan); charactersRead = Tcl_ReadChars(chan, resultPtr, toRead, 0); if (charactersRead < 0) { /* @@ -480,7 +480,7 @@ Tcl_ReadObjCmd( "error reading \"%s\": %s", TclGetString(chanObjPtr), Tcl_PosixError(interp))); } - Tcl_Release(chan); + TclChannelRelease(chan); Tcl_DecrRefCount(resultPtr); return TCL_ERROR; } @@ -499,7 +499,7 @@ Tcl_ReadObjCmd( } } Tcl_SetObjResult(interp, resultPtr); - Tcl_Release(chan); + TclChannelRelease(chan); Tcl_DecrRefCount(resultPtr); return TCL_OK; } @@ -559,7 +559,7 @@ Tcl_SeekObjCmd( mode = modeArray[optionIndex]; } - Tcl_Preserve(chan); + TclChannelPreserve(chan); result = Tcl_Seek(chan, offset, mode); if (result == Tcl_LongAsWide(-1)) { /* @@ -574,10 +574,10 @@ Tcl_SeekObjCmd( "error during seek on \"%s\": %s", TclGetString(objv[1]), Tcl_PosixError(interp))); } - Tcl_Release(chan); + TclChannelRelease(chan); return TCL_ERROR; } - Tcl_Release(chan); + TclChannelRelease(chan); return TCL_OK; } @@ -624,7 +624,7 @@ Tcl_TellObjCmd( return TCL_ERROR; } - Tcl_Preserve(chan); + TclChannelPreserve(chan); newLoc = Tcl_Tell(chan); /* @@ -635,7 +635,7 @@ Tcl_TellObjCmd( code = TclChanCaughtErrorBypass(interp, chan); - Tcl_Release(chan); + TclChannelRelease(chan); if (code) { return TCL_ERROR; } diff --git a/generic/tclIORChan.c b/generic/tclIORChan.c index 3506a44..21c766e 100644 --- a/generic/tclIORChan.c +++ b/generic/tclIORChan.c @@ -658,7 +658,7 @@ TclChanCreateObjCmd( chan = Tcl_CreateChannel(&tclRChannelType, TclGetString(rcId), rcPtr, mode); rcPtr->chan = chan; - Tcl_Preserve(chan); + TclChannelPreserve(chan); chanPtr = (Channel *) chan; if ((methods & NULLABLE_METHODS) != NULLABLE_METHODS) { @@ -2188,7 +2188,7 @@ FreeReflectedChannel( { Channel *chanPtr = (Channel *) rcPtr->chan; - Tcl_Release(chanPtr); + TclChannelRelease((Tcl_Channel)chanPtr); if (rcPtr->name) { Tcl_DecrRefCount(rcPtr->name); } diff --git a/generic/tclInt.h b/generic/tclInt.h index 9a2e8dd..1bb2103 100644 --- a/generic/tclInt.h +++ b/generic/tclInt.h @@ -2864,6 +2864,8 @@ MODULE_SCOPE int TclByteArrayMatch(const unsigned char *string, int strLen, const unsigned char *pattern, int ptnLen, int flags); MODULE_SCOPE double TclCeil(const mp_int *a); +MODULE_SCOPE void TclChannelPreserve(Tcl_Channel chan); +MODULE_SCOPE void TclChannelRelease(Tcl_Channel chan); MODULE_SCOPE int TclCheckBadOctal(Tcl_Interp *interp, const char *value); MODULE_SCOPE int TclChanCaughtErrorBypass(Tcl_Interp *interp, -- cgit v0.12 From 4a9e78c1c81ac5ec6d37a5ead62ff57fa90b0875 Mon Sep 17 00:00:00 2001 From: dgp Date: Wed, 30 Jul 2014 02:58:27 +0000 Subject: Complete transformation off Tcl_Preserve() legacy onto ref counting. --- generic/tclIO.c | 22 +++++++--------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/generic/tclIO.c b/generic/tclIO.c index 36ec903..7381f4d 100644 --- a/generic/tclIO.c +++ b/generic/tclIO.c @@ -1903,12 +1903,7 @@ void TclChannelPreserve( Tcl_Channel chan) { - Channel *chanPtr = (Channel *) chan; - - if (chanPtr->refCount == 0) { - Tcl_Preserve(chan); - } - chanPtr->refCount++; + ((Channel *)chan)->refCount++; } void @@ -1917,10 +1912,15 @@ TclChannelRelease( { Channel *chanPtr = (Channel *) chan; + if (chanPtr->refCount == 0) { + Tcl_Panic("Channel released more than preserved"); + } if (--chanPtr->refCount) { return; } - Tcl_Release(chan); + if (chanPtr->typePtr == NULL) { + ckfree(chanPtr); + } } @@ -2071,11 +2071,6 @@ Tcl_UnstackChannel( result = ChanClose(chanPtr, interp); chanPtr->typePtr = NULL; - /* - * AK: Tcl_NotifyChannel may hold a reference to this block of memory - */ - - Tcl_EventuallyFree(chanPtr, TCL_DYNAMIC); UpdateInterest(statePtr->topChanPtr); if (result != 0) { @@ -3028,7 +3023,6 @@ CloseChannel( downChanPtr->upChanPtr = NULL; chanPtr->typePtr = NULL; - Tcl_EventuallyFree(chanPtr, TCL_DYNAMIC); return Tcl_Close(interp, (Tcl_Channel) downChanPtr); } @@ -3036,13 +3030,11 @@ CloseChannel( * There is only the TOP Channel, so we free the remaining pointers we * have and then ourselves. Since this is the last of the channels in the * stack, make sure to free the ChannelState structure associated with it. - * We use Tcl_EventuallyFree to allow for any last references. */ chanPtr->typePtr = NULL; Tcl_EventuallyFree(statePtr, TCL_DYNAMIC); - Tcl_EventuallyFree(chanPtr, TCL_DYNAMIC); return errorCode; } -- cgit v0.12 From f10bee2648e7e87c576a1afe761e8525255d3a7a Mon Sep 17 00:00:00 2001 From: dgp Date: Wed, 30 Jul 2014 16:41:24 +0000 Subject: [3757cdf808] More clock refactoring with spooky impact on [string match] performance. --- generic/tclClock.c | 32 ++++++++++++++++++++++---------- 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/generic/tclClock.c b/generic/tclClock.c index 524a9e8..9d4bcd6 100644 --- a/generic/tclClock.c +++ b/generic/tclClock.c @@ -522,6 +522,26 @@ ClockGetdatefieldsObjCmd( */ static int +FetchEraField( + Tcl_Interp *interp, + Tcl_Obj *dict, + Tcl_Obj *key, + int *storePtr) +{ + Tcl_Obj *value = NULL; + + if (Tcl_DictObjGet(interp, dict, key, &value) != TCL_OK) { + return TCL_ERROR; + } + if (value == NULL) { + Tcl_SetObjResult(interp, Tcl_NewStringObj( + "expected key(s) not found in dictionary", -1)); + return TCL_ERROR; + } + return Tcl_GetIndexFromObj(interp, value, eras, "era", TCL_EXACT, storePtr); +} + +static int FetchIntField( Tcl_Interp *interp, Tcl_Obj *dict, @@ -552,7 +572,6 @@ ClockGetjuliandayfromerayearmonthdayObjCmd( Tcl_Obj *dict; ClockClientData *data = clientData; Tcl_Obj *const *literals = data->literals; - Tcl_Obj *fieldPtr; int changeover; int copied = 0; int status; @@ -567,10 +586,7 @@ ClockGetjuliandayfromerayearmonthdayObjCmd( return TCL_ERROR; } dict = objv[1]; - if (Tcl_DictObjGet(interp, dict, literals[LIT_ERA], &fieldPtr) != TCL_OK - || fieldPtr == NULL - || Tcl_GetIndexFromObj(interp, fieldPtr, eras, "era", TCL_EXACT, - &era) != TCL_OK + if (FetchEraField(interp, dict, literals[LIT_ERA], &era) != TCL_OK || FetchIntField(interp, dict, literals[LIT_YEAR], &fields.year) != TCL_OK || FetchIntField(interp, dict, literals[LIT_MONTH], &fields.month) @@ -640,7 +656,6 @@ ClockGetjuliandayfromerayearweekdayObjCmd( Tcl_Obj *dict; ClockClientData *data = clientData; Tcl_Obj *const *literals = data->literals; - Tcl_Obj *fieldPtr; int changeover; int copied = 0; int status; @@ -655,10 +670,7 @@ ClockGetjuliandayfromerayearweekdayObjCmd( return TCL_ERROR; } dict = objv[1]; - if (Tcl_DictObjGet(interp, dict, literals[LIT_ERA], &fieldPtr) != TCL_OK - || fieldPtr == NULL - || Tcl_GetIndexFromObj(interp, fieldPtr, eras, "era", TCL_EXACT, - &era) != TCL_OK + if (FetchEraField(interp, dict, literals[LIT_ERA], &era) != TCL_OK || FetchIntField(interp, dict, literals[LIT_ISO8601YEAR], &fields.iso8601Year) != TCL_OK || FetchIntField(interp, dict, literals[LIT_ISO8601WEEK], -- cgit v0.12