Skip to content

Commit

Permalink
Update SvPV() etc to avoid calling sv_2pv_flags() for cached IV strings.
Browse files Browse the repository at this point in the history
This "teaches" them the new SV flags combination implemented by the previous
commit.
  • Loading branch information
nwc10 committed Feb 19, 2022
1 parent bb5bc97 commit cff7643
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 11 deletions.
11 changes: 7 additions & 4 deletions sv.c
Expand Up @@ -3316,11 +3316,14 @@ Perl_sv_2pv_flags(pTHX_ SV *const sv, STRLEN *const lp, const U32 flags)
where the value has subsequently been used in the other sense
and had a value cached.
This (somewhat) hack means that we retain the cached stringification,
but the existing SvPV() macros end up entering this function (which
returns the cached value) instead of using it directly inline.
However, the result is that if a value is SVf_IOK|SVf_POK then it
but don't set SVf_POK. Hence if a value is SVf_IOK|SVf_POK then it
originated as "42", whereas if it's SVf_IOK then it originated as 42.
(ignore SVp_IOK and SVp_POK) */
(ignore SVp_IOK and SVp_POK)
The SvPV macros are now updated to recognise this specific case
(and that there isn't overloading or magic that could alter the
cached value) and so return the cached value immediately without
re-entering this function, getting back here to this block of code,
and repeating the same conversion. */
SvPOKp_on(sv);
}
else if (SvNOK(sv)) {
Expand Down
25 changes: 18 additions & 7 deletions sv.h
Expand Up @@ -1859,19 +1859,30 @@ scalar.
#define SvPV_const(sv, len) SvPV_flags_const(sv, len, SV_GMAGIC)
#define SvPV_mutable(sv, len) SvPV_flags_mutable(sv, len, SV_GMAGIC)

/* This test is "is there a cached PV that we can use directly?"
* We can if
* a) SVf_POK is true and there's definitely no get magic on the scalar
* b) SVp_POK is true, there's no get magic, and we know that the cached PV
* came from an IV conversion.
* For the latter case, we don't set SVf_POK so that we can distinguish whether
* the value originated as a string or as an integer, before we cached the
* second representation. */
#define SvPOK_or_cached_IV(sv) \
(((SvFLAGS(sv) & (SVf_POK|SVs_GMG)) == SVf_POK) || ((SvFLAGS(sv) & (SVf_IOK|SVp_POK|SVs_GMG)) == (SVf_IOK|SVp_POK)))

#define SvPV_flags(sv, len, flags) \
(SvPOK_nog(sv) \
(SvPOK_or_cached_IV(sv) \
? ((len = SvCUR(sv)), SvPVX(sv)) : sv_2pv_flags(sv, &len, flags))
#define SvPV_flags_const(sv, len, flags) \
(SvPOK_nog(sv) \
(SvPOK_or_cached_IV(sv) \
? ((len = SvCUR(sv)), SvPVX_const(sv)) : \
(const char*) sv_2pv_flags(sv, &len, (flags|SV_CONST_RETURN)))
#define SvPV_flags_const_nolen(sv, flags) \
(SvPOK_nog(sv) \
(SvPOK_or_cached_IV(sv) \
? SvPVX_const(sv) : \
(const char*) sv_2pv_flags(sv, 0, (flags|SV_CONST_RETURN)))
#define SvPV_flags_mutable(sv, len, flags) \
(SvPOK_nog(sv) \
(SvPOK_or_cached_IV(sv) \
? ((len = SvCUR(sv)), SvPVX_mutable(sv)) : \
sv_2pv_flags(sv, &len, (flags|SV_MUTABLE_RETURN)))

Expand All @@ -1896,16 +1907,16 @@ scalar.
: sv_pvn_force_flags(sv, &len, flags|SV_MUTABLE_RETURN))

#define SvPV_nolen(sv) \
(SvPOK_nog(sv) \
(SvPOK_or_cached_IV(sv) \
? SvPVX(sv) : sv_2pv_flags(sv, 0, SV_GMAGIC))

/* "_nomg" in these defines means no mg_get() */
#define SvPV_nomg_nolen(sv) \
(SvPOK_nog(sv) \
(SvPOK_or_cached_IV(sv) \
? SvPVX(sv) : sv_2pv_flags(sv, 0, 0))

#define SvPV_nolen_const(sv) \
(SvPOK_nog(sv) \
(SvPOK_or_cached_IV(sv) \
? SvPVX_const(sv) : sv_2pv_flags(sv, 0, SV_GMAGIC|SV_CONST_RETURN))

#define SvPV_nomg(sv, len) SvPV_flags(sv, len, 0)
Expand Down

0 comments on commit cff7643

Please sign in to comment.