S_invlist_trim maybe doesn't need to SvPV_renew every time?

[richardleach]

Description
S_invlist_trim does a surprising (to me) amount ofrealloc() calls during even
a make perl.

S_invlist_trim always calls realloc() via SvPV_renew, but looking at some of
the underlying values, many of these calls will not result in an actual reallocation
(because of malloc alignment requirements, or larger-than-asked-for allocations),
and the saving otherwise is arguably not worth it.

For example, printing out the relevant values on each call shows:

...
Going to resize from 0 to MAX(9, 1) - actual buffer size is 9
Going to resize from 168 to MAX(9, 169) - actual buffer size is 170
Going to resize from 0 to MAX(9, 1) - actual buffer size is 9
Going to resize from 168 to MAX(9, 169) - actual buffer size is 170
Going to resize from 0 to MAX(9, 1) - actual buffer size is 9
Going to resize from 168 to MAX(9, 169) - actual buffer size is 170
Going to resize from 40 to MAX(9, 41) - actual buffer size is 186
Going to resize from 136 to MAX(9, 137) - actual buffer size is 210
Going to resize from 40 to MAX(9, 41) - actual buffer size is 178
Going to resize from 168 to MAX(9, 169) - actual buffer size is 170
Going to resize from 0 to MAX(9, 1) - actual buffer size is 154
Going to resize from 168 to MAX(9, 169) - actual buffer size is 170
Going to resize from 0 to MAX(9, 1) - actual buffer size is 9
Going to resize from 168 to MAX(9, 169) - actual buffer size is 170
Going to resize from 0 to MAX(9, 1) - actual buffer size is 9
Going to resize from 168 to MAX(9, 169) - actual buffer size is 170
Going to resize from 0 to MAX(9, 1) - actual buffer size is 9
Going to resize from 168 to MAX(9, 169) - actual buffer size is 170
...

Should S_invlist_trim just return without doing the SvPV_renew if:

• MAX(min_size, SvCUR(invlist) + 1) == SvLEN(invlist)
• The difference between the two values is minimal (maybe less than a pointer's worth in it?)

Steps to Reproduce
Apply this quick 'n' dirty patch and run make perl:

diff --git a/regcomp_invlist.c b/regcomp_invlist.c
index 5289d5a5d5..45e53363a9 100644
--- a/regcomp_invlist.c
+++ b/regcomp_invlist.c
@@ -252,7 +252,7 @@ S_invlist_trim(SV* invlist)
     PERL_ARGS_ASSERT_INVLIST_TRIM;
 
     assert(is_invlist(invlist));
-
+PerlIO_stdoutf("Going to resize from %li to MAX(%li, %li) - actual buffer size is %li\n", SvCUR(invlist), min_size, SvCUR(invlist)+1, SvLEN(invlist));
     SvPV_renew(invlist, MAX(min_size, SvCUR(invlist) + 1));
 }
 

Expected behavior
S_invlist_trim returns without reallocating if the size difference is zero or minimal.

Perl configuration
blead

[khwilliamson]

I agree. I thought I brought this up sometime; without any responses. But I might never have gotten round to it.

There are other places in core where I think we shouldn't trim

[richardleach]

What are the conditions - if any - under which S_invlist_trim should do a reallocation, do you think? (I don't have much of an understanding of invlists at all.)