Skip to content

eval_sv: add a G_USEHINTS flag #21421

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
tonycoz merged 2 commits into from
Aug 31, 2023
Merged

eval_sv: add a G_USEHINTS flag #21421

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e4a57a5
features: populate cop_features from a hints hash the hard way
tonycoz Aug 23, 2023
346e58f
eval_sv: add a G_USEHINTS flag
tonycoz Aug 23, 2023
File filter

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
features: populate cop_features from a hints hash the hard way 
I originally optimised populating cop_features for eval by storing
the hints mask in "feature/bits" and then fetching that when
re-populating the hints for eval.

But that has turned out to be too fragile, so iterate over the
possible feature keys and populate cop_features from that.

I could perhaps have avoided this cost by ensuring "feature/bits" was
set where else it was needed, but this code already iterates to build
the hints hash, iterating again doesn't increase the scale of the work
we're doing.
  • Loading branch information
@tonycoz
tonycoz committed Aug 28, 2023
commit e4a57a577b746f91fca47676996280f5a3818efa
172 changes: 163 additions & 9 deletions feature.h
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion op.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12227,7 +12227,6 @@ Perl_ck_eval(pTHX_ OP *o)
/* Store a copy of %^H that pp_entereval can pick up. */
HV *hh = hv_copy_hints_hv(GvHV(PL_hintgv));
OP *hhop;
STOREFEATUREBITSHH(hh);
Copy link
Contributor

@leonerd leonerd Aug 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, took me a little while to work out why this bit, until I saw that we're now reconstructing the feature bits from the entire hinthash. Is that OK? In general the hh might contain a lot more keys (e.g. other bits of enabled syntax and options), so iterating the whole thing might take a little while, as compared just storing/fetching the UV as we used to.
hhop = newSVOP(OP_HINTSEVAL, 0, MUTABLE_SV(hh));
/* append hhop to only child */
op_sibling_splice(o, cUNOPo->op_first, 0, hhop);
Expand Down
54 changes: 45 additions & 9 deletions regen/feature.pl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -392,14 +392,7 @@ sub longest {

#define CLEARFEATUREBITS() (PL_compiling.cop_features = 0)

#define STOREFEATUREBITSHH(hh) \\
(hv_stores((hh), "feature/bits", newSVuv(PL_compiling.cop_features)))

#define FETCHFEATUREBITSHH(hh) \\
STMT_START { \\
SV **fbsv = hv_fetchs((hh), "feature/bits", FALSE); \\
PL_compiling.cop_features = fbsv ? SvUV(*fbsv) : 0; \\
} STMT_END
#define FETCHFEATUREBITSHH(hh) S_fetch_feature_bits_hh(aTHX_ (hh))
Copy link
Contributor

@bulk88 bulk88 Apr 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This commit turned 1 hv_fetch(), into 26 always failing hv_fetch() calls, on a HV* with only 1 key/1 HE* in it, with a key name of "CORE/prevailing_version".

#endif /* PERL_CORE or PERL_EXT */

Expand Down Expand Up @@ -434,7 +427,7 @@ sub longest {
}
#endif /* PERL_IN_OP_C */

#ifdef PERL_IN_MG_C
#if defined(PERL_IN_MG_C) || defined(PERL_IN_PP_CTL_C)

#define magic_sethint_feature(keysv, keypv, keylen, valsv, valbool) \\
S_magic_sethint_feature(aTHX_ (keysv), (keypv), (keylen), (valsv), (valbool))
Expand Down Expand Up @@ -491,6 +484,49 @@ sub longest {
}
#endif /* PERL_IN_MG_C */

/* subject to change */
struct perl_feature_bit {
const char *name;
STRLEN namelen;
U32 mask;
};

#ifdef PERL_IN_PP_CTL_C

static const struct perl_feature_bit
PL_feature_bits[] = {
EOJ
for my $key (sort keys %feature) {
my $val = $feature{$key};
print $h <<EOJ;
{
/* feature $key */
"feature_$val",
STRLENs("feature_$val"),
FEATURE_\U$val\E_BIT
},
EOJ
}

print $h <<EOJ;
{ NULL, 0, 0U }
};

PERL_STATIC_INLINE void
S_fetch_feature_bits_hh(pTHX_ HV *hh) {
PL_compiling.cop_features = 0;

const struct perl_feature_bit *fb = PL_feature_bits;
while (fb->name) {
SV **svp = hv_fetch(hh, fb->name, (I32)fb->namelen, 0);
if (svp && SvTRUE(*svp))
PL_compiling.cop_features |= fb->mask;
++fb;
Copy link
Contributor

@bulk88 bulk88 Apr 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This while loop has 26 hash key name entries to test/look for. All 26 hash keys, are never found in typical production perl code. The HV* called hh, has only 1 HE*/key in it total called "CORE/prevailing_version". Pretty bad O(n) problems here with this PR.
}
}

#endif

#endif /* PERL_FEATURE_H_ */
EOJ

Expand Down