Skip to content

Commit d0b93b3

Browse files
committed
Faster string reversal when source/dest buffers are distinct
`pp_reverse()` has traditionally operated on a single string buffer, reversing it in place. When `pp_reverse()` takes a single SV argument, whose buffer cannot be swiped by `sv_setsv_flags()`, a straight copy of the buffer is taken and then that is reversed. In such cases, two complete passes over the entire string are needed to reverse a non-UTF8 string, and reversing a UTF8 string takes three complete passes. This commit enables `pp_reverse()` to apply the "swipe" test itself and, for straightforward copy cases, to avoid calling `sv_setsv_flags()`. Instead, it does the work of preparing the `TARG` SV and reverse copies the string (UTF8 or otherwise) in a single pass. The performance improvement will vary by compiler and CPU. With gcc 12.2.0 on Linux running on "znver3" hardware, performance for both UTF8 and non-UTF8 strings approximately doubled. Using clang-11 instead on the same machine gave a fivefold improvement for the non-UTF8 case.
1 parent 84fa6b0 commit d0b93b3

File tree

1 file changed

+79
-9
lines changed

1 file changed

+79
-9
lines changed

pp.c

Lines changed: 79 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6603,8 +6603,7 @@ PP_wrapped(pp_reverse, 0, 1)
66036603
SP = oldsp;
66046604
}
66056605
}
6606-
else {
6607-
char *up;
6606+
else { /* GIMME_V != G_LIST. Doing string reversal. */
66086607
dTARGET;
66096608
STRLEN len;
66106609

@@ -6613,17 +6612,88 @@ PP_wrapped(pp_reverse, 0, 1)
66136612
do_join(TARG, &PL_sv_no, MARK, SP);
66146613
SP = MARK + 1;
66156614
SETs(TARG);
6616-
} else if (SP > MARK) {
6617-
sv_setsv_flags(TARG, *SP, SV_GMAGIC);
6618-
SETs(TARG);
66196615
} else {
6620-
sv_setsv_flags(TARG, DEFSV, SV_GMAGIC);
6621-
XPUSHs(TARG);
6616+
SV * src_sv = NULL;
6617+
/* Determine the source SV and get TARG on the stack */
6618+
if (SP > MARK) {
6619+
src_sv = *SP;
6620+
SETs(TARG);
6621+
} else {
6622+
src_sv = DEFSV;
6623+
XPUSHs(TARG);
6624+
}
6625+
assert(src_sv);
6626+
assert(src_sv != TARG);
6627+
6628+
if (/* Fallback to sv_setsv_flags() + in-place reversal if: */
6629+
/* src_sv may need careful handling */
6630+
SvTYPE(src_sv) > SVt_PVMG || SvGMAGICAL(src_sv) ||
6631+
SvVOK(src_sv) ||
6632+
/* src_sv doesn't contain a valid string */
6633+
!(SvFLAGS(src_sv) & SVp_POK) ||
6634+
/* TARG may need careful handling */
6635+
SvTYPE(TARG) > SVt_PVMG ||
6636+
/* sv_setsv_flags() will swipe src_sv's buffer */
6637+
sv_can_swipe_pv_buf(src_sv)
6638+
) {
6639+
sv_setsv_flags(TARG, src_sv, SV_GMAGIC);
6640+
/* FALLTHROUGH */
6641+
} else { /* Source & destination buffers are distinct. By not
6642+
* calling sv_setsv_flags(), we can do a reverse copy
6643+
* in a single pass, rather than 2-3 passes. */
6644+
6645+
const char * src = SvPV_const(src_sv, len);
6646+
6647+
/* Prepare the TARG. */
6648+
if (SvTYPE(TARG) < SVt_PV) {
6649+
SvUPGRADE(TARG, SvTYPE(src_sv)); /* No buffer allocation here */
6650+
} else if(SvTHINKFIRST(TARG)) {
6651+
SV_CHECK_THINKFIRST_COW_DROP(TARG); /* Drops any buffer */
6652+
}
6653+
SvSETMAGIC(TARG);
6654+
SvGROW(TARG, len + 1);
6655+
SvCUR_set(TARG, len);
6656+
SvPOK_only(TARG);
6657+
*SvEND(TARG) = '\0';
6658+
if (SvTAINTED(src_sv))
6659+
SvTAINT(TARG);
6660+
6661+
/* Do the reverse copy */
6662+
if (DO_UTF8(src_sv)) {
6663+
SvUTF8_on(TARG);
6664+
6665+
const U8* s = (const U8*)src;
6666+
U8* dd = (U8*)(SvPVX(TARG) + len);
6667+
const U8* send = (const U8*)(s + len);
6668+
int bytes = 0;
6669+
while (s < send) {
6670+
bytes = UTF8SKIP(s);
6671+
if (bytes == 1) {
6672+
*--dd = *s++;
6673+
} else {
6674+
dd -= bytes;
6675+
U8* d2 = dd;
6676+
while (bytes-- > 0)
6677+
*d2++ = *s++;
6678+
}
6679+
}
6680+
} else {
6681+
char * outp= SvPVX(TARG);
6682+
const char *p = src + len;
6683+
while (p != src)
6684+
*outp++ = *--p;
6685+
}
6686+
RETURN;
6687+
}
66226688
}
6689+
6690+
/* Traditional in-place reversal routines */
66236691
SvSETMAGIC(TARG); /* remove any utf8 length magic */
66246692

6625-
up = SvPV_force(TARG, len);
6693+
char *up = SvPV_force(TARG, len);
6694+
66266695
if (len > 1) {
6696+
/* The traditional way, operate on the current byte buffer */
66276697
char *down;
66286698
if (DO_UTF8(TARG)) { /* first reverse each character */
66296699
U8* s = (U8*)SvPVX(TARG);
@@ -6655,8 +6725,8 @@ PP_wrapped(pp_reverse, 0, 1)
66556725
*up++ = *down;
66566726
*down-- = tmp;
66576727
}
6658-
(void)SvPOK_only_UTF8(TARG);
66596728
}
6729+
(void)SvPOK_only_UTF8(TARG);
66606730
}
66616731
RETURN;
66626732
}

0 commit comments

Comments
 (0)