Skip to content
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.

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

sv.h: struct body_details bodies_by_type is unreadable, add more comments #22880

Open
wants to merge 1 commit into
base: blead
Choose a base branch
from

Conversation

bulk88
Copy link
Contributor

@bulk88 bulk88 commented Jan 1, 2025

-Comments only patch.

-Note! This commit edits sv_inline.h.
sv.h is NOT edited. "sv_inline.h" is too long to fit in the git title.
"sv.h: improve comments" is the same thing as
"sv_inline.h: improve comments" for anyone looking at the git log.

-the C type decl of "struct body_details" is more than 1 screenful up,
from the initialization and setting contents of "struct body_details".
-setting up a split-screen in an IDE takes a couple clicks/key-strokes
and a few seconds of time
-alternative of pressing PgUp/PgDwn still takes wall time -pressing key F1 "Jump next bookmark" takes less wall time PgUp/PgDwn
but still takes wall time
-pressing any key even once, is slower than moving your eyes -no core or CPAN XS dev needs to unconditionally memorizing what is
inside "struct body_details" as bare minimum Perl C API knowledge required
to write bug-free-stable Perl C code and stop SEGVs/heap corruption.
Remembering "SV *nsv = newSV(0); *nsv = *old_sv;" is illegal, is the law.
Remembering "struct body_details" values and ghost fields is optional.
-CPAN perl illguts is nice, but too out of date to be safe to use or code
against. Last updated was in 2014 for 5.20. Right now this is blead 5.41.
-Recently perl has gotten alot of newSVAnyX(the_X) and
sv_setAnyX_fresh(the_X) optimizations, like inline, and to understand
those commits and changes requires looking at struct body_details alot.

-So to fix all these issues. Add a comment summarizing the field names,
right next to the values of the fields. If you see any line of code,
with a SV body details value initializer, you always will see the summary
comment somewhere on the same screen without scrolling.
-make sure the comment is only 1 line for best screen space use

  • ", NV=0;, " is harder/slower to read than ", NV=0; , " -make sure the comment is always visible in the IDE whenever the cursor
    is inside the values, so the same comment is duplicated a few times to
    fix this bullet point
    -maybe in near future blead will get an optimization, shaving

    newSV(0x00000000) newSV_false() newSV_true() newSV_zero()
    newSVbool(a_bool) newSViv(2) newSVnv(3.0)

down to 3 CPU OPs if x86-32b perl. Makes the body table more readable for any dev writing any future commit in this category or style.

-Chk this commit's PR/ticket for newSVnv() is 3 CPU OP details.


  • This set of changes does not require a perldelta entry.

…ents

-Comments only patch.

-Note! This commit edits sv_inline.h.
 sv.h is NOT edited. "sv_inline.h" is too long to fit in the git title.
 "sv.h: improve comments" is the same thing as
 "sv_inline.h: improve comments" for anyone looking at the git log.

-the C type decl of "struct body_details" is more than 1 screenful up,
 from the initialization and setting contents of "struct body_details".
-setting up a split-screen in an IDE takes a couple clicks/key-strokes
 and a few seconds of time
-alternative of pressing PgUp/PgDwn still takes wall time
-pressing key F1 "Jump next bookmark" takes less wall time PgUp/PgDwn
 but still takes wall time
-pressing any key even once, is slower than moving your eyes
-no core or CPAN XS dev needs to unconditionally memorizing what is
 inside "struct body_details" as bare minimum Perl C API knowledge required
 to write bug-free-stable Perl C code and stop SEGVs/heap corruption.
 Remembering "SV *nsv = newSV(0); *nsv = *old_sv;" is illegal, is the law.
 Remembering "struct body_details" values and ghost fields is optional.
-CPAN perl illguts is nice, but too out of date to be safe to use or code
 against. Last updated was in 2014 for 5.20. Right now this is blead 5.41.
-Recently perl has gotten alot of newSVAnyX(the_X) and
 sv_setAnyX_fresh(the_X) optimizations, like inline, and to understand
 those commits and changes requires looking at struct body_details alot.

-So to fix all these issues. Add a comment summarizing the field names,
 right next to the values of the fields. If you see any line of code,
 with a SV body details value initializer, you always will see the summary
 comment somewhere on the same screen without scrolling.
-make sure the comment is only 1 line for best screen space use
- ", NV=0;, " is harder/slower to read than ", NV=0; , "
-make sure the comment is always visible in the IDE whenever the cursor
 is inside the values, so the same comment is duplicated a few times to
 fix this bullet point
-maybe in near future blead will get an optimization, shaving

  newSV(0x00000000) newSV_false() newSV_true() newSV_zero()
  newSVbool(a_bool) newSViv(2) newSVnv(3.0)

down to 3 CPU OPs if x86-32b perl. Makes the body table more readable
for any dev writing any future commit in this category or style.

-Chk this commit's PR/ticket for newSVnv() is 3 CPU OP details.
@bulk88
Copy link
Contributor Author

bulk88 commented Jan 1, 2025

-Chk this commit's PR/ticket for newSVnv() is 3 CPU OP details.

The post went too off topic as I wrote, it belongs on Medium or Reddit.

It was about allocing out bodyless SVIVs or NVs, with SMID/SSE 16/32 byte mem->reg reg->mem ops, or const-int-literal SV head templates.

basically 32b perl heads are 16 bytes, ARM32/64 NEON FPUs (post iphone 1) ops are 16. Intel SSE is 16. But b/c SvANY slot (# 1) isn't a const lit value, SV head templates are just 4+4+4 on 32b and 4+4+8 on 64b.

64b perl is hv head is 24 bytes. Also 24 isnt aligned.... or a value of 32. QUADMATH NV would make the SV head 32 bytes long. End of story. other one is too long

@@ -180,6 +180,7 @@ ALIGNED_TYPE(XPVOBJ);
STRUCT_OFFSET(type, last_member) \
+ sizeof (((type*)SvANY((const SV *)0))->last_member)

/* sz_if_arena, copy_sz, ghost_sz, SVt, !upg, NV=0; , has_arena, arena_sz */
Copy link
Contributor

Choose a reason for hiding this comment

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

I think using the actual member names would be less confusing, even if they don't have the best possible names.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants