X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=blobdiff_plain;f=t%2Frecurse.t;h=2b2e1668a51a5c0b16c0de7bf9530935c788ac54;hb=a8fa215c7b5b72b98f8e6029907087b0ff176594;hp=4c67b29661d867322a7040650788f983189f4039;hpb=a52ceccdd55939cd397dcc1f4474d509ed47e5cb;p=p5sagit%2FDevel-Size.git diff --git a/t/recurse.t b/t/recurse.t index 4c67b29..2b2e166 100644 --- a/t/recurse.t +++ b/t/recurse.t @@ -9,6 +9,7 @@ use Test::More; use strict; use Devel::Size ':all'; +use Config; my %types = ( NULL => undef, @@ -21,7 +22,7 @@ my %types = ( PVMG => do { my $a = $!; $a = "Bang!"; $a }, ); -plan(tests => 16 + 4 *12 + 2 * scalar keys %types); +plan(tests => 20 + 4 * 12 + 2 * scalar keys %types); ############################################################################# # verify that pointer sizes in array slots are sensible: @@ -282,3 +283,95 @@ sub cmp_array_ro { "Type $type, total_size() recurses to the referent"); } } + +# The intent of the following block of tests was to avoid repeating the +# potential regression if one changes how hashes are iterated. Specifically, +# commit f3cf7e20cc2a7a5a moves the iteration over hash values from total_size() +# to sv_size(). The final commit is complex, and somewhat a hack, as described +# in the comment in Size.xs above the definition of "NO_RECURSION". + +# My original assumption was that the change (moving the iteration) was going to +# be simple, and look something like this: + +=for a can of worms :-( + +--- Size.xs 2015-03-20 21:00:31.000000000 +0100 ++++ ../Devel-Size-messy/Size.xs 2015-03-20 20:51:19.000000000 +0100 +@@ -615,6 +615,8 @@ + st->total_size += HEK_BASESIZE + cur_entry->hent_hek->hek_len + 2; + } + } ++ if (recurse) ++ sv_size(aTHX_ st, HeVAL(cur_entry), recurse); + cur_entry = cur_entry->hent_next; + } + } +@@ -828,17 +830,6 @@ + } + } + TAG;break; +- +- case SVt_PVHV: TAG; +- dbg_printf(("# Found type HV\n")); +- /* Is there anything in here? */ +- if (hv_iterinit((HV *)thing)) { +- HE *temp_he; +- while ((temp_he = hv_iternext((HV *)thing))) { +- av_push(pending_array, hv_iterval((HV *)thing, temp_he)); +- } +- } +- TAG;break; + + case SVt_PVGV: TAG; + dbg_printf(("# Found type GV\n")); + +=cut + +# nice and clean, removes 11 lines of special case clause for SVt_PVHV, adding +# only 2 into an existing loop. + +# And it opened up a total can of worms. Existing tests failed because typeglobs +# in subroutines leading to symbol tables were now being followed, making +# reported sizes for subroutines now massively bigger. + +# And it turned out (or seemed to be) that subroutines could even end up +# dragging in the entire symbol table in some cases. Hence a block of tests +# was added to verify that the reported size of &cmp_array_ro didn't explode as +# a result of this (or any further) refactoring. + +# Obviously the patch above is broken, so it never got applied. But the test to +# prevent it *did*. Which was fine for 4 years. Except that it turns out that +# the test is actually sensitive to the size of Test::More::is() (because the +# subroutine cmp_array_ro() calls is()). And hence the test now *fails* because +# Test::More::is() got refactored. + +# Which is a pain. +# So we get back to "what are we actually trying to test?" +# And really, the minimal thing that we were actually trying to test all along +# was *only* that a subroutine in a package with (other) imported subroutines +# doesn't get the size of their package rolled into it. +# Hence *this* is what the test should have been all along: + +{ + package SWIT; + use Test::More; + sub sees_test_more { + # This subroutine is in a package whose stash now contains typeglobs + # which point to subroutines in Test::More. \%Test::More:: is rather + # big, and we shouldn't be counting is size as part of the size of this + # (empty!) subroutine. + } +} + +{ + # This used to be total_size(\&cmp_array_ro); + my $sub_size = total_size(\&SWIT::sees_test_more); + my $want = 1.5 + 0.125 * $Config{ptrsize}; + cmp_ok($sub_size, '>=', $want, "subroutine is at least ${want}K"); + cmp_ok($sub_size, '<=', 51200, 'subroutine is no more than 50K') + or diag 'Is total_size() dragging in the entire symbol table?'; + cmp_ok(total_size(\%Test::More::), '>=', 102400, + "Test::More's symbol table is at least 100K"); +} + +cmp_ok(total_size(\%Exporter::), '>', total_size(\%Exporter::Heavy::));