From 21ad2c33e88cdca35006778053c71100709ccea8 Mon Sep 17 00:00:00 2001 From: dgp Date: Mon, 5 Jun 2017 17:57:33 +0000 Subject: Revert performance optimization as first step to providing a refactored one. --- generic/tclExecute.c | 25 +------------------------ 1 file changed, 1 insertion(+), 24 deletions(-) diff --git a/generic/tclExecute.c b/generic/tclExecute.c index 30ef536..cfcdd26 100644 --- a/generic/tclExecute.c +++ b/generic/tclExecute.c @@ -2685,34 +2685,11 @@ TEBCresume( opnd = TclGetUInt1AtPtr(pc+1); - objv = &OBJ_AT_DEPTH(opnd-1); - /* minor optimization in simplest cases */ - switch (opnd) { - case 1: /* only one object */ - objResultPtr = *objv; - goto endINST_STR_CONCAT1; - case 2: /* two objects - check empty */ - if (objv[0]->bytes == &tclEmptyString) { - objResultPtr = objv[1]; - goto endINST_STR_CONCAT1; - } - else - if (objv[1]->bytes == &tclEmptyString) { - objResultPtr = objv[0]; - goto endINST_STR_CONCAT1; - } - break; - case 0: /* no objects - use new empty */ - TclNewObj(objResultPtr); - goto endINST_STR_CONCAT1; - } - /* do concat */ if (TCL_OK != TclStringCatObjv(interp, /* inPlace */ 1, - opnd, objv, &objResultPtr)) { + opnd, &OBJ_AT_DEPTH(opnd-1), &objResultPtr)) { TRACE_ERROR(interp); goto gotError; } - endINST_STR_CONCAT1: TRACE_WITH_OBJ(("%u => ", opnd), objResultPtr); NEXT_INST_V(2, opnd, 1); -- cgit v0.12 From 735ced34c942925be92107aa7752ab143eaf6fb2 Mon Sep 17 00:00:00 2001 From: dgp Date: Mon, 5 Jun 2017 20:03:19 +0000 Subject: Optimize TclStringCatObjv() for case when only one argument is non-empty. --- generic/tclStringObj.c | 30 ++++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/generic/tclStringObj.c b/generic/tclStringObj.c index 3478cbb..a4c242a 100644 --- a/generic/tclStringObj.c +++ b/generic/tclStringObj.c @@ -2848,7 +2848,7 @@ TclStringCatObjv( Tcl_Obj **objPtrPtr) { Tcl_Obj *objPtr, *objResultPtr, * const *ov; - int oc, length = 0, binary = 1, first = 0; + int oc, length = 0, binary = 1, first = 0, last = 0; int allowUniChar = 1, requestUniChar = 0; /* assert (objc >= 2) */ @@ -2904,8 +2904,11 @@ TclStringCatObjv( int numBytes; Tcl_GetByteArrayFromObj(objPtr, &numBytes); /* PANIC? */ - if (length == 0) { - first = objc - oc - 1; + if (numBytes) { + last = objc - oc - 1; + if (length == 0) { + first = last; + } } length += numBytes; } @@ -2920,8 +2923,11 @@ TclStringCatObjv( int numChars; Tcl_GetUnicodeFromObj(objPtr, &numChars); /* PANIC? */ - if (length == 0) { - first = objc - oc - 1; + if (numChars) { + last = objc - oc - 1; + if (length == 0) { + first = last; + } } length += numChars; } @@ -2935,8 +2941,11 @@ TclStringCatObjv( objPtr = *ov++; Tcl_GetStringFromObj(objPtr, &numBytes); /* PANIC? */ - if ((length == 0) && numBytes) { - first = objc - oc - 1; + if (numBytes) { + last = objc - oc - 1; + if (length == 0) { + first = last; + } } length += numBytes; } @@ -2956,8 +2965,13 @@ TclStringCatObjv( *objPtrPtr = objv[0]; return TCL_OK; } + if (last == first) { + /* Only one non-empty value; return it */ + *objPtrPtr = objv[first]; + return TCL_OK; + } - objv += first; objc -= first; + objv += first; objc = (last - first + 1); if (binary) { /* Efficiently produce a pure byte array result */ -- cgit v0.12 From 1172340e247bd64f10a4c5e5b812bd5283ffbb83 Mon Sep 17 00:00:00 2001 From: sebres Date: Tue, 6 Jun 2017 08:34:00 +0000 Subject: small code review: don't need to check length if unchanged + the same case if 0 length --- generic/tclStringObj.c | 35 ++++++++++++++++++----------------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/generic/tclStringObj.c b/generic/tclStringObj.c index a4c242a..c1c15f2 100644 --- a/generic/tclStringObj.c +++ b/generic/tclStringObj.c @@ -2897,7 +2897,7 @@ TclStringCatObjv( if (binary) { /* Result will be pure byte array. Pre-size it */ ov = objv; oc = objc; - while (oc-- && (length >= 0)) { + while (oc--) { objPtr = *ov++; if (objPtr->bytes == NULL) { @@ -2909,14 +2909,16 @@ TclStringCatObjv( if (length == 0) { first = last; } + if ((length += numBytes) < 0) { + break; /* overflow */ + } } - length += numBytes; } } } else if (allowUniChar && requestUniChar) { /* Result will be pure Tcl_UniChar array. Pre-size it. */ ov = objv; oc = objc; - while (oc-- && (length >= 0)) { + while (oc--) { objPtr = *ov++; if ((objPtr->bytes == NULL) || (objPtr->length)) { @@ -2929,13 +2931,15 @@ TclStringCatObjv( first = last; } } - length += numChars; + if ((length += numChars) < 0) { + break; /* overflow */ + } } } } else { /* Result will be concat of string reps. Pre-size it. */ ov = objv; oc = objc; - while (oc-- && (length >= 0)) { + while (oc--) { int numBytes; objPtr = *ov++; @@ -2946,11 +2950,19 @@ TclStringCatObjv( if (length == 0) { first = last; } + if ((length += numBytes) < 0) { + break; /* overflow */ + } } - length += numBytes; } } + if (last == first || length == 0) { + /* Only one non-empty value or zero length; return first */ + *objPtrPtr = objv[first]; + return TCL_OK; + } + if (length < 0) { if (interp) { Tcl_SetObjResult(interp, Tcl_ObjPrintf( @@ -2960,17 +2972,6 @@ TclStringCatObjv( return TCL_ERROR; } - if (length == 0) { - /* Total length of zero means every value has length zero */ - *objPtrPtr = objv[0]; - return TCL_OK; - } - if (last == first) { - /* Only one non-empty value; return it */ - *objPtrPtr = objv[first]; - return TCL_OK; - } - objv += first; objc = (last - first + 1); if (binary) { -- cgit v0.12 From 449b9c2c29e600d4e02c0430f360262aabc2ddb7 Mon Sep 17 00:00:00 2001 From: sebres Date: Tue, 6 Jun 2017 09:01:18 +0000 Subject: amend to [eac4656f1e8cf793] (moved to scope where numChars != 0 in Unicode case) --- generic/tclStringObj.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/generic/tclStringObj.c b/generic/tclStringObj.c index c1c15f2..6332e9f 100644 --- a/generic/tclStringObj.c +++ b/generic/tclStringObj.c @@ -2930,9 +2930,9 @@ TclStringCatObjv( if (length == 0) { first = last; } - } - if ((length += numChars) < 0) { - break; /* overflow */ + if ((length += numChars) < 0) { + break; /* overflow */ + } } } } -- cgit v0.12 From 47558a128c45f7915b69e10652573089efc3a897 Mon Sep 17 00:00:00 2001 From: sebres Date: Tue, 6 Jun 2017 09:25:00 +0000 Subject: makes TclStringCatObjv safe accepting objc = 0 (or 1), then fast exits with new object / first; check-cycles rewritten to be still more faster. --- generic/tclStringObj.c | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/generic/tclStringObj.c b/generic/tclStringObj.c index 6332e9f..b78394e 100644 --- a/generic/tclStringObj.c +++ b/generic/tclStringObj.c @@ -2851,7 +2851,11 @@ TclStringCatObjv( int oc, length = 0, binary = 1, first = 0, last = 0; int allowUniChar = 1, requestUniChar = 0; - /* assert (objc >= 2) */ + if (objc <= 1) { + /* Only one or no objects; return first or empty */ + *objPtrPtr = objc ? objv[0] : Tcl_NewObj(); + return TCL_OK; + } /* * Analyze to determine what representation result should be. @@ -2861,7 +2865,7 @@ TclStringCatObjv( */ ov = objv, oc = objc; - while (oc-- && (binary || allowUniChar)) { + do { objPtr = *ov++; if (objPtr->bytes) { @@ -2892,12 +2896,12 @@ TclStringCatObjv( } } } - } + } while (--oc && (binary || allowUniChar)); if (binary) { /* Result will be pure byte array. Pre-size it */ ov = objv; oc = objc; - while (oc--) { + do { objPtr = *ov++; if (objPtr->bytes == NULL) { @@ -2905,7 +2909,7 @@ TclStringCatObjv( Tcl_GetByteArrayFromObj(objPtr, &numBytes); /* PANIC? */ if (numBytes) { - last = objc - oc - 1; + last = objc - oc; if (length == 0) { first = last; } @@ -2914,11 +2918,11 @@ TclStringCatObjv( } } } - } + } while (--oc); } else if (allowUniChar && requestUniChar) { /* Result will be pure Tcl_UniChar array. Pre-size it. */ ov = objv; oc = objc; - while (oc--) { + do { objPtr = *ov++; if ((objPtr->bytes == NULL) || (objPtr->length)) { @@ -2926,7 +2930,7 @@ TclStringCatObjv( Tcl_GetUnicodeFromObj(objPtr, &numChars); /* PANIC? */ if (numChars) { - last = objc - oc - 1; + last = objc - oc; if (length == 0) { first = last; } @@ -2935,18 +2939,18 @@ TclStringCatObjv( } } } - } + } while (--oc); } else { /* Result will be concat of string reps. Pre-size it. */ ov = objv; oc = objc; - while (oc--) { + do { int numBytes; objPtr = *ov++; Tcl_GetStringFromObj(objPtr, &numBytes); /* PANIC? */ if (numBytes) { - last = objc - oc - 1; + last = objc - oc; if (length == 0) { first = last; } @@ -2954,7 +2958,7 @@ TclStringCatObjv( break; /* overflow */ } } - } + } while (--oc); } if (last == first || length == 0) { -- cgit v0.12 From 93784caa4d5d0a0dc6fb02b30f273b4e95a73489 Mon Sep 17 00:00:00 2001 From: dgp Date: Tue, 6 Jun 2017 12:56:37 +0000 Subject: A few more tweaks to streamline and clarify. --- generic/tclStringObj.c | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/generic/tclStringObj.c b/generic/tclStringObj.c index b78394e..aae52ba 100644 --- a/generic/tclStringObj.c +++ b/generic/tclStringObj.c @@ -2851,12 +2851,16 @@ TclStringCatObjv( int oc, length = 0, binary = 1, first = 0, last = 0; int allowUniChar = 1, requestUniChar = 0; + /* assert ( objc >= 0 ) */ + if (objc <= 1) { /* Only one or no objects; return first or empty */ *objPtrPtr = objc ? objv[0] : Tcl_NewObj(); return TCL_OK; } + /* assert ( objc >= 2 ) */ + /* * Analyze to determine what representation result should be. * GOALS: Avoid shimmering & string rep generation. @@ -2914,7 +2918,7 @@ TclStringCatObjv( first = last; } if ((length += numBytes) < 0) { - break; /* overflow */ + goto overflow; } } } @@ -2935,7 +2939,7 @@ TclStringCatObjv( first = last; } if ((length += numChars) < 0) { - break; /* overflow */ + goto overflow; } } } @@ -2955,27 +2959,19 @@ TclStringCatObjv( first = last; } if ((length += numBytes) < 0) { - break; /* overflow */ + goto overflow; } } } while (--oc); } - if (last == first || length == 0) { + if (last == first /*|| length == 0 */) { /* Only one non-empty value or zero length; return first */ + /* NOTE: (length == 0) implies (last == first) */ *objPtrPtr = objv[first]; return TCL_OK; } - if (length < 0) { - if (interp) { - Tcl_SetObjResult(interp, Tcl_ObjPrintf( - "max size for a Tcl value (%d bytes) exceeded", INT_MAX)); - Tcl_SetErrorCode(interp, "TCL", "MEMORY", NULL); - } - return TCL_ERROR; - } - objv += first; objc = (last - first + 1); if (binary) { @@ -3106,6 +3102,14 @@ TclStringCatObjv( } *objPtrPtr = objResultPtr; return TCL_OK; + + overflow: + if (interp) { + Tcl_SetObjResult(interp, Tcl_ObjPrintf( + "max size for a Tcl value (%d bytes) exceeded", INT_MAX)); + Tcl_SetErrorCode(interp, "TCL", "MEMORY", NULL); + } + return TCL_ERROR; } /* -- cgit v0.12