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

module constant.pm uses +1000 kB in 522 compare to 514 #15154

Closed
p5pRT opened this issue Jan 27, 2016 · 36 comments
Closed

module constant.pm uses +1000 kB in 522 compare to 514 #15154

p5pRT opened this issue Jan 27, 2016 · 36 comments

Comments

@p5pRT
Copy link

p5pRT commented Jan 27, 2016

Migrated from rt.perl.org#127392 (status was 'resolved')

Searchable as RT127392$

@p5pRT
Copy link
Author

p5pRT commented Jan 27, 2016

From @atoomic

The memory increase is not explained by one single commit, it's probably an addition of few more things.
But we can identify one commit between v5.19.7 and v5.19.8 which explains the main increase of memory.

# use perl5.22.1 version of constant

perl514 -I~/perl/522/lib/perl5/5.22.1/ -e 'use constant; print qx{grep VmRSS /proc/$$/status}';
VmRSS​: 1920 kB
perl522 -e 'use constant; print qx{grep VmRSS /proc/$$/status}'
VmRSS​: 3048 kB

After applying the suggested patch to constant this can be reduced by ~800 kB
the memory increase is mainly coming from the optional _ at the beginning of the regexp
my $normal_constant_name = qr/^_?[^\W_0-9]\w*\z/;

perl514 -I~/perl/522/lib/perl5/5.22.1/ -e 'use constant; print qx{grep VmRSS /proc/$$/status}';
VmRSS​: 1920 kB
perl522 -e 'use constant; print qx{grep VmRSS /proc/$$/status}'
VmRSS​: 2260 kB

Doing a git bisect I could identify this commit as one of the biggest memory increase​:
http​://perl5.git.perl.org/perl.git/commitdiff/bcb875216f24899d543c036aebdba0835f8d22e6

Before commit HEAD=bcb8752^

/root/perlbin_tmp/bin/perl -e 'my $f = $ENV{user} =~ qr{_?[\W\_]}; print qx{grep VmRSS /proc/$$/status}'
VmRSS​: 1684 kB
/root/perlbin_tmp/bin/perl -e 'use constant FOO => 42; print qx{grep VmRSS /proc/$$/status}'
VmRSS​: 2292 kB

With HEAD=bcb8752

/root/perlbin_tmp/bin/perl -e 'my $f = $ENV{user} =~ qr{_?[\W\_]}; print qx{grep VmRSS /proc/$$/status}'
VmRSS​: 1996 kB
/root/perlbin_tmp/bin/perl -e 'use constant FOO => 42; print qx{grep VmRSS /proc/$$/status}'
VmRSS​: 2644 kB

More detailed analysis of memory usage over versions

# 5.14.4

p14 -e 'my $f = $ENV{user} =~ qr{_?[\W\_]}; print qx{grep VmRSS /proc/$$/status}'
VmRSS​: 1488 kB

# 5.18.1

/root/perlbin_tmp/bin/perl5.18.1 -e 'my $f = $ENV{user} =~ qr{_?[\W\_]}; print qx{grep VmRSS /proc/$$/status}'
VmRSS​: 1616 kB

# 5.18.4

/root/perlbin_tmp/bin/perl5.18.4 -e 'my $f = $ENV{user} =~ qr{_?[\W\_]}; print qx{grep VmRSS /proc/$$/status}'
VmRSS​: 1612 kB

# 5.19.0

/root/perlbin_tmp/bin/perl5.19.0 -e 'my $f = $ENV{user} =~ qr{_?[\W\_]}; print qx{grep VmRSS /proc/$$/status}'
VmRSS​: 1604 kB

# 5.19.6

/root/perlbin_tmp/bin/perl -e 'my $f = $ENV{user} =~ qr{_?[\W\_]}; print qx{grep VmRSS /proc/$$/status}'
VmRSS​: 1664 kB

# 5.19.7

/root/perlbin_tmp/bin/perl -e 'my $f = $ENV{user} =~ qr{_?[\W\_]}; print qx{grep VmRSS /proc/$$/status}'
VmRSS​: 1648 kB

# 5.19.8

/root/perlbin_tmp/bin/perl -e 'my $f = $ENV{user} =~ qr{_?[\W\_]}; print qx{grep VmRSS /proc/$$/status}'
VmRSS​: 1980 kB

# 5.19.9

/root/perlbin_tmp/bin/perl -e 'my $f = $ENV{user} =~ qr{_?[\W\_]}; print qx{grep VmRSS /proc/$$/status}'
VmRSS​: 2020 kB

# 5.19.11

/root/perlbin_tmp/bin/perl -e 'my $f = $ENV{user} =~ qr{_?[\W\_]}; print qx{grep VmRSS /proc/$$/status}'
VmRSS​: 2028 kB

# 5.20.0

/root/perlbin_tmp/bin/perl -e 'my $f = $ENV{user} =~ qr{_?[\W\_]}; print qx{grep VmRSS /proc/$$/status}'
VmRSS​: 2000 kB

# 5.20.1

/root/perlbin_tmp/bin/perl -e 'my $f = $ENV{user} =~ qr{_?[\W\_]}; print qx{grep VmRSS /proc/$$/status}'
VmRSS​: 2000 kB

# 5.22.1

/root/perlbin_tmp/bin/perl -e 'my $f = $ENV{user} =~ qr{_?[\W\_]}; print qx{grep VmRSS /proc/$$/status}'
VmRSS​: 2160 kB

@p5pRT
Copy link
Author

p5pRT commented Jan 27, 2016

From @atoomic

0001-Reduce-memory-footprint-of-constant.pm.patch
From d212aef5d7f91a80e2370fe7149b8c8084cc6285 Mon Sep 17 00:00:00 2001
From: Nicolas Rochelemagne <rochelemagne@cpanel.net>
Date: Wed, 27 Jan 2016 13:23:25 -0600
Subject: [PATCH] Reduce memory footprint of constant.pm

Compare to v5.14 constant.pm increased its memory
footprint by about ~1000 kB.

The increase cannot be explained by one single commit but
we can bisect that the main increase of memory for constant.pm
appears to come from bcb875216f24899d543 (between v5.19.7 and v5.19.8).

The memory increase comes from updates to regcomp.c. We can workaround
this in constant.pm by simplifying the normal_constant_name regexp and
checking the optional leading '_' differently.

Note that we could also use '$altered_name =~ s{^_}{};' instead of using
substr.

This commit is reducing the footprint of constant.pm by ~800 kB.
---
 dist/constant/lib/constant.pm | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/dist/constant/lib/constant.pm b/dist/constant/lib/constant.pm
index e4b8fd2..5a28aa7 100644
--- a/dist/constant/lib/constant.pm
+++ b/dist/constant/lib/constant.pm
@@ -17,7 +17,9 @@ my %forced_into_main = map +($_, 1),
 
 my %forbidden = (%keywords, %forced_into_main);
 
-my $normal_constant_name = qr/^_?[^\W_0-9]\w*\z/;
+# this can also start by an _ but do not add it as part of the regexp
+# use and extra 800 kB by adding ^_? to the regexp
+my $normal_constant_name = qr/^[^\W_0-9]\w*\z/;
 my $tolerable = qr/^[A-Za-z_]\w*\z/;
 my $boolean = qr/^[01]?\z/;
 
@@ -95,8 +97,13 @@ sub import {
 	    $pkg = $caller;
 	}
 
-	# Normal constant name
-	if ($name =~ $normal_constant_name and !$forbidden{$name}) {
+	my $altered_name = $name;
+	# optional _ at the start: use substr to save some memory
+	if ( my $first = substr( $name, 0, 1 ) ) {
+		substr( $altered_name, 0, 1, '' ) if $first eq '_';
+	}
+
+	if ($altered_name =~ $normal_constant_name and !$forbidden{$name}) {
 	    # Everything is okay
 
 	# Name forced into main, but we're not in main. Fatal.
@@ -119,7 +126,7 @@ sub import {
 		    warnings::warn("Constant name '$name' is " .
 			"forced into package main::");
 		}
-	    }
+    }
 
 	# Looks like a boolean
 	# use constant FRED == fred;
-- 
2.7.0

@p5pRT
Copy link
Author

p5pRT commented Jan 27, 2016

From @jkeenan

On Wed Jan 27 11​:37​:43 2016, atoomic wrote​:

The memory increase is not explained by one single commit, it's
probably an addition of few more things.
But we can identify one commit between v5.19.7 and v5.19.8 which
explains the main increase of memory.

# use perl5.22.1 version of constant

perl514 -I~/perl/522/lib/perl5/5.22.1/ -e 'use constant; print
qx{grep VmRSS /proc/$$/status}';
VmRSS​: 1920 kB
perl522 -e 'use constant; print qx{grep VmRSS /proc/$$/status}'
VmRSS​: 3048 kB

After applying the suggested patch to constant this can be reduced by
~800 kB
the memory increase is mainly coming from the optional _ at the
beginning of the regexp
my $normal_constant_name = qr/^_?[^\W_0-9]\w*\z/;

perl514 -I~/perl/522/lib/perl5/5.22.1/ -e 'use constant; print
qx{grep VmRSS /proc/$$/status}';
VmRSS​: 1920 kB
perl522 -e 'use constant; print qx{grep VmRSS /proc/$$/status}'
VmRSS​: 2260 kB

Doing a git bisect I could identify this commit as one of the biggest
memory increase​:
http​://perl5.git.perl.org/perl.git/commitdiff/bcb875216f24899d543c036aebdba0835f8d22e6

Before commit HEAD=bcb8752^

/root/perlbin_tmp/bin/perl -e 'my $f = $ENV{user} =~ qr{_?[\W\_]};
print qx{grep VmRSS /proc/$$/status}'
VmRSS​: 1684 kB
/root/perlbin_tmp/bin/perl -e 'use constant FOO => 42; print qx{grep
VmRSS /proc/$$/status}'
VmRSS​: 2292 kB

With HEAD=bcb8752

/root/perlbin_tmp/bin/perl -e 'my $f = $ENV{user} =~ qr{_?[\W\_]};
print qx{grep VmRSS /proc/$$/status}'
VmRSS​: 1996 kB
/root/perlbin_tmp/bin/perl -e 'use constant FOO => 42; print qx{grep
VmRSS /proc/$$/status}'
VmRSS​: 2644 kB

More detailed analysis of memory usage over versions

# 5.14.4

p14 -e 'my $f = $ENV{user} =~ qr{_?[\W\_]}; print qx{grep VmRSS
/proc/$$/status}'
VmRSS​: 1488 kB

# 5.18.1

/root/perlbin_tmp/bin/perl5.18.1 -e 'my $f = $ENV{user} =~
qr{_?[\W\_]}; print qx{grep VmRSS /proc/$$/status}'
VmRSS​: 1616 kB

# 5.18.4

/root/perlbin_tmp/bin/perl5.18.4 -e 'my $f = $ENV{user} =~
qr{_?[\W\_]}; print qx{grep VmRSS /proc/$$/status}'
VmRSS​: 1612 kB

# 5.19.0

/root/perlbin_tmp/bin/perl5.19.0 -e 'my $f = $ENV{user} =~
qr{_?[\W\_]}; print qx{grep VmRSS /proc/$$/status}'
VmRSS​: 1604 kB

# 5.19.6

/root/perlbin_tmp/bin/perl -e 'my $f = $ENV{user} =~ qr{_?[\W\_]};
print qx{grep VmRSS /proc/$$/status}'
VmRSS​: 1664 kB

# 5.19.7

/root/perlbin_tmp/bin/perl -e 'my $f = $ENV{user} =~ qr{_?[\W\_]};
print qx{grep VmRSS /proc/$$/status}'
VmRSS​: 1648 kB

# 5.19.8

/root/perlbin_tmp/bin/perl -e 'my $f = $ENV{user} =~ qr{_?[\W\_]};
print qx{grep VmRSS /proc/$$/status}'
VmRSS​: 1980 kB

# 5.19.9

/root/perlbin_tmp/bin/perl -e 'my $f = $ENV{user} =~ qr{_?[\W\_]};
print qx{grep VmRSS /proc/$$/status}'
VmRSS​: 2020 kB

# 5.19.11

/root/perlbin_tmp/bin/perl -e 'my $f = $ENV{user} =~ qr{_?[\W\_]};
print qx{grep VmRSS /proc/$$/status}'
VmRSS​: 2028 kB

# 5.20.0

/root/perlbin_tmp/bin/perl -e 'my $f = $ENV{user} =~ qr{_?[\W\_]};
print qx{grep VmRSS /proc/$$/status}'
VmRSS​: 2000 kB

# 5.20.1

/root/perlbin_tmp/bin/perl -e 'my $f = $ENV{user} =~ qr{_?[\W\_]};
print qx{grep VmRSS /proc/$$/status}'
VmRSS​: 2000 kB

# 5.22.1

/root/perlbin_tmp/bin/perl -e 'my $f = $ENV{user} =~ qr{_?[\W\_]};
print qx{grep VmRSS /proc/$$/status}'
VmRSS​: 2160 kB

These numbers have jumped around a bit between perl versions. See attachment; all probes run on the same machine.

Thank you very much.

--
James E Keenan (jkeenan@​cpan.org)

@p5pRT
Copy link
Author

p5pRT commented Jan 27, 2016

From @jkeenan

$ perlbrew use perl-5.10.1
$ perl -e 'use constant; print qx{grep VmRSS /proc/$$/status}'
VmRSS​: 2056 kB

$ perlbrew use perl-5.14.4
$ perl -e 'use constant; print qx{grep VmRSS /proc/$$/status}'
VmRSS​: 2216 kB

$ perlbrew use perl-5.16.3
$ perl -e 'use constant; print qx{grep VmRSS /proc/$$/status}'
VmRSS​: 4208 kB

$ perlbrew use perl-5.18.2
$ perl -e 'use constant; print qx{grep VmRSS /proc/$$/status}'
VmRSS​: 2344 kB

$ perlbrew use perl-5.20.1
$ perl -e 'use constant; print qx{grep VmRSS /proc/$$/status}'
VmRSS​: 3152 kB

$ perlbrew use perl-5.22.0
$ perl -e 'use constant; print qx{grep VmRSS /proc/$$/status}'
VmRSS​: 3396 kB

@p5pRT
Copy link
Author

p5pRT commented Jan 27, 2016

The RT System itself - Status changed from 'new' to 'open'

@p5pRT
Copy link
Author

p5pRT commented Feb 1, 2016

From @tonycoz

On Wed Jan 27 11​:37​:43 2016, atoomic wrote​:

The memory increase is not explained by one single commit, it's
probably an addition of few more things.
But we can identify one commit between v5.19.7 and v5.19.8 which
explains the main increase of memory.

Most of the extra memory use introduced in bcb8752 seems to be in creating inversion lists, test code​:

PERL_DESTRUCT_LEVEL=2 valgrind --tool=massif --time-unit=B ./perl -Ilib -e '$x =qr/^_?[^\W_0-9]\w*\z/;'

From massif at around peak memory,


  n time(B) total(B) useful-heap(B) extra-heap(B) stacks(B)


27 922,464 824,256 812,102 12,154 0
98.53% (812,102B) (heap allocation functions) malloc/new/new[], --alloc-fns, etc
.
->80.33% (662,101B) 0x532212​: Perl_safesysrealloc (util.c​:173)
| ->79.98% (659,203B) 0x4F7407​: S_invlist_trim (regcomp.c​:7784)
| | ->78.89% (650,281B) 0x4F80D1​: Perl__invlist_union_maybe_complement_2nd (regc
omp.c​:8271)
| | | ->77.80% (641,288B) 0x4F8B12​: Perl__add_range_to_invlist (regcomp.c​:8581)
| | | | ->77.80% (641,288B) 0x4F8BF4​: S_add_cp_to_invlist (regcomp.c​:8621)
| | | | | ->77.80% (641,288B) 0x4D4807​: S_get_ANYOF_cp_list_for_ssc (regcomp.c​:1
047)
| | | | | ->77.80% (641,288B) 0x4D4D92​: S_ssc_or (regcomp.c​:1285)
| | | | | ->77.80% (641,288B) 0x4E80A0​: S_study_chunk (regcomp.c​:4652)
| | | | | ->77.80% (641,288B) 0x4F21A1​: Perl_re_op_compile (regcomp.c​:6701
)
| | | | | ->77.80% (641,288B) 0x42C6AD​: Perl_pmruntime (op.c​:4873)
| | | | | ->77.80% (641,288B) 0x4C5817​: Perl_yyparse (perly.y​:1353)
| | | | | ->77.80% (641,288B) 0x45603E​: S_parse_body (perl.c​:2274)
| | | | | ->77.80% (641,288B) 0x454666​: perl_parse (perl.c​:1586)
| | | | | ->77.80% (641,288B) 0x41CF24​: main (perlmain.c​:110)
| | | | |
| | | | ->00.00% (0B) in 1+ places, all below ms_print's threshold (01.00%)
| | | |
| | | ->01.09% (8,993B) 0x5142E2​: S_regclass (regcomp.c​:14538)
| | | | ->01.09% (8,993B) 0x5066D2​: S_regatom (regcomp.c​:10900)
| | | | ->01.09% (8,993B) 0x5029C2​: S_regpiece (regcomp.c​:10161)
| | | | ->01.09% (8,993B) 0x5022CF​: S_regbranch (regcomp.c​:10088)
| | | | ->01.09% (8,993B) 0x4FFF88​: S_reg (regcomp.c​:9837)
| | | | ->01.09% (8,993B) 0x4F1248​: Perl_re_op_compile (regcomp.c​:6488)
| | | | ->01.09% (8,993B) 0x42C6AD​: Perl_pmruntime (op.c​:4873)
| | | | ->01.09% (8,993B) 0x4C5817​: Perl_yyparse (perly.y​:1353)
| | | | ->01.09% (8,993B) 0x45603E​: S_parse_body (perl.c​:2274)
| | | | ->01.09% (8,993B) 0x454666​: perl_parse (perl.c​:1586)
| | | | ->01.09% (8,993B) 0x41CF24​: main (perlmain.c​:110)
| | | |
| | | ->00.00% (0B) in 1+ places, all below ms_print's threshold (01.00%)
| | |
| | ->01.08% (8,922B) 0x4F8837​: Perl__invlist_intersection_maybe_complement_2nd
(regcomp.c​:8513)
| | ->01.08% (8,897B) 0x4D47BF​: S_get_ANYOF_cp_list_for_ssc (regcomp.c​:1039)
| | | ->01.08% (8,897B) 0x4D4D92​: S_ssc_or (regcomp.c​:1285)
| | | ->01.08% (8,897B) 0x4E80A0​: S_study_chunk (regcomp.c​:4652)
| | | ->01.08% (8,897B) 0x4F21A1​: Perl_re_op_compile (regcomp.c​:6701)
| | | ->01.08% (8,897B) 0x42C6AD​: Perl_pmruntime (op.c​:4873)
| | | ->01.08% (8,897B) 0x4C5817​: Perl_yyparse (perly.y​:1353)
| | | ->01.08% (8,897B) 0x45603E​: S_parse_body (perl.c​:2274)
| | | ->01.08% (8,897B) 0x454666​: perl_parse (perl.c​:1586)
| | | ->01.08% (8,897B) 0x41CF24​: main (perlmain.c​:110)
| | |
| | ->00.00% (25B) in 1+ places, all below ms_print's threshold (01.00%)
| |
| ->00.35% (2,898B) in 1+ places, all below ms_print's threshold (01.00%)

without the ^_​:


  n time(B) total(B) useful-heap(B) extra-heap(B) stacks(B)


21 168,744 167,192 149,720 17,472 0
89.55% (149,720B) (heap allocation functions) malloc/new/new[], --alloc-fns, etc
.
->68.31% (114,201B) 0x5320F2​: Perl_safesysmalloc (util.c​:86)
| ->31.86% (53,264B) 0x592F44​: Perl_more_bodies (sv.c​:1059)
| | ->22.58% (37,744B) 0x593CFD​: Perl_sv_upgrade (sv.c​:1364)
| | | ->13.14% (21,968B) 0x5D1A4A​: Perl_newSV_type (sv.c​:9046)
| | | | ->02.43% (4,056B) 0x455712​: S_parse_body (perl.c​:2141)
| | | | | ->02.43% (4,056B) 0x454666​: perl_parse (perl.c​:1586)
| | | | | ->02.43% (4,056B) 0x41CF24​: main (perlmain.c​:110)
| | | | |
| | | | ->02.43% (4,056B) 0x43CE53​: Perl_newXS_len_flags (op.c​:8183)
| | | | | ->02.43% (4,056B) 0x43C956​: Perl_newXS_flags (op.c​:8131)
| | | | | ->02.43% (4,056B) 0x6CA1DC​: Perl_boot_core_UNIVERSAL (universal.c​:10

in bcb8752^ with the ^_​:


  n time(B) total(B) useful-heap(B) extra-heap(B) stacks(B)


30 121,728 121,024 112,742 8,282 0
31 128,136 127,432 119,142 8,290 0
32 132,672 131,280 122,923 8,357 0
33 136,760 135,368 127,003 8,365 0
34 140,840 139,448 131,059 8,389 0
35 145,968 142,128 133,327 8,801 0
93.81% (133,327B) (heap allocation functions) malloc/new/new[], --alloc-fns, etc.
->67.81% (96,381B) 0x532411​: Perl_safesysmalloc (util.c​:86)
| ->37.48% (53,264B) 0x593263​: Perl_more_bodies (sv.c​:1059)
| | ->26.56% (37,744B) 0x59401C​: Perl_sv_upgrade (sv.c​:1364)
| | | ->15.46% (21,968B) 0x5D1D69​: Perl_newSV_type (sv.c​:9046)
| | | | ->02.85% (4,056B) 0x455712​: S_parse_body (perl.c​:2141)
| | | | | ->02.85% (4,056B) 0x454666​: perl_parse (perl.c​:1586)
| | | | | ->02.85% (4,056B) 0x41CF24​: main (perlmain.c​:110)
| | | | |
| | | | ->02.85% (4,056B) 0x43CE53​: Perl_newXS_len_flags (op.c​:8183)
| | | | | ->02.85% (4,056B) 0x43C956​: Perl_newXS_flags (op.c​:8131)
| | | | | ->02.85% (4,056B) 0x6CA4FB​: Perl_boot_core_UNIVERSAL (universal.c​:1045)
| | | | | ->02.85% (4,056B) 0x4557A5​: S_parse_body (perl.c​:2149)
| | | | | ->02.85% (4,056B) 0x454666​: perl_parse (perl.c​:1586)
| | | | | ->02.85% (4,056B) 0x41CF24​: main (perlmain.c​:110)
| | | | |

Tony

@p5pRT
Copy link
Author

p5pRT commented Feb 3, 2016

From @khwilliamson

Thanks for reporting this problem.

This is not a specific issue for constant.pm, but is due to a change in the perl interpreter itself, and affects other modules besides constant.pm. The better long term solution is to change the interpreter, and leave constant.pm as-is. This will be done for 5.24.

If it is necessary to fix constant.pm to not use so much memory when loaded with earlier perl versions, there is a better patch in my opinion, which I can furnish if you desire, for your consideration.

--
Karl Williamson

@p5pRT
Copy link
Author

p5pRT commented Feb 3, 2016

From @atoomic

Thanks Karl for your answer.

I definitely agree fixing the problem as its root sounds better, and glad to hear the 5.24 will address this kind of concerns.
As a perl5.22 user, I try to reduce the memory bloat and would like to read your better patch ( or ideas / draft ) for constant.pm

Thanks in advance
nicolas

On Wed Feb 03 10​:07​:40 2016, khw wrote​:

Thanks for reporting this problem.

This is not a specific issue for constant.pm, but is due to a change
in the perl interpreter itself, and affects other modules besides
constant.pm. The better long term solution is to change the
interpreter, and leave constant.pm as-is. This will be done for 5.24.

If it is necessary to fix constant.pm to not use so much memory when
loaded with earlier perl versions, there is a better patch in my
opinion, which I can furnish if you desire, for your consideration.

@p5pRT
Copy link
Author

p5pRT commented Feb 3, 2016

From @khwilliamson

On 02/03/2016 11​:58 AM, Atoomic via RT wrote​:

Thanks Karl for your answer.

I definitely agree fixing the problem as its root sounds better, and glad to hear the 5.24 will address this kind of concerns.
As a perl5.22 user, I try to reduce the memory bloat and would like to read your better patch ( or ideas / draft ) for constant.pm

Thanks in advance
nicolas

The problem stems from our handling of Unicode characters, which before
had been deferred to runtime, but now is done at compile time, which
speeds up execution for those programs that need them, and slows down
compilation slightly. But compilation is only done once, and execution
with regex backtracking, can be done many many times. So it is a good
trade-off. However, the current method is memory inefficient, and can
be cleaned up significantly, which I'll do for 5.24 (God willing).

In the case of constant.pm, most times the constant will just be ASCII,
so doesn't need the generality for Unicode. The attached uses the same
pattern, but defers compilation until it is needed the first time. If
the constant name is all ASCII, the regex modifier /a can be used to
skip the Unicode handling, and not use so much memory. The patch
doesn't save any memory if Unicode characters are in the constant name,
but does otherwise. Your patch would save in all cases, but seems to me
to complicate the code unnecessarily, unless I'm wrong about how often
non-ASCII constants are done.

My patch needs an eval to enable the module to work on perls before when
/a was available. Those use the full match, but there wasn't excessive
memory use back then, so it isn't a problem. I did not test on anything
but blead.

Here are the results on my 64-bit system with DEBUGGING, and no
optimizations​:

Before attached patch​:
perl -e 'use utf8; use constant PI => 4 * atan2(1, 1); print qx{grep
VmRSS /proc/$$/status}'
  VmRSS​: 9292 kB

./perl -e 'use utf8; use constant π => 4 * atan2(1, 1); print qx{grep
VmRSS /proc/$$/status}'
VmRSS​: 9348 kB

After patch​:

perl -e 'use utf8; use constant PI => 4 * atan2(1, 1); print qx{grep
VmRSS /proc/$$/status}'
VmRSS​: 8104 kB

./perl -e 'use utf8; use constant π => 4 * atan2(1, 1); print qx{grep
VmRSS /proc/$$/status}'
VmRSS​: 9288 kB

Running it multiple times gives somewhat different numbers each time,
but in the same ball park.

On Wed Feb 03 10​:07​:40 2016, khw wrote​:

Thanks for reporting this problem.

This is not a specific issue for constant.pm, but is due to a change
in the perl interpreter itself, and affects other modules besides
constant.pm. The better long term solution is to change the
interpreter, and leave constant.pm as-is. This will be done for 5.24.

If it is necessary to fix constant.pm to not use so much memory when
loaded with earlier perl versions, there is a better patch in my
opinion, which I can furnish if you desire, for your consideration.

---
via perlbug​: queue​: perl5 status​: open
https://rt-archive.perl.org/perl5/Ticket/Display.html?id=127392

@p5pRT
Copy link
Author

p5pRT commented Feb 3, 2016

From @khwilliamson

0007-constant.pm-lower-memory-use.patch
From e14f3a24ed48118d85f3c6b0236b74dc0f199a43 Mon Sep 17 00:00:00 2001
From: Karl Williamson <khw@cpan.org>
Date: Wed, 3 Feb 2016 13:41:11 -0700
Subject: [PATCH 7/7] constant.pm lower memory use

---
 dist/constant/lib/constant.pm | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/dist/constant/lib/constant.pm b/dist/constant/lib/constant.pm
index e4b8fd2..6a691cc 100644
--- a/dist/constant/lib/constant.pm
+++ b/dist/constant/lib/constant.pm
@@ -17,7 +17,7 @@ my %forced_into_main = map +($_, 1),
 
 my %forbidden = (%keywords, %forced_into_main);
 
-my $normal_constant_name = qr/^_?[^\W_0-9]\w*\z/;
+my $uncompiled_normal_constant_name = '^_?[^\W_0-9]\w*\z';
 my $tolerable = qr/^[A-Za-z_]\w*\z/;
 my $boolean = qr/^[01]?\z/;
 
@@ -95,8 +95,15 @@ sub import {
 	    $pkg = $caller;
 	}
 
-	# Normal constant name
-	if ($name =~ $normal_constant_name and !$forbidden{$name}) {
+	# Normal constant name.  Perl 5.20 and 5.22 consumed too much memory
+        # when compiling '$uncompiled_normal_constant_name'.  But this can
+        # mostly be avoided by using /a, which we can do if the constant name
+        # is ASCII-only.  (/a is available starting in 5.14.)
+	if (($] lt 5.014 || $name =~ /[[:^ascii:]]/)
+             ? $name =~ /$uncompiled_normal_constant_name/
+             : eval "$name =~ /$uncompiled_normal_constant_name/a"
+            and !$forbidden{$name})
+        {
 	    # Everything is okay
 
 	# Name forced into main, but we're not in main. Fatal.
-- 
2.1.4

@p5pRT
Copy link
Author

p5pRT commented Feb 8, 2016

From @ap

* Karl Williamson <public@​khwilliamson.com> [2016-02-03 21​:55]​:

+ if (($] lt 5.014 || $name =~ /[[​:^ascii​:]]/)
+ ? $name =~ /$uncompiled_normal_constant_name/
+ : eval "$name =~ /$uncompiled_normal_constant_name/a"

That cannot possibly be correct. The eval code string interpolates the
variable values into the code rather than refer to them by name. I think
you wanted single quotes around the code string?

N.B.​: as long as $name is a valid identifier that’s not defined in the
constant package, I think it might even parse “correctly” as a bareword
string. And the other variable is also likely to work when interpolated.
So this code might work a lot of the time… by accident. Fascinating…

@p5pRT
Copy link
Author

p5pRT commented Feb 8, 2016

From @khwilliamson

On 02/07/2016 09​:09 PM, Aristotle Pagaltzis wrote​:

* Karl Williamson <public@​khwilliamson.com> [2016-02-03 21​:55]​:

+ if (($] lt 5.014 || $name =~ /[[​:^ascii​:]]/)
+ ? $name =~ /$uncompiled_normal_constant_name/
+ : eval "$name =~ /$uncompiled_normal_constant_name/a"

That cannot possibly be correct. The eval code string interpolates the
variable values into the code rather than refer to them by name. I think
you wanted single quotes around the code string?

N.B.​: as long as $name is a valid identifier that’s not defined in the
constant package, I think it might even parse “correctly” as a bareword
string. And the other variable is also likely to work when interpolated.
So this code might work a lot of the time… by accident. Fascinating…

Well, I tested it before I publicized it, and it worked. But you're
right that $name needs to be escaped, and single quotes does it.

@p5pRT
Copy link
Author

p5pRT commented Feb 10, 2016

From @atoomic

Thanks for the suggestion that also works, here is the metrics I have

no patch> perl522 -Ilib -e 'use utf8; use constant PI => 4 * atan2(1, 1); print qx{grep VmRSS /proc/$$/status}'
VmRSS​: 3068 kB
with patch from e14f3a24ed4> perl522 -Ilib -e 'use utf8; use constant PI => 4 * atan2(1, 1); print qx{grep VmRSS /proc/$$/status}'
VmRSS​: 2488 kB
with patch from d212aef5d7f> perl522 -Ilib -e 'use utf8; use constant PI => 4 * atan2(1, 1); print qx{grep VmRSS /proc/$$/status}'
VmRSS​: 2280 kB

@p5pRT
Copy link
Author

p5pRT commented Feb 18, 2016

From @toddr

Created by @toddr

We have discovered a discrepancy with perl 5.22 when non-ascii regexes
are used. The difference is so significant, it looks almost like a
memory leak.

The program

my @​re;
push @​re, qr{(?​:\.|(?​:[-\w\$\d\/*]|\\[0-2]\d\d)+(?​:\.(?​:[-\w\$\d\/]|\\[0-2]\d\d)+)*\.?)};
push @​re, qr{(?​:\.|(?​:[-\w\$\d\/*]|\\[0-2]\d\d)+(?​:\.(?​:[-\w\$\d\/]|\\[0-2]\d\d)+)*\.?)};
push @​re, qr{(?​:\.|(?​:[-\w\$\d\/*]|\\[0-2]\d\d)+(?​:\.(?​:[-\w\$\d\/]|\\[0-2]\d\d)+)*\.?)};
push @​re, qr{(?​:\.|(?​:[-\w\$\d\/*]|\\[0-2]\d\d)+(?​:\.(?​:[-\w\$\d\/]|\\[0-2]\d\d)+)*\.?)};
push @​re, qr{(?​:\.|(?​:[-\w\$\d\/*]|\\[0-2]\d\d)+(?​:\.(?​:[-\w\$\d\/]|\\[0-2]\d\d)+)*\.?)};

map { "foo" =~ $_ } @​re;
print qx{grep RSS /proc/$$/status};

When I run this, I get​:

$>p14 foo.pl
VmRSS​: 1536 kB

$>p22 foo.pl
VmRSS​: 5316 kB

NOW, add an "a" modifier to the end of the regexes and you get​:

$>p14 foo.pl
VmRSS​: 1540 kB

$>p22 foo.pl
VmRSS​: 1852 kB

If I only add an "a" modifier to some of the regexes, the number I get
is in-between so it's cumulative.

Perl Info

Flags:
    category=core
    severity=medium

Site configuration information for perl 5.22.1:

Configured by cPanel at Thu Feb 18 13:34:29 CST 2016.

Summary of my perl5 (revision 5 version 22 subversion 1) configuration:

  Platform:
    osname=linux, osvers=2.6.32-431.29.2.el6.i686, archname=i386-linux-64int
    uname='linux rpmb-32-centos-65.dev.cpanel.net
2.6.32-431.29.2.el6.i686 #1 smp tue sep 9 20:14:52 utc 2014 i686 i686
i386 gnulinux '
    config_args='-des -Dusedevel -Darchname=i386-linux-64int
-Dcc=/usr/bin/gcc -Dcpp=/usr/bin/cpp -DDEBUGGING=none -Doptimize=-Os
-Dusemymalloc=n -Duseshrplib -Duselargefiles=yes -Duseposix=true
-Dhint=recommended -Duseperlio=yes -Dccflags=-DPERL_DISABLE_PMC
-I/usr/local/cpanel/3rdparty/perl/522/include
-L/usr/local/cpanel/3rdparty/perl/522/lib
-I/usr/local/cpanel/3rdparty/include -L/usr/local/cpanel/3rdparty/lib
-Dcppflags=-I/usr/local/cpanel/3rdparty/perl/522/include
-L/usr/local/cpanel/3rdparty/perl/522/lib
-I/usr/local/cpanel/3rdparty/include -L/usr/local/cpanel/3rdparty/lib
-Dldflags=-Wl,-rpath -Wl,/usr/local/cpanel/3rdparty/perl/522/lib
-L/usr/local/cpanel/3rdparty/perl/522/lib
-L/usr/local/cpanel/3rdparty/lib
-Dprefix=/usr/local/cpanel/3rdparty/perl/522
-Dsiteprefix=/opt/cpanel/perl5/522 -Dsitebin=/opt/cpanel/perl5/522/bin
-Dsitelib=/opt/cpanel/perl5/522/site_lib -Dusevendorprefix=true
-Dvendorbin=/usr/local/cpanel/3rdparty/perl/522/bin
-Dvendorprefix=/usr/local/cpanel/3rdparty/perl/522/lib/perl5
-Dvendorlib=/usr/local/cpanel/3rdparty/perl/522/lib/perl5/cpanel_lib
-Dprivlib=/usr/local/cpanel/3rdparty/perl/522/lib/perl5/5.22.1
-Dman1dir=none -Dman3dir=none
-Dscriptdir=/usr/local/cpanel/3rdparty/perl/522/bin
-Dscriptdirexp=/usr/local/cpanel/3rdparty/perl/522/bin
-Dsiteman1dir=none -Dsiteman3dir=none -Dinstallman1dir=none
-Dversiononly=no -Dinstallusrbinperl=no -Dcf_by=cPanel
-Dmyhostname=localhost -Dperladmin=root@localhost
-Dcf_email=support@cpanel.net
-Di_dbm=/usr/local/cpanel/3rdparty/include
-Di_gdbm=/usr/local/cpanel/3rdparty/include
-Di_ndbm=/usr/local/cpanel/3rdparty/include -DDB_File=true -Ud_dosuid
-Uuserelocatableinc -Umad -Uusethreads -Uusemultiplicity -Uusesocks
-Uuselongdouble -Aldflags=-L/usr/local/cpanel/3rdparty/perl/522/lib
-L/usr/local/cpanel/3rdparty/lib -L/usr/lib -L/lib -lgdbm
-Dlocincpth=/usr/local/cpanel/3rdparty/perl/522/include
/usr/local/cpanel/3rdparty/include /usr/local/include  -Duse64bitint
-Uuse64bitall -Acflags=-fPIC -DPIC -m32
-I/usr/local/cpanel/3rdparty/perl/522/include
-I/usr/local/cpanel/3rdparty/include
-Dlibpth=/usr/local/cpanel/3rdparty/perl/522/lib
/usr/local/cpanel/3rdparty/lib /usr/local/lib /lib /usr/lib '
    hint=recommended, useposix=true, d_sigaction=define
    useithreads=undef, usemultiplicity=undef
    use64bitint=define, use64bitall=undef, uselongdouble=undef
    usemymalloc=n, bincompat5005=undef
  Compiler:
    cc='/usr/bin/gcc', ccflags ='-DPERL_DISABLE_PMC
-I/usr/local/cpanel/3rdparty/perl/522/include
-L/usr/local/cpanel/3rdparty/perl/522/lib
-I/usr/local/cpanel/3rdparty/include -L/usr/local/cpanel/3rdparty/lib
-fwrapv -fno-strict-aliasing -pipe -fstack-protector
-I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64
-D_FORTIFY_SOURCE=2',
    optimize='-Os',
    cppflags='-I/usr/local/cpanel/3rdparty/perl/522/include
-L/usr/local/cpanel/3rdparty/perl/522/lib
-I/usr/local/cpanel/3rdparty/include -L/usr/local/cpanel/3rdparty/lib
-DPERL_DISABLE_PMC -I/usr/local/cpanel/3rdparty/perl/522/include
-L/usr/local/cpanel/3rdparty/perl/522/lib
-I/usr/local/cpanel/3rdparty/include -L/usr/local/cpanel/3rdparty/lib
-fwrapv -fno-strict-aliasing -pipe -fstack-protector
-I/usr/local/include'
    ccversion='', gccversion='4.4.7 20120313 (Red Hat 4.4.7-4)', gccosandvers=''
    intsize=4, longsize=4, ptrsize=4, doublesize=8,
byteorder=12345678, doublekind=3
    d_longlong=define, longlongsize=8, d_longdbl=define,
longdblsize=12, longdblkind=3
    ivtype='long long', ivsize=8, nvtype='double', nvsize=8,
Off_t='off_t', lseeksize=8
    alignbytes=4, prototype=define

  Linker and Libraries:
    ld='/usr/bin/gcc', ldflags ='-Wl,-rpath
-Wl,/usr/local/cpanel/3rdparty/perl/522/lib
-L/usr/local/cpanel/3rdparty/perl/522/lib
-L/usr/local/cpanel/3rdparty/lib
-L/usr/local/cpanel/3rdparty/perl/522/lib
-L/usr/local/cpanel/3rdparty/lib -L/usr/lib -L/lib -lgdbm
-fstack-protector -L/usr/local/lib'
    libpth=/usr/local/cpanel/3rdparty/perl/522/lib
/usr/local/cpanel/3rdparty/lib /usr/local/lib /lib /usr/lib
/usr/local/lib /usr/lib
    libs=-lpthread -lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lc
    perllibs=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc
    libc=libc-2.12.so, so=so, useshrplib=true, libperl=libperl.so
    gnulibc_version='2.12'

  Dynamic Linking:
    dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E
-Wl,-rpath,/usr/local/cpanel/3rdparty/perl/522/lib/perl5/5.22.1/i386-linux-64int/CORE'
    cccdlflags='-fPIC', lddlflags='-shared -Os
-L/usr/local/cpanel/3rdparty/perl/522/lib
-L/usr/local/cpanel/3rdparty/lib -L/usr/lib -L/lib -L/usr/local/lib
-fstack-protector'

Locally applied patches:
    cPanel patches
    cPanel INC path changes
    Remove . from @INC


@INC for perl 5.22.1:
    /usr/local/cpanel
    /usr/local/cpanel/3rdparty/perl/522/lib/perl5/cpanel_lib/i386-linux-64int
    /usr/local/cpanel/3rdparty/perl/522/lib/perl5/cpanel_lib
    /usr/local/cpanel/3rdparty/perl/522/lib/perl5/5.22.1/i386-linux-64int
    /usr/local/cpanel/3rdparty/perl/522/lib/perl5/5.22.1
    /opt/cpanel/perl5/522/site_lib/i386-linux-64int
    /opt/cpanel/perl5/522/site_lib



Environment for perl 5.22.1:
    HOME=/root
    LANG=en_US.UTF-8
    LANGUAGE (unset)
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=/usr/local/cpanel/bin:/usr/local/cpanel/3rdparty/bin:/usr/local/cpanel/3rdparty/perl/522/bin:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin:/opt/cpanel/perl5/514/bin
    PERL_BADLANG (unset)
    SHELL=/bin/zsh

@p5pRT
Copy link
Author

p5pRT commented Feb 18, 2016

From @tonycoz

On Thu Feb 18 15​:24​:26 2016, TODDR wrote​:

We have discovered a discrepancy with perl 5.22 when non-ascii regexes
are used. The difference is so significant, it looks almost like a
memory leak.

The program

my @​re;
push @​re, qr{(?​:\.|(?​:[-\w\$\d\/*]|\\[0-2]\d\d)+(?​:\.(?​:[-
\w\$\d\/]|\\[0-2]\d\d)+)*\.?)};
push @​re, qr{(?​:\.|(?​:[-\w\$\d\/*]|\\[0-2]\d\d)+(?​:\.(?​:[-
\w\$\d\/]|\\[0-2]\d\d)+)*\.?)};
push @​re, qr{(?​:\.|(?​:[-\w\$\d\/*]|\\[0-2]\d\d)+(?​:\.(?​:[-
\w\$\d\/]|\\[0-2]\d\d)+)*\.?)};
push @​re, qr{(?​:\.|(?​:[-\w\$\d\/*]|\\[0-2]\d\d)+(?​:\.(?​:[-
\w\$\d\/]|\\[0-2]\d\d)+)*\.?)};
push @​re, qr{(?​:\.|(?​:[-\w\$\d\/*]|\\[0-2]\d\d)+(?​:\.(?​:[-
\w\$\d\/]|\\[0-2]\d\d)+)*\.?)};

map { "foo" =~ $_ } @​re;
print qx{grep RSS /proc/$$/status};

When I run this, I get​:

$>p14 foo.pl
VmRSS​: 1536 kB

$>p22 foo.pl
VmRSS​: 5316 kB

NOW, add an "a" modifier to the end of the regexes and you get​:

$>p14 foo.pl
VmRSS​: 1540 kB

$>p22 foo.pl
VmRSS​: 1852 kB

If I only add an "a" modifier to some of the regexes, the number I get
is in-between so it's cumulative.

This looks like https://rt-archive.perl.org/perl5/Ticket/Display.html?id=127392

@p5pRT
Copy link
Author

p5pRT commented Feb 18, 2016

The RT System itself - Status changed from 'new' to 'open'

@p5pRT
Copy link
Author

p5pRT commented Feb 24, 2016

From @khwilliamson

The two tickets for excess memory use have now been consolidated.

blead now contains a fix to lower peak memory use. I would appreciate it if the requestors would try blead out on their respective problems and report back. As the commit says (accb436) on my box,

For [perl #127392], peak was cut from around 8600kB to 7800kB.
For [perl #127568], the savings are about 37%

@p5pRT
Copy link
Author

p5pRT commented Feb 24, 2016

From @atoomic

Excellent ! I did some basic tests for both tickets
I can notice a good improvement for constant (ticket=127392) but a smaller one for \w as shown below
anyway this looks good, but would still consider a partial patch for constant.pm as this can then save more

** with patch accb436
# perl -E 'my $f = $ENV{user} =~ qr{_?[\W\_]}; print qx{grep VmRSS /proc/$$/status}'
VmRSS​: 2120 kB
# perl -e 'use constant; print qx{grep VmRSS /proc/$$/status}'
VmRSS​: 2484 kB

** without patch (head=accb436^)
# perl -E 'my $f = $ENV{user} =~ qr{_?[\W\_]}; print qx{grep VmRSS /proc/$$/status}'
VmRSS​: 2260 kB
# perl -e 'use constant; print qx{grep VmRSS /proc/$$/status}'
VmRSS​: 2884 kB

On Wed Feb 24 09​:32​:13 2016, khw wrote​:

The two tickets for excess memory use have now been consolidated.

blead now contains a fix to lower peak memory use. I would appreciate
it if the requestors would try blead out on their respective problems
and report back. As the commit says
(accb436) on my box,

For [perl #127392], peak was cut from around 8600kB to 7800kB.
For [perl #127568], the savings are about 37%

@p5pRT
Copy link
Author

p5pRT commented Feb 28, 2016

From @khwilliamson

blead now contains some further reductions in memory use. Could you try it now, please

--
Karl Williamson

@p5pRT
Copy link
Author

p5pRT commented Feb 28, 2016

From @toddr

On Sat Feb 27 17​:18​:56 2016, khw wrote​:

blead now contains some further reductions in memory use. Could you
try it now, please

Karl,

Atoomic and I are backporting and testing these to 5.22.1 actually. Can you provide the commits so we can test them please? If doing so is in any way dangerous, please let us know.

@p5pRT
Copy link
Author

p5pRT commented Feb 28, 2016

From @jkeenan

On Sat Feb 27 17​:18​:56 2016, khw wrote​:

blead now contains some further reductions in memory use. Could you
try it now, please

See data from various perl versions, including blead, attached.

--
James E Keenan (jkeenan@​cpan.org)

@p5pRT
Copy link
Author

p5pRT commented Feb 28, 2016

From @jkeenan

$ perlbrew use perl-5.10.1;perl -e 'use utf8; use constant PI => 4 * atan2(1, 1); print qx{grep VmRSS /proc/$$/status}'
VmRSS​: 3056 kB

$ perlbrew use perl-5.14.4;perl -e 'use utf8; use constant PI => 4 * atan2(1, 1); print qx{grep VmRSS /proc/$$/status}'
VmRSS​: 2288 kB

$ perlbrew use perl-5.16.3;perl -e 'use utf8; use constant PI => 4 * atan2(1, 1); print qx{grep VmRSS /proc/$$/status}'
VmRSS​: 4264 kB

$ perlbrew use perl-5.18.4;perl -e 'use utf8; use constant PI => 4 * atan2(1, 1); print qx{grep VmRSS /proc/$$/status}'
VmRSS​: 2356 kB

$ perlbrew use perl-5.20.1;perl -e 'use utf8; use constant PI => 4 * atan2(1, 1); print qx{grep VmRSS /proc/$$/status}'
VmRSS​: 3256 kB

$ perlbrew use perl-5.22.0;perl -e 'use utf8; use constant PI => 4 * atan2(1, 1); print qx{grep VmRSS /proc/$$/status}'
VmRSS​: 3472 kB

$ ./perl -v | head -2 | tail -1
This is perl 5, version 23, subversion 9 (v5.23.9 (v5.23.8-29-g6c0a9fb)) built for x86_64-linux

$./perl -Ilib -Mutf8 -e 'use constant PI =&gt; 4 * atan2(1, 1); print qx{grep VmRSS /proc/$$/status}'
VmRSS​: 2660 kB

@p5pRT
Copy link
Author

p5pRT commented Feb 28, 2016

From @khwilliamson

On 02/27/2016 07​:15 PM, Todd Rinaldo via RT wrote​:

On Sat Feb 27 17​:18​:56 2016, khw wrote​:

blead now contains some further reductions in memory use. Could you
try it now, please

Karl,

Atoomic and I are backporting and testing these to 5.22.1 actually. Can you provide the commits so we can test them please? If doing so is in any way dangerous, please let us know.

---
via perlbug​: queue​: perl5 status​: open
https://rt-archive.perl.org/perl5/Ticket/Display.html?id=127392

I don't guarantee that these will cleanly apply, but take all patches in
blead that change regcomp.c starting with

f0c0c5a

and ending with

6c0a9fb

You need not take (but it's trivial)​:
a7629b1

@p5pRT
Copy link
Author

p5pRT commented Feb 29, 2016

From @atoomic

here is the list to consider, some minor changes to proto.h are required
will give a headup with some results later

* 6c0a9fb regcomp.c​: Use less peak memory (2 days ago, Karl Williamson)
* ff26755 regcomp.c​: Modify an assert (2 days ago, Karl Williamson)
* e2506fa regcomp.c​: Avoid an unnecessary mortal SV (2 days ago, Karl Williamson)
* 3001ef3 regcomp.c​: Improve free-ing up unused inversion list space (2 days ago, Karl Williamson)
* e7d603c regcomp.c​: Add new static inline convenience function (2 days ago, Karl Williamson)
* acb2202 regcomp.c​: Change variable names, white-space (2 days ago, Karl Williamson)
* 96a7241 regcomp.c​: Change name of static function (2 days ago, Karl Williamson)
* 2263ed2 perlfunc overhaul (2 days ago, Lukas Mai)
[not needed] * a7629b1 regcomp.c​: Add missing 'STATIC' (3 days ago, Karl Williamson)
* b75fc44 Fix regex failures on big-endian systems (3 days ago, Karl Williamson)
* a975eeb perlvar​: hyperlink from $^D to perlrun/-D (5 days ago, Lukas Mai)
* d25f767 podcheck.t​: expand POD sequences in L</...> section names (5 days ago, Lukas Mai)
* 69202fa Fix perldoc -f sleep to use $SIG{ALRM} instead of $SIG{ALARM}. (5 days ago, Matthew Horsfall)
* accb436 Use less memory in compiling regexes (5 days ago, Karl Williamson)

On Sat Feb 27 21​:57​:05 2016, public@​khwilliamson.com wrote​:

On 02/27/2016 07​:15 PM, Todd Rinaldo via RT wrote​:

On Sat Feb 27 17​:18​:56 2016, khw wrote​:

blead now contains some further reductions in memory use. Could you
try it now, please

Karl,

Atoomic and I are backporting and testing these to 5.22.1 actually.
Can you provide the commits so we can test them please? If doing so
is in any way dangerous, please let us know.

---
via perlbug​: queue​: perl5 status​: open
https://rt-archive.perl.org/perl5/Ticket/Display.html?id=127392

I don't guarantee that these will cleanly apply, but take all patches
in
blead that change regcomp.c starting with

f0c0c5a

and ending with

6c0a9fb

You need not take (but it's trivial)​:
a7629b1

@p5pRT
Copy link
Author

p5pRT commented Feb 29, 2016

From @atoomic

From the tests I made, only accb436 seems to reduce the memory for the two tickets

here are the one liner used​:
#127568​: \w> perl -e 'my $f = $ENV{user} =~ qr{_?[\W\_]}; print qx{egrep "VmRSS|VmPeak" /proc/$$/status}'
#127392​: constant​: PI> perl -e 'use utf8; use constant PI => 4 * atan2(1, 1); print qx{egrep "VmRSS|VmPeak" /proc/$$/status}'
#127392​: constant only> perl -e 'use constant; print qx{egrep "VmRSS|VmPeak" /proc/$$/status}'

* with HEAD=e1ad4fb Improve wording of perldelta entry for ae146f5

#127568​: \w
VmPeak​: 7680 kB / -8% ( compared to reference = accb436^ )
VmRSS​: 1848 kB / -27%
#127392​: constant​: PI
VmPeak​: 7948 kB / -5%
VmRSS​: 2532 kB / -14%
#127392​: constant only
VmPeak​: 7944 kB / -5%
VmRSS​: 2480 kB / -14%

* with HEAD=accb436

#127568​: \w
VmPeak​: 7676 kB / -8%
VmRSS​: 1864 kB / -26%
#127392​: constant​: PI
VmPeak​: 7944 kB / -5%
VmRSS​: 2532 kB / -14%
#127392​: constant only
VmPeak​: 7940 kB / -5%
VmRSS​: 2484 kB / -14%

* with HEAD=accb436^=f0c0c5a

#127568​: \w
VmPeak​: 8340 kB
VmRSS​: 2528 kB
#127392​: constant​: PI
VmPeak​: 8340 kB
VmRSS​: 2928 kB
#127392​: constant only
VmPeak​: 8336 kB
VmRSS​: 2884 kB

On Sat Feb 27 21​:57​:05 2016, public@​khwilliamson.com wrote​:

On 02/27/2016 07​:15 PM, Todd Rinaldo via RT wrote​:

On Sat Feb 27 17​:18​:56 2016, khw wrote​:

blead now contains some further reductions in memory use. Could you
try it now, please

Karl,

Atoomic and I are backporting and testing these to 5.22.1 actually.
Can you provide the commits so we can test them please? If doing so
is in any way dangerous, please let us know.

---
via perlbug​: queue​: perl5 status​: open
https://rt-archive.perl.org/perl5/Ticket/Display.html?id=127392

I don't guarantee that these will cleanly apply, but take all patches
in
blead that change regcomp.c starting with

f0c0c5a

and ending with

6c0a9fb

You need not take (but it's trivial)​:
a7629b1

@p5pRT
Copy link
Author

p5pRT commented Mar 2, 2016

From @atoomic

Thanks Karl for your patches, I've backported most of them to 5.22 (only require some minor adjustment in proto.h)
and this works pretty fine.

Backport on top of 5.22.1 are here​:
https://github.com/atoomic/perl5/tree/perl522.1-memory

if we are interested to add them to a next 5.22.* release

It took me a few minutes to understand that commit af72888 was fixing a segfault when running​:
./perl -I.. -e 'my $p = qr{(?d​:[^\x{00E0}]?)}i'

af72888 regcomp.c​: Improve free-ing up unused inversion list spac

Here is a nice sample to prove that we are now saving the memory and avoid the leak
before> perl -e 'my $f = $ENV{user} =~ qr{_?[\W\_]}; my $fa = $ENV{user} =~ qr{_?[\W\_]}; my $fb = $ENV{user} =~ qr{_?[\W\_]}; my $fc = $ENV{user} =~ qr{_?[\W\_]}; my $fd = $ENV{user} =~ qr{_?[\W\_]}; my $fe = $ENV{user} =~ qr{_?[\W\_]}; my $ff = $ENV{user} =~ qr{_?[\W\_]}; my $fg = $ENV{user} =~ qr{_?[\W\_]}; my $fh = $ENV{user} =~ qr{_?[\W\_]}; print qx{egrep "VmRSS|VmPeak" /proc/$$/status}'
VmPeak​: 13520 kB
VmRSS​: 7980 kB

after> perl -e 'my $f = $ENV{user} =~ qr{_?[\W\_]}; my $fa = $ENV{user} =~ qr{_?[\W\_]}; my $fb = $ENV{user} =~ qr{_?[\W\_]}; my $fc = $ENV{user} =~ qr{_?[\W\_]}; my $fd = $ENV{user} =~ qr{_?[\W\_]}; my $fe = $ENV{user} =~ qr{_?[\W\_]}; my $ff = $ENV{user} =~ qr{_?[\W\_]}; my $fg = $ENV{user} =~ qr{_?[\W\_]}; my $fh = $ENV{user} =~ qr{_?[\W\_]}; print qx{egrep "VmRSS|VmPeak" /proc/$$/status}'
VmPeak​: 7644 kB
VmRSS​: 2032 kB

On Mon Feb 29 08​:49​:35 2016, atoomic wrote​:

From the tests I made, only accb436 seems to reduce the memory for the
two tickets

here are the one liner used​:
#127568​: \w> perl -e 'my $f = $ENV{user} =~ qr{_?[\W\_]}; print
qx{egrep "VmRSS|VmPeak" /proc/$$/status}'
#127392​: constant​: PI> perl -e 'use utf8; use constant PI => 4 *
atan2(1, 1); print qx{egrep "VmRSS|VmPeak" /proc/$$/status}'
#127392​: constant only> perl -e 'use constant; print qx{egrep
"VmRSS|VmPeak" /proc/$$/status}'

* with HEAD=e1ad4fb Improve wording of perldelta entry for ae146f5

#127568​: \w
VmPeak​: 7680 kB / -8% ( compared to reference = accb436^ )
VmRSS​: 1848 kB / -27%
#127392​: constant​: PI
VmPeak​: 7948 kB / -5%
VmRSS​: 2532 kB / -14%
#127392​: constant only
VmPeak​: 7944 kB / -5%
VmRSS​: 2480 kB / -14%

* with HEAD=accb436

#127568​: \w
VmPeak​: 7676 kB / -8%
VmRSS​: 1864 kB / -26%
#127392​: constant​: PI
VmPeak​: 7944 kB / -5%
VmRSS​: 2532 kB / -14%
#127392​: constant only
VmPeak​: 7940 kB / -5%
VmRSS​: 2484 kB / -14%

* with HEAD=accb436^=f0c0c5a

#127568​: \w
VmPeak​: 8340 kB
VmRSS​: 2528 kB
#127392​: constant​: PI
VmPeak​: 8340 kB
VmRSS​: 2928 kB
#127392​: constant only
VmPeak​: 8336 kB
VmRSS​: 2884 kB

On Sat Feb 27 21​:57​:05 2016, public@​khwilliamson.com wrote​:

On 02/27/2016 07​:15 PM, Todd Rinaldo via RT wrote​:

On Sat Feb 27 17​:18​:56 2016, khw wrote​:

blead now contains some further reductions in memory use. Could
you
try it now, please

Karl,

Atoomic and I are backporting and testing these to 5.22.1
actually.
Can you provide the commits so we can test them please? If doing so
is in any way dangerous, please let us know.

---
via perlbug​: queue​: perl5 status​: open
https://rt-archive.perl.org/perl5/Ticket/Display.html?id=127392

I don't guarantee that these will cleanly apply, but take all patches
in
blead that change regcomp.c starting with

f0c0c5a

and ending with

6c0a9fb

You need not take (but it's trivial)​:
a7629b1

@p5pRT
Copy link
Author

p5pRT commented Mar 2, 2016

From @khwilliamson

On 03/02/2016 09​:48 AM, Atoomic via RT wrote​:

Thanks Karl for your patches, I've backported most of them to 5.22 (only require some minor adjustment in proto.h)
and this works pretty fine.

These newer numbers look a lot better than the 5% savings earlier given.
  Is it just because of it being a different test? Can this ticket be
closed?

Backport on top of 5.22.1 are here​:
https://github.com/atoomic/perl5/tree/perl522.1-memory

if we are interested to add them to a next 5.22.* release

It took me a few minutes to understand that commit af72888 was fixing a segfault when running​:
./perl -I.. -e 'my $p = qr{(?d​:[^\x{00E0}]?)}i'

I did not know that. I just saw that the code didn't look right. I'll
add this to the tests, though I'm somewhat surprised it isn't there
already. Maybe this was failing under your particular configuration
that we don't have in our smokes.

af72888 regcomp.c​: Improve free-ing up unused inversion list spac

Here is a nice sample to prove that we are now saving the memory and avoid the leak
before> perl -e 'my $f = $ENV{user} =~ qr{_?[\W\_]}; my $fa = $ENV{user} =~ qr{_?[\W\_]}; my $fb = $ENV{user} =~ qr{_?[\W\_]}; my $fc = $ENV{user} =~ qr{_?[\W\_]}; my $fd = $ENV{user} =~ qr{_?[\W\_]}; my $fe = $ENV{user} =~ qr{_?[\W\_]}; my $ff = $ENV{user} =~ qr{_?[\W\_]}; my $fg = $ENV{user} =~ qr{_?[\W\_]}; my $fh = $ENV{user} =~ qr{_?[\W\_]}; print qx{egrep "VmRSS|VmPeak" /proc/$$/status}'
VmPeak​: 13520 kB
VmRSS​: 7980 kB

after> perl -e 'my $f = $ENV{user} =~ qr{_?[\W\_]}; my $fa = $ENV{user} =~ qr{_?[\W\_]}; my $fb = $ENV{user} =~ qr{_?[\W\_]}; my $fc = $ENV{user} =~ qr{_?[\W\_]}; my $fd = $ENV{user} =~ qr{_?[\W\_]}; my $fe = $ENV{user} =~ qr{_?[\W\_]}; my $ff = $ENV{user} =~ qr{_?[\W\_]}; my $fg = $ENV{user} =~ qr{_?[\W\_]}; my $fh = $ENV{user} =~ qr{_?[\W\_]}; print qx{egrep "VmRSS|VmPeak" /proc/$$/status}'
VmPeak​: 7644 kB
VmRSS​: 2032 kB

On Mon Feb 29 08​:49​:35 2016, atoomic wrote​:

From the tests I made, only accb436 seems to reduce the memory for the
two tickets

here are the one liner used​:
#127568​: \w> perl -e 'my $f = $ENV{user} =~ qr{_?[\W\_]}; print
qx{egrep "VmRSS|VmPeak" /proc/$$/status}'
#127392​: constant​: PI> perl -e 'use utf8; use constant PI => 4 *
atan2(1, 1); print qx{egrep "VmRSS|VmPeak" /proc/$$/status}'
#127392​: constant only> perl -e 'use constant; print qx{egrep
"VmRSS|VmPeak" /proc/$$/status}'

* with HEAD=e1ad4fb Improve wording of perldelta entry for ae146f5

#127568​: \w
VmPeak​: 7680 kB / -8% ( compared to reference = accb436^ )
VmRSS​: 1848 kB / -27%
#127392​: constant​: PI
VmPeak​: 7948 kB / -5%
VmRSS​: 2532 kB / -14%
#127392​: constant only
VmPeak​: 7944 kB / -5%
VmRSS​: 2480 kB / -14%

* with HEAD=accb436

#127568​: \w
VmPeak​: 7676 kB / -8%
VmRSS​: 1864 kB / -26%
#127392​: constant​: PI
VmPeak​: 7944 kB / -5%
VmRSS​: 2532 kB / -14%
#127392​: constant only
VmPeak​: 7940 kB / -5%
VmRSS​: 2484 kB / -14%

* with HEAD=accb436^=f0c0c5a

#127568​: \w
VmPeak​: 8340 kB
VmRSS​: 2528 kB
#127392​: constant​: PI
VmPeak​: 8340 kB
VmRSS​: 2928 kB
#127392​: constant only
VmPeak​: 8336 kB
VmRSS​: 2884 kB

On Sat Feb 27 21​:57​:05 2016, public@​khwilliamson.com wrote​:

On 02/27/2016 07​:15 PM, Todd Rinaldo via RT wrote​:

On Sat Feb 27 17​:18​:56 2016, khw wrote​:

blead now contains some further reductions in memory use. Could
you
try it now, please

Karl,

Atoomic and I are backporting and testing these to 5.22.1
actually.
Can you provide the commits so we can test them please? If doing so
is in any way dangerous, please let us know.

---
via perlbug​: queue​: perl5 status​: open
https://rt-archive.perl.org/perl5/Ticket/Display.html?id=127392

I don't guarantee that these will cleanly apply, but take all patches
in
blead that change regcomp.c starting with

f0c0c5a

and ending with

6c0a9fb

You need not take (but it's trivial)​:
a7629b1

---
via perlbug​: queue​: perl5 status​: open
https://rt-archive.perl.org/perl5/Ticket/Display.html?id=127392

@p5pRT
Copy link
Author

p5pRT commented Mar 2, 2016

From @atoomic

no need to add some tests the segfault test I provided is already part of the testsuite and indeed this was only triggered using some configure options
otherwise it was not triggered

we can close this ticket
thanks
nicolas

On Wed Mar 02 09​:55​:08 2016, public@​khwilliamson.com wrote​:

On 03/02/2016 09​:48 AM, Atoomic via RT wrote​:

Thanks Karl for your patches, I've backported most of them to 5.22
(only require some minor adjustment in proto.h)
and this works pretty fine.

These newer numbers look a lot better than the 5% savings earlier
given.
Is it just because of it being a different test? Can this ticket be
closed?

Backport on top of 5.22.1 are here​:
https://github.com/atoomic/perl5/tree/perl522.1-memory

if we are interested to add them to a next 5.22.* release

It took me a few minutes to understand that commit af72888 was fixing
a segfault when running​:
./perl -I.. -e 'my $p = qr{(?d​:[^\x{00E0}]?)}i'

I did not know that. I just saw that the code didn't look right.
I'll
add this to the tests, though I'm somewhat surprised it isn't there
already. Maybe this was failing under your particular configuration
that we don't have in our smokes.

af72888 regcomp.c​: Improve free-ing up unused inversion list spac

Here is a nice sample to prove that we are now saving the memory and
avoid the leak
before> perl -e 'my $f = $ENV{user} =~ qr{_?[\W\_]}; my $fa =
before> $ENV{user} =~ qr{_?[\W\_]}; my $fb = $ENV{user} =~
before> qr{_?[\W\_]}; my $fc = $ENV{user} =~ qr{_?[\W\_]}; my $fd =
before> $ENV{user} =~ qr{_?[\W\_]}; my $fe = $ENV{user} =~
before> qr{_?[\W\_]}; my $ff = $ENV{user} =~ qr{_?[\W\_]}; my $fg =
before> $ENV{user} =~ qr{_?[\W\_]}; my $fh = $ENV{user} =~
before> qr{_?[\W\_]}; print qx{egrep "VmRSS|VmPeak" /proc/$$/status}'
VmPeak​: 13520 kB
VmRSS​: 7980 kB

after> perl -e 'my $f = $ENV{user} =~ qr{_?[\W\_]}; my $fa =
after> $ENV{user} =~ qr{_?[\W\_]}; my $fb = $ENV{user} =~
after> qr{_?[\W\_]}; my $fc = $ENV{user} =~ qr{_?[\W\_]}; my $fd =
after> $ENV{user} =~ qr{_?[\W\_]}; my $fe = $ENV{user} =~
after> qr{_?[\W\_]}; my $ff = $ENV{user} =~ qr{_?[\W\_]}; my $fg =
after> $ENV{user} =~ qr{_?[\W\_]}; my $fh = $ENV{user} =~
after> qr{_?[\W\_]}; print qx{egrep "VmRSS|VmPeak" /proc/$$/status}'
VmPeak​: 7644 kB
VmRSS​: 2032 kB

On Mon Feb 29 08​:49​:35 2016, atoomic wrote​:

From the tests I made, only accb436 seems to reduce the memory for
the
two tickets

here are the one liner used​:
#127568​: \w> perl -e 'my $f = $ENV{user} =~ qr{_?[\W\_]}; print
qx{egrep "VmRSS|VmPeak" /proc/$$/status}'
#127392​: constant​: PI> perl -e 'use utf8; use constant PI => 4 *
atan2(1, 1); print qx{egrep "VmRSS|VmPeak" /proc/$$/status}'
#127392​: constant only> perl -e 'use constant; print qx{egrep
"VmRSS|VmPeak" /proc/$$/status}'

* with HEAD=e1ad4fb Improve wording of perldelta entry for
ae146f5

#127568​: \w
VmPeak​: 7680 kB / -8% ( compared to reference = accb436^ )
VmRSS​: 1848 kB / -27%
#127392​: constant​: PI
VmPeak​: 7948 kB / -5%
VmRSS​: 2532 kB / -14%
#127392​: constant only
VmPeak​: 7944 kB / -5%
VmRSS​: 2480 kB / -14%

* with HEAD=accb436

#127568​: \w
VmPeak​: 7676 kB / -8%
VmRSS​: 1864 kB / -26%
#127392​: constant​: PI
VmPeak​: 7944 kB / -5%
VmRSS​: 2532 kB / -14%
#127392​: constant only
VmPeak​: 7940 kB / -5%
VmRSS​: 2484 kB / -14%

* with HEAD=accb436^=f0c0c5a

#127568​: \w
VmPeak​: 8340 kB
VmRSS​: 2528 kB
#127392​: constant​: PI
VmPeak​: 8340 kB
VmRSS​: 2928 kB
#127392​: constant only
VmPeak​: 8336 kB
VmRSS​: 2884 kB

On Sat Feb 27 21​:57​:05 2016, public@​khwilliamson.com wrote​:

On 02/27/2016 07​:15 PM, Todd Rinaldo via RT wrote​:

On Sat Feb 27 17​:18​:56 2016, khw wrote​:

blead now contains some further reductions in memory use. Could
you
try it now, please

Karl,

Atoomic and I are backporting and testing these to 5.22.1
actually.
Can you provide the commits so we can test them please? If doing
so
is in any way dangerous, please let us know.

---
via perlbug​: queue​: perl5 status​: open
https://rt-archive.perl.org/perl5/Ticket/Display.html?id=127392

I don't guarantee that these will cleanly apply, but take all
patches
in
blead that change regcomp.c starting with

f0c0c5a

and ending with

6c0a9fb

You need not take (but it's trivial)​:
a7629b1

---
via perlbug​: queue​: perl5 status​: open
https://rt-archive.perl.org/perl5/Ticket/Display.html?id=127392

@p5pRT
Copy link
Author

p5pRT commented Mar 2, 2016

From @khwilliamson

On 03/02/2016 01​:56 PM, Atoomic via RT wrote​:

we can close this ticket
thanks
nicolas

It occurred to me afterwards that there was desire to patch constant.pm
to not use so much memory in perls before 5.24, and that p5p is upstream
for that dual-life module.

So what should be done, and by whom?

@p5pRT
Copy link
Author

p5pRT commented Mar 2, 2016

From @toddr

On Wed Mar 02 15​:05​:08 2016, public@​khwilliamson.com wrote​:

It occurred to me afterwards that there was desire to patch constant.pm
to not use so much memory in perls before 5.24, and that p5p is upstream
for that dual-life module.

So what should be done, and by whom?

I'm inclined to say the bug was in Perl not core so patching constant is inappropriate. It's been broken since 5.16 and nobody has noticed till now right?

As for whether it should be officially backported to 11.52, I'm I suspect backport criteria would exclude these patches unless it really was a memory leak? At this point I'm unclear if it was just unnecessary use of memory that was later freed or if it actually is a memory leak.

@p5pRT
Copy link
Author

p5pRT commented Mar 2, 2016

From @khwilliamson

On 03/02/2016 04​:08 PM, Todd Rinaldo via RT wrote​:

At this point I'm unclear if it was just unnecessary use of memory that was later freed or if it actually is a memory leak.

It was not a memory leak.

@p5pRT
Copy link
Author

p5pRT commented Mar 2, 2016

From @toddr

On Wed Mar 02 15​:08​:54 2016, TODDR wrote​:

I'm inclined to say the bug was in Perl not core so patching constant

Apologies I meant. Perl not constant

@p5pRT
Copy link
Author

p5pRT commented Mar 22, 2016

From @khwilliamson

Closed with the approval of the OP
--
Karl Williamson

@p5pRT
Copy link
Author

p5pRT commented Mar 22, 2016

@khwilliamson - Status changed from 'open' to 'pending release'

@p5pRT
Copy link
Author

p5pRT commented May 13, 2016

From @khwilliamson

Thank you for submitting this report. You have helped make Perl better.
 
With the release of Perl 5.24.0 on May 9, 2016, this and 149 other issues have been resolved.

Perl 5.24.0 may be downloaded via https://metacpan.org/release/RJBS/perl-5.24.0

@p5pRT
Copy link
Author

p5pRT commented May 13, 2016

@khwilliamson - Status changed from 'pending release' to 'resolved'

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

No branches or pull requests

1 participant