search

Perl / perl5

🐪 The Perl programming language
https://dev.perl.org/perl5/
Other
1.91k stars 542 forks source link

extend test.valgrind to also run linux's perf stat #11643

Closed p5pRT closed 12 years ago

p5pRT commented 13 years ago

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

Searchable as RT98788$

p5pRT commented 13 years ago

From @jimc

Created by @jimc

forthcoming patch alters make test.valgrind target and t/TEST so that it can also run linux perf stat on all the perl tests.

With it\, and following env\, make test.valgrind works​:

  VALGRIND=perf   VG_TEST=--help   VG_OPTS='stat -- '

$> make test.valgrind; PERL_VALGRIND=1 VALGRIND=perf ./runtests choose t/base/cond....................................................ok t/base/if......................................................ok t/base/lex.....................................................ok ...

[jimc@​groucho perl]$ cat t/base/*.perf-stat

Performance counter stats for './perl base/cond.t'​:

  5.882071 task-clock # 0.850 CPUs utilized   1 context-switches # 0.000 M/sec   1 CPU-migrations # 0.000 M/sec   483 page-faults # 0.082 M/sec   4\,688\,843 cycles # 0.797 GHz   \ stalled-cycles-frontend   \ stalled-cycles-backend   3\,368\,118 instructions # 0.72 insns per cycle   718\,821 branches # 122.205 M/sec   48\,053 branch-misses # 6.68% of all branches

  0.006920536 seconds time elapsed

Perl Info ``` Flags: category=core severity=low This perlbug was built using Perl 5.12.3 in the Fedora build system. It is being executed now by Perl 5.15.2 - Wed Sep 7 12:17:39 MDT 2011. Site configuration information for perl 5.15.2: Configured by jimc at Wed Sep 7 12:17:39 MDT 2011. Summary of my perl5 (revision 5 version 15 subversion 2) configuration: Commit id: d39de89300b9384bad8b2cf88917ce9f104ae8b2 Platform: osname=linux, osvers=2.6.35.14-95.fc14.x86_64, archname=x86_64-linux uname='linux groucho.jimc.earth 2.6.35.14-95.fc14.x86_64 #1 smp tue aug 16 21:01:58 utc 2011 x86_64 x86_64 x86_64 gnulinux ' config_args='-des -Dusedevel -DDEBUGGING=both' hint=recommended, useposix=true, d_sigaction=define useithreads=undef, usemultiplicity=undef useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef use64bitint=define, use64bitall=define, uselongdouble=undef usemymalloc=n, bincompat5005=undef Compiler: cc='cc', ccflags ='-DDEBUGGING -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64', optimize='-O2 -g', cppflags='-DDEBUGGING -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include' ccversion='', gccversion='4.5.1 20100924 (Red Hat 4.5.1-4)', gccosandvers='' intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678 d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16 ivtype='long', ivsize=8, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8 alignbytes=8, prototype=define Linker and Libraries: ld='cc', ldflags =' -fstack-protector -L/usr/local/lib' libpth=/usr/local/lib /lib/../lib64 /usr/lib/../lib64 /lib /usr/lib /lib64 /usr/lib64 /usr/local/lib64 libs=-lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lc -lgdbm_compat perllibs=-lnsl -ldl -lm -lcrypt -lutil -lc libc=/lib/libc-2.13.so, so=so, useshrplib=false, libperl=libperl.a gnulibc_version='2.13' Dynamic Linking: dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E' cccdlflags='-fPIC', lddlflags='-shared -O2 -g -L/usr/local/lib -fstack-protector' Locally applied patches: @INC for perl 5.15.2: lib /usr/local/lib/perl5/site_perl/5.15.2/x86_64-linux /usr/local/lib/perl5/site_perl/5.15.2 /usr/local/lib/perl5/5.15.2/x86_64-linux /usr/local/lib/perl5/5.15.2 /usr/local/lib/perl5/site_perl . Environment for perl 5.15.2: HOME=/home/jimc LANG=en_US.utf8 LANGUAGE (unset) LD_LIBRARY_PATH=/usr/lib64/mpich2/lib:/usr/lib64/alliance/lib:/usr/lib64/alliance/lib LOGDIR (unset) PATH=/usr/lib64/qt-3.3/bin:/usr/lib64/mpich2/bin:/usr/lib64/ccache:/usr/local/bin:/usr/bin:/bin:/usr/local/sbin:/usr/sbin:/sbin:/usr/lib64/alli ance/bin:/usr/libexec/sdcc:/home/jimc/bin:./bin:.:/usr/lib64/alliance/bin:/usr/libexec/sdcc PERL_BADLANG (unset) SHELL=/bin/bash ```
p5pRT commented 13 years ago

From @jimc

heres the patch.

p5pRT commented 13 years ago

From @jimc

0001-adjust-TEST-s-valgrind-invocation-to-also-work-with-.patch ```diff From 3467a0fff1b9927ab25b1b49c6d2cab61c141f25 Mon Sep 17 00:00:00 2001 From: Jim Cromie Date: Sat, 12 Mar 2011 17:17:49 -0700 Subject: [PATCH] adjust TEST's valgrind invocation to also work with linux's perf stat Move --log-fd=3 option from unconditional invocation into VG_OPTS default value. A future version of perf will understand --log-fd=3, but other tools probably will not, with this we can accommodate them, and the current version of perf. Makefile.SH: Set VALGRIND var conditionally, to allow cmdline override (this is probably non-portable, will need review at least). perl.valgrind.config target's test of $(VALGRIND) is simplified to use $(VG_TEST), which defaults to its legacy value: ./perl -e 1 2>/dev/null. Setting it to '--help' is needed for perf, and would also work to verify that valgrind is runnable, but current test is slightly more comprehensive for valgrind, so Ive left that for user to change in the environment. t/TEST: 1. --log-fd=3 is in default, but can be overridden by setting VG_OPTS 2. several variable renames to clarify purpose 3. $toolnm to rename output file with flexible suffix, ie: valgrind, cachegrind, perf-stat 4. add perf to cachegrind as a special case, avoid culling of valgrind output files by their content With above, and following env, make test.valgrind works: # --log-fd isnt mainline yet. VALGRIND=/home/jimc/projects/lx/linux-2.6/tools/perf/perf VG_TEST=--help VG_OPTS='stat --log-fd=3 -- ' $> make test.valgrind; PERL_VALGRIND=1 VALGRIND='/home/jimc/projects/lx/linux-2.6/tools/perf/perf' ./runtests choose t/base/cond....................................................ok t/base/if......................................................ok t/base/lex.....................................................ok ... [jimc@groucho perl]$ cat t/base/*.perf-stat Performance counter stats for './perl base/cond.t': 5.882071 task-clock # 0.850 CPUs utilized 1 context-switches # 0.000 M/sec 1 CPU-migrations # 0.000 M/sec 483 page-faults # 0.082 M/sec 4,688,843 cycles # 0.797 GHz stalled-cycles-frontend stalled-cycles-backend 3,368,118 instructions # 0.72 insns per cycle 718,821 branches # 122.205 M/sec 48,053 branch-misses # 6.68% of all branches 0.006920536 seconds time elapsed This patch will allow you to use released version of perf, just drop the --log-fd from VG_OPTS. The tests will fail, because perf will write to STDOUT, and foul the harness. The following runs cachegrind, creates t/*/*.cachegrind files. It is much slower than using perf-stat. $> export VG_OPTS='--tool=cachegrind --log-fd=3 -- ' $> make test.valgrind ==25822== Cachegrind, a cache and branch-prediction profiler ==25822== Copyright (C) 2002-2009, and GNU GPL'd, by Nicholas Nethercote et al. ==25822== Using Valgrind-3.5.0 and LibVEX; rerun with -h for copyright info ==25822== Command: ./perl base/cond.t ==25822== ==25822== ==25822== I refs: 1,680,072 ==25822== I1 misses: 8,129 ==25822== L2i misses: 3,675 ==25822== I1 miss rate: 0.48% ==25822== L2i miss rate: 0.21% ==25822== ==25822== D refs: 604,393 (400,033 rd + 204,360 wr) ==25822== D1 misses: 12,599 ( 8,838 rd + 3,761 wr) ==25822== L2d misses: 6,261 ( 2,966 rd + 3,295 wr) ==25822== D1 miss rate: 2.0% ( 2.2% + 1.8% ) ==25822== L2d miss rate: 1.0% ( 0.7% + 1.6% ) ==25822== ==25822== L2 refs: 20,728 ( 16,967 rd + 3,761 wr) ==25822== L2 misses: 9,936 ( 6,641 rd + 3,295 wr) ==25822== L2 miss rate: 0.4% ( 0.3% + 1.6% ) NB: The following almost works; t runs the 1st test 5 times, and produces 1 statistics file, but it fails, because TEST sees multiple leaders, (FAILED--seen duplicate leader) and exits immediately, because it happens in t/base. A work-around is easy enough, but adds yet another knob. TBD. $> VALGRIND=perf VG_OPTS='stat -r5 --log-fd=3 --' make test.valgrind Performance counter stats for './perl base/cond.t' (5 runs): 5.568965 task-clock # 0.833 CPUs utilized ( +- 1.82% ) 0 context-switches # 0.000 M/sec ( +- 61.24% ) 0 CPU-migrations # 0.000 M/sec ( +-100.00% ) 478 page-faults # 0.086 M/sec ( +- 0.37% ) 4,441,737 cycles # 0.798 GHz ( +- 1.84% ) stalled-cycles-frontend stalled-cycles-backend 3,183,574 instructions # 0.72 insns per cycle ( +- 2.30% ) 669,241 branches # 120.173 M/sec ( +- 2.87% ) 41,826 branch-misses # 6.25% of all branches ( +- 3.78% ) 0.006688160 seconds time elapsed ( +- 1.49% ) This patch is really a proof-of-concept; perf tool has far more capabilities than t/TEST can exploit well, but this is a start, and makes perf foo experimentation easier. --- Makefile.SH | 5 +++-- t/TEST | 36 ++++++++++++++++++++++-------------- 2 files changed, 25 insertions(+), 16 deletions(-) diff --git a/Makefile.SH b/Makefile.SH index 18ef6aa..7c8b545 100755 --- a/Makefile.SH +++ b/Makefile.SH @@ -333,7 +333,8 @@ $make_set_make # If you're going to use valgrind and it can't be invoked as plain valgrind # then you'll need to change this, or override it on the make command line. -VALGRIND=valgrind +VALGRIND ?= valgrind +VG_TEST ?= ./perl -e 1 2>/dev/null DTRACE = $dtrace DTRACE_H = $dtrace_h @@ -887,7 +888,7 @@ perl.valgrind.config: config.sh @grep "^usemymalloc=" config.sh @grep "^usemymalloc='n'" config.sh >/dev/null || exit 1 @echo "And of course you have to have valgrind..." - $(VALGRIND) ./perl -e 1 2>/dev/null || exit 1 + $(VALGRIND) $(VG_TEST) || exit 1 # Third Degree Perl (Tru64 only) diff --git a/t/TEST b/t/TEST index 0c24c17..f80a627 100755 --- a/t/TEST +++ b/t/TEST @@ -290,12 +290,13 @@ sub _cmd { if ($ENV{PERL_VALGRIND}) { my $perl_supp = $options->{return_dir} ? "$options->{return_dir}/perl.supp" : "perl.supp"; - my $valgrind = $ENV{VALGRIND} // 'valgrind'; + my $valgrind_exe = $ENV{VALGRIND} // 'valgrind'; my $vg_opts = $ENV{VG_OPTS} - // "--suppressions=$perl_supp --leak-check=yes " - . "--leak-resolution=high --show-reachable=yes " + // '--log-fd=3 ' + . "--suppressions=$perl_supp --leak-check=yes " + . "--leak-resolution=high --show-reachable=yes " . "--num-callers=50 --track-origins=yes"; - $perl = "$valgrind --log-fd=3 $vg_opts $perl"; + $perl = "$valgrind_exe $vg_opts $perl"; $redir = "3>$Valgrind_Log"; if ($options->{run_dir}) { $Valgrind_Log = "$options->{run_dir}/$Valgrind_Log"; @@ -305,7 +306,6 @@ sub _cmd { my $args = "$options->{testswitch} $options->{switch} $options->{utf8}"; $cmd = $perl . _quote_args($args) . " $test $redir"; } - return $cmd; } @@ -515,7 +515,7 @@ EOT } # + 3 : we want three dots between the test name and the "ok" my $dotdotdot = $maxlen + 3 ; - my $valgrind = 0; + my $grind_ct = 0; # count of non-empty valgrind reports my $total_files = @tests; my $good_files = 0; my $tested_files = 0; @@ -647,7 +647,9 @@ EOT } if ($ENV{PERL_VALGRIND}) { - my @valgrind; + my $toolnm = $ENV{VALGRIND}; + $toolnm =~ s|.*/||; # keep basename + my @valgrind; # gets content of file if (-e $Valgrind_Log) { if (open(V, $Valgrind_Log)) { @valgrind = ; @@ -656,11 +658,17 @@ EOT warn "$0: Failed to open '$Valgrind_Log': $!\n"; } } - if ($ENV{VG_OPTS} =~ /cachegrind/) { - if (rename $Valgrind_Log, "$test.valgrind") { - $valgrind = $valgrind + 1; + if ($ENV{VG_OPTS} =~ /(cachegrind)/ or $toolnm =~ /(perf)/) { + $toolnm = $1; + if ($toolnm eq 'perf') { + # append perfs subcommand, not just stat + my ($sub) = split /\s/, $ENV{VG_OPTS}; + $toolnm .= "-$sub"; + } + if (rename $Valgrind_Log, "$test.$toolnm") { + $grind_ct++; } else { - warn "$0: Failed to create '$test.valgrind': $!\n"; + warn "$0: Failed to create '$test.$toolnm': $!\n"; } } elsif (@valgrind) { @@ -681,7 +689,7 @@ EOT } if ($errors or $leaks) { if (rename $Valgrind_Log, "$test.valgrind") { - $valgrind = $valgrind + 1; + $grind_ct = $grind_ct + 1; } else { warn "$0: Failed to create '$test.valgrind': $!\n"; } @@ -798,8 +806,8 @@ SHRDLU_5 print sprintf("u=%.2f s=%.2f cu=%.2f cs=%.2f scripts=%d tests=%d\n", $user,$sys,$cuser,$csys,$tested_files,$totmax); if ($ENV{PERL_VALGRIND}) { - my $s = $valgrind == 1 ? '' : 's'; - print "$valgrind valgrind report$s created.\n", ; + my $s = $grind_ct == 1 ? '' : 's'; + print "$grind_ct valgrind report$s created.\n", ; } } exit ($::bad_files != 0); -- 1.7.4.4 ```
p5pRT commented 13 years ago

From @cpansprout

On Fri Sep 09 12​:50​:37 2011\, yoduh wrote​:

heres the patch.

Thank you. Applied as c7b956bbbaff0c4616c7bf80a5b723a58b55bbf6.

p5pRT commented 13 years ago

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

p5pRT commented 13 years ago

@cpansprout - Status changed from 'open' to 'resolved'

p5pRT commented 13 years ago

From @jimc

On Fri\, Sep 9\, 2011 at 10​:42 PM\, Father Chrysostomos via RT \perlbug\-followup@​perl\.org wrote​:

On Fri Sep 09 12​:50​:37 2011\, yoduh wrote​:

heres the patch.

Thank you.  Applied as c7b956bbbaff0c4616c7bf80a5b723a58b55bbf6.

Thank you.

Does this mean that ?= is portable to all unix make exes ?

  Set VALGRIND var conditionally\, to allow cmdline override (this is   probably non-portable\, will need review at least).

--- a/Makefile.SH +++ b/Makefile.SH @​@​ -333\,7 +333\,8 @​@​ $make_set_make

# If you're going to use valgrind and it can't be invoked as plain valgrind # then you'll need to change this\, or override it on the make command line. -VALGRIND=valgrind +VALGRIND ?= valgrind +VG_TEST ?= ./perl -e 1 2>/dev/null

Apologies for not noting this more LOUDLY\, ie in the email too.

p5pRT commented 13 years ago

From @cpansprout

On Fri Sep 09 23​:37​:26 2011\, yoduh wrote​:

On Fri\, Sep 9\, 2011 at 10​:42 PM\, Father Chrysostomos via RT \perlbug\-followup@​perl\.org wrote​:

On Fri Sep 09 12​:50​:37 2011\, yoduh wrote​:

heres the patch.

Thank you. �Applied as c7b956bbbaff0c4616c7bf80a5b723a58b55bbf6.

Thank you.

Does this mean that ?= is portable to all unix make exes ?

Er\, I *hope* so. It passed all tests for me. I didn’t think about it. We’ll see by tomorrow whether it causes smoke.

(I probably should have pushed this to a smoke-me branch first.)

Set VALGRIND var conditionally\, to allow cmdline override \(this is
probably non\-portable\, will need review at least\)\.

--- a/Makefile.SH +++ b/Makefile.SH @​@​ -333\,7 +333\,8 @​@​ $make_set_make

# If you're going to use valgrind and it can't be invoked as plain valgrind # then you'll need to change this\, or override it on the make command line. -VALGRIND=valgrind +VALGRIND ?= valgrind +VG_TEST ?= ./perl -e 1 2>/dev/null

Apologies for not noting this more LOUDLY\, ie in the email too.

p5pRT commented 13 years ago

From @jimc

re-using this ticket\, since its the same topic. hope thats fine.

1. adds .gitignore entries for *.valgrind\, *.perf-stat\, *.cachegrind files 2. adds prose to perlhack.pod where test.valgrind target is discussed 3. cleans up cachegrind.out.$pid intermediate files not cleaned up by valgrind.

Ive considered creating test.perf and test.cachegrind targets; - easier invocation\, w/o fiddling w environment vars - test.valgrind needs -g\, test.perf would not.

If this makes sense\, perhaps patch 2 should wait.

p5pRT commented 13 years ago

From @jimc

0001-add-3-test.valgrind-outputs-to-.gitignore.patch ```diff From 86895a37a5c07da672c5df0c68f7a53604df1ea3 Mon Sep 17 00:00:00 2001 From: Jim Cromie Date: Sat, 10 Sep 2011 19:16:18 -0600 Subject: [PATCH 1/3] add 3 test.valgrind outputs to .gitignore --- .gitignore | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-) diff --git a/.gitignore b/.gitignore index deb3c8d..80b2a8b 100644 --- a/.gitignore +++ b/.gitignore @@ -137,3 +137,8 @@ MANIFEST.new *.swp *~ .#* + +# test.valgrind final outputs, excluding intermediate files (pls report) +*.cachegrind +*.perf-stat +*.valgrind -- 1.7.4.4 ```
p5pRT commented 13 years ago

From @jimc

0002-pod-perlhack.pod-describe-test.valgrind-support-for-.patch ```diff From 4e888c38e394ab1714e7ac2821e78afffef7afa1 Mon Sep 17 00:00:00 2001 From: Jim Cromie Date: Sun, 11 Sep 2011 01:19:55 -0600 Subject: [PATCH 2/3] pod/perlhack.pod: describe test.valgrind support for linux perf tool add paragraph and example invocation describing use of Linux perf with make test.valgrind, via environmental vars. test.perf should probably get its own make rule for ease of use, but mostly because test.valgrind dependency (perl.valgrind.config) requires -g, which perf itself doesnt need. If so, this patch should wait. --- pod/perlhack.pod | 11 ++++++++++- 1 files changed, 10 insertions(+), 1 deletions(-) diff --git a/pod/perlhack.pod b/pod/perlhack.pod index 60e2b31..1656fb4 100644 --- a/pod/perlhack.pod +++ b/pod/perlhack.pod @@ -768,7 +768,16 @@ F, F and F tests. (Only in Linux) Run all the tests using the memory leak + naughty memory access tool "valgrind". The log files will be named -F. +F or F. + + make test.valgrind + VG_OPTS='--tool=cachegrind --log-fd=3' make test.valgrind + +This target also supports use of Linux's perf tool, and writes log +files named F, or for other perf subcommads, +F. + + VALGRIND=perf VG_TEST=--help VG_OPTS='stat --log-fd=3 --' make test.valgrind =item * test.torture torturetest -- 1.7.4.4 ```
p5pRT commented 13 years ago

From @jimc

0003-t-TEST-clean-up-cachegrind.out.-pid-intermediate-fil.patch ```diff From 502202a972543ed1f33f50a5545445831edaa3bb Mon Sep 17 00:00:00 2001 From: Jim Cromie Date: Sun, 11 Sep 2011 23:32:50 -0600 Subject: [PATCH 3/3] t/TEST: clean up cachegrind.out.$pid intermediate files running cachegrind leaves lots of intermediate files, delete them at the end. Killing make test leaves them around, but this may be useful for some debugging purposes. Rework _find_tests($dir) into _find_files($patt,$dir) and wrapper, to support existing uses and new one. --- t/TEST | 37 ++++++++++++++++++++++++------------- 1 files changed, 24 insertions(+), 13 deletions(-) diff --git a/t/TEST b/t/TEST index 0a354ba..a57261c 100755 --- a/t/TEST +++ b/t/TEST @@ -171,20 +171,24 @@ my %skip = ( ); # Roll your own File::Find! -sub _find_tests { - my($dir) = @_; - opendir DIR, $dir or die "Trouble opening $dir: $!"; - foreach my $f (sort { $a cmp $b } readdir DIR) { - next if $skip{$f}; - - my $fullpath = "$dir/$f"; - - if (-d $fullpath) { - _find_tests($fullpath); - } elsif ($f =~ /\.t$/) { - push @ARGV, $fullpath; +sub _find_tests { our @found=(); push @ARGV, _find_files('\.t$', $_[0]) } +sub _find_files { + my($patt, @dirs) = @_; + for my $dir (@dirs) { + opendir DIR, $dir or die "Trouble opening $dir: $!"; + foreach my $f (sort { $a cmp $b } readdir DIR) { + next if $skip{$f}; + + my $fullpath = "$dir/$f"; + + if (-d $fullpath) { + _find_files($patt, $fullpath); + } elsif ($f =~ /$patt/) { + push @found, $fullpath; + } } } + @found; } @@ -522,6 +526,7 @@ EOT my $tested_files = 0; my $totmax = 0; my %failed_tests; + my $toolnm; # valgrind, cachegrind, perf while (my $test = shift @tests) { my ($test_start_time, @starttimes) = 0; @@ -658,7 +663,7 @@ EOT } if ($ENV{PERL_VALGRIND}) { - my $toolnm = $ENV{VALGRIND}; + $toolnm = $ENV{VALGRIND}; $toolnm =~ s|.*/||; # keep basename my @valgrind; # gets content of file if (-e $Valgrind_Log) { @@ -842,6 +847,12 @@ SHRDLU_5 if ($ENV{PERL_VALGRIND}) { my $s = $grind_ct == 1 ? '' : 's'; print "$grind_ct valgrind report$s created.\n", ; + if ($toolnm eq 'cachegrind') { + # cachegrind leaves a lot of cachegrind.out.$pid litter + # around the tree, find and delete them + unlink _find_files('cachegrind.out.\d+$', + qw ( ../t ../cpan ../ext ../dist/ )); + } } } exit ($::bad_files != 0); -- 1.7.4.4 ```
p5pRT commented 13 years ago

From @cpansprout

On Sun Sep 11 22​:54​:15 2011\, yoduh wrote​:

re-using this ticket\, since its the same topic. hope thats fine.

1. adds .gitignore entries for *.valgrind\, *.perf-stat\, *.cachegrind files 2. adds prose to perlhack.pod where test.valgrind target is discussed 3. cleans up cachegrind.out.$pid intermediate files not cleaned up by valgrind.

Ive considered creating test.perf and test.cachegrind targets; - easier invocation\, w/o fiddling w environment vars - test.valgrind needs -g\, test.perf would not.

If this makes sense\, perhaps patch 2 should wait.

That does make sense. I’ll wait.

I’ve applied 1 and 3 as 87dfd78cc5 and c96083ea\, respectively. Thank you.

p5pRT commented 13 years ago

@cpansprout - Status changed from 'resolved' to 'open'

p5pRT commented 13 years ago

From @jimc

Ive considered creating test.perf and test.cachegrind targets; - easier invocation\, w/o fiddling w environment vars - test.valgrind needs -g\, test.perf would not.

If this makes sense\, perhaps patch 2 should wait.

That does make sense.  I’ll wait.

I’ve applied 1 and 3 as 87dfd78cc5 and c96083ea\, respectively.  Thank you.

Ok\, heres those 2 new make targets.

a few new questions​:

1- all the VALGRIND-ish envars are now used more generally\, perhaps they need a more general name. Suggestions / bikeshedding ?

PERL_DUT - (mnemonic - device under test (or really tool)) replaces PERL_VALGRIND\, which is just a flag (in t/TEST\, t/test.pl) since the tests run under some tool\, this seems to capture it.

PERL_DUTOOL - name/path of tool used when PERL_DUT is true.

VG_TEST - can go away\, hardcoded in each target. test.perf doesnt actually run that test anymore\, since I dropped its dependency on perl.valgrind.config

2- linux-perf tool version.

released perf doesnt understand --log-fd\, so patch above doesnt use it But it can\, if you have newer version\, and add it to VG_OPTS. Which future tense should I document for perlhack ?

3- PERL_DUT_DIR

Id like to add a new envar\, as a destination for all the *.{valgrind\,cachegrind\,perf-stat} files\, probably including a dated subdir to archive them automatically when set. Id also put the Storable file written if -d $ENV{HARNESS_TIMER} Sensible ? good name ?

p5pRT commented 13 years ago

From @jimc

0001-Makefile.SH-add-test.cachegrind-test.perf-targets.patch ```diff From 4febbe2c26caa79f6651c3f1206394d982edd2e9 Mon Sep 17 00:00:00 2001 From: Jim Cromie Date: Mon, 12 Sep 2011 11:47:08 -0600 Subject: [PATCH] Makefile.SH: add test.cachegrind test.perf targets To simplify use of valgrind's cachegrind and linux's perf tool, this gives them their own targets in the makefiles. It also eliminates test.perf's need for a -g build, formerly inherited from reusing the test.valgrind target with environmental overrides. make test.cachegrind does: PERL_VALGRIND=1 VALGRIND='valgrind' VG_OPTS='--tool=cachegrind --log-fd=3' ./runtests choose t/base/cond ................................................... ok t/base/if ..................................................... ok t/base/lex .................................................... ok t/base/num .................................................... ok t/base/pat .................................................... ok .... make test.perf does: PERL_VALGRIND=1 VALGRIND=perf VG_TEST=--help VG_OPTS='stat --' ./runtests choose t/base/cond ................................................... Performance counter stats for './perl base/cond.t': 5.877139 task-clock-msecs # 0.825 CPUs 0 context-switches # 0.000 M/sec 0 CPU-migrations # 0.000 M/sec 484 page-faults # 0.082 M/sec 4645893 cycles # 790.502 M/sec (scaled from 66.14%) 2225660 instructions # 0.479 IPC 584940 branches # 99.528 M/sec 46492 branch-misses # 7.948 % 1344890 cache-references # 228.834 M/sec (scaled from 44.84%) 43667 cache-misses # 7.430 M/sec (scaled from 27.92%) 0.007124343 seconds time elapsed ok t/base/if ..................................................... Current released version of perf does not understand --log-fd option, so I've left it out of the make rule, hence the extra output above. Tests still pass though. Once its in mainline (or perhaps in distros), I'll add --log-fd=3 into the rule. In meantime, if you want to get --log-fd support, its here: now: git://github.com/jimc/linux-2.6.git perf/core branch soon: git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git perf/core mainline: linux-v3.2 ? To build it, cd to tools/perf and make (its not built by top-level make) make install puts it in ~/bin etc. --- Makefile.SH | 12 ++++++++++++ 1 files changed, 12 insertions(+), 0 deletions(-) diff --git a/Makefile.SH b/Makefile.SH index 7c8b545..0a7ffce 100755 --- a/Makefile.SH +++ b/Makefile.SH @@ -1444,6 +1444,18 @@ utest.valgrind ucheck.valgrind: test_prep.valgrind perl.valgrind.config test_notty.valgrind: test_prep.valgrind perl.valgrind.config PERL_VALGRIND=1 $(RUN_TESTS) no-tty +test.cachegrind check.valgrind: test_prep perl.valgrind.config + PERL_VALGRIND=1 VALGRIND='$(VALGRIND)' VG_OPTS='--tool=cachegrind --log-fd=3' $(RUN_TESTS) choose + +# Targets for testing with linux's perf tool. +# As of 3.1-rc3, only upstream understands --log-fd, so using this +# will dump perf-stat output to stderr, not to *.perf-stat files. +# After --log-fd is released (perhaps waiting til its in distros), +# we'll add it to VG_OPTS. + +test.perf: test-prep + PERL_VALGRIND=1 VALGRIND=perf VG_TEST=--help VG_OPTS='stat --' $(RUN_TESTS) choose + # Targets for Third Degree testing. test_prep.third: test_prep perl.third -- 1.7.4.4 ```
p5pRT commented 13 years ago

From @jimc

uhm hold off on applying that patch. I think I botched some of the VG_OPTS defaults..

On Mon\, Sep 12\, 2011 at 1​:02 PM\, Jim Cromie \jim\.cromie@​gmail\.com wrote​:

Ive considered creating test.perf and test.cachegrind targets; - easier invocation\, w/o fiddling w environment vars - test.valgrind needs -g\, test.perf would not.

If this makes sense\, perhaps patch 2 should wait.

That does make sense.  I’ll wait.

I’ve applied 1 and 3 as 87dfd78cc5 and c96083ea\, respectively.  Thank you.

Ok\, heres those 2 new make targets.

a few new questions​:

1- all the VALGRIND-ish envars are now used more generally\, perhaps they need a more general name.  Suggestions / bikeshedding ?

PERL_DUT - (mnemonic - device under test (or really tool)) replaces PERL_VALGRIND\, which is just a flag (in t/TEST\, t/test.pl) since the tests run under some tool\, this seems to capture it.

PERL_DUTOOL - name/path of tool used when PERL_DUT is true.

VG_TEST - can go away\, hardcoded in each target. test.perf doesnt actually run that test anymore\, since I dropped its dependency on perl.valgrind.config

2- linux-perf tool version.

released perf doesnt understand --log-fd\, so patch above doesnt use it But it can\, if you have newer version\, and add it to VG_OPTS. Which future tense should I document for perlhack ?

3- PERL_DUT_DIR

Id like to add a new envar\, as a destination for all the *.{valgrind\,cachegrind\,perf-stat} files\, probably including a dated subdir to archive them automatically when set. Id also put the Storable file written if -d $ENV{HARNESS_TIMER} Sensible ? good name ?

p5pRT commented 12 years ago

From @jimc

1 - fixes gmake-isms by adding those ?= conditionals only on linux\, which uses gmake by default

2 - emit valgrind* targets only on linux.

p5pRT commented 12 years ago

From @jimc

0001-Makefile.SH-fix-gmake-isms.patch ```diff From c5d7e2afd7acb1dd3a9b3ecfd8c601909ef43f0c Mon Sep 17 00:00:00 2001 From: Jim Cromie Date: Wed, 28 Sep 2011 15:12:07 -0600 Subject: [PATCH 1/2] Makefile.SH: fix ?= gmake-isms commit c7b956bbbaff changed Makefile.SH to emit gmake-only syntax, fix that by doing so only on linux, by inserting a spitshell dependent on osname. This isnt the most direct fix, but it starts to isolate linux-only/mostly stuff, like test.valgrind. --- Makefile.SH | 10 ++++++++++ 1 files changed, 10 insertions(+), 0 deletions(-) diff --git a/Makefile.SH b/Makefile.SH index d135ec0..c20f660 100755 --- a/Makefile.SH +++ b/Makefile.SH @@ -331,11 +331,21 @@ $make_set_make # Mention $gmake here so it gets probed for by Configure. +!GROK!THIS! + +case "${osname}" in +linux*) +$spitshell >>$Makefile </dev/null +!GROK!THIS! + ;; +esac + +$spitshell >>$Makefile <
p5pRT commented 12 years ago

From @jimc

0002-Makefile.SH-emit-make-valgrind-targets-only-on-linux.patch ```diff From 076d28d8e602a94bff17e95a9fe11cf59657582d Mon Sep 17 00:00:00 2001 From: Jim Cromie Date: Wed, 28 Sep 2011 16:11:13 -0600 Subject: [PATCH 2/2] Makefile.SH: emit make valgrind* targets only on linux This inserts several case $osname in linux) spitshell ... esac statements to emit valgrind targets only where valgrind is available. Platform dependence is better than checking for valgrind executable because it serves as a subtle hint that it can be installed. Other platforms can be added by those who have them. --- Makefile.SH | 20 ++++++++++++++++++++ 1 files changed, 20 insertions(+), 0 deletions(-) diff --git a/Makefile.SH b/Makefile.SH index c20f660..92300a4 100755 --- a/Makefile.SH +++ b/Makefile.SH @@ -889,6 +889,11 @@ purecov$(PERL_EXE): $& perlmain$(OBJ_EXT) $(LIBPERL) $(static_ext) ext.libs $(PE quant$(PERL_EXE): $& perlmain$(OBJ_EXT) $(LIBPERL) $(static_ext) ext.libs $(PERLEXPORT) $(SHRPENV) $(LDLIBPTH) quantify $(CC) -o quantperl $(CLDFLAGS) $(CCDLFLAGS) perlmain$(OBJ_EXT) $(static_ext) $(LLIBPERL) `cat ext.libs` $(libs) +!NO!SUBS! + +case "${osname}${osvers}" in +linux*) + $spitshell >>$Makefile <<'!NO!SUBS!' # Valgrind perl (currently Linux only) perl.valgrind.config: config.sh @@ -899,6 +904,11 @@ perl.valgrind.config: config.sh @grep "^usemymalloc='n'" config.sh >/dev/null || exit 1 @echo "And of course you have to have valgrind..." $(VALGRIND) $(VG_TEST) || exit 1 +!NO!SUBS! + ;; +esac + +$spitshell >>$Makefile <<'!NO!SUBS!' # Third Degree Perl (Tru64 only) @@ -1441,6 +1451,11 @@ test.utf16 check.utf16: test_prep utest.utf16 ucheck.utf16: test_prep TEST_ARGS="-utf8 -utf16" $(RUN_TESTS) choose +!NO!SUBS! + +case "${osname}${osvers}" in +linux*) + $spitshell >>$Makefile <<'!NO!SUBS!' # Targets for valgrind testing: test_prep.valgrind: test_prep perl.valgrind @@ -1453,6 +1468,11 @@ utest.valgrind ucheck.valgrind: test_prep.valgrind perl.valgrind.config test_notty.valgrind: test_prep.valgrind perl.valgrind.config PERL_VALGRIND=1 $(RUN_TESTS) no-tty +!NO!SUBS! + ;; +esac + +$spitshell >>$Makefile <<'!NO!SUBS!' # Targets for Third Degree testing. -- 1.7.4.4 ```
p5pRT commented 12 years ago

From @tonycoz

On Wed\, Sep 28\, 2011 at 04​:50​:49PM -0600\, Jim Cromie wrote​:

1 - fixes gmake-isms by adding those ?= conditionals only on linux\, which uses gmake by default

2 - emit valgrind* targets only on linux.

valgrind is available on darwin* too.

Why is the gmake-ism is needed? Variables set on the make command-line should override those set in the Makefile.

Tony

* though apparently not on Lion

p5pRT commented 12 years ago

From @jimc

On Wed\, Sep 28\, 2011 at 5​:54 PM\, Tony Cook \tony@&#8203;develop\-help\.com wrote​:

On Wed\, Sep 28\, 2011 at 04​:50​:49PM -0600\, Jim Cromie wrote​:

1 - fixes gmake-isms  by adding those ?= conditionals only on linux\, which uses gmake by default

2 - emit valgrind* targets only on linux.

valgrind is available on darwin* too.

whats the $osname​:$osver value to use in the cases ?

Why is the gmake-ism is needed?

its not essential. I tried it\, it worked as Id hoped\, I moved on.

For the fix\, Nicholas outlined 3 choices\, I chose 1\, since that also moved towards supporting a test.perf target.

 Variables set on the make command-line should override those set in the Makefile.

Id expect ?= to work that way\, however the var was previously set.

Tony

* though apparently not on Lion

p5pRT commented 12 years ago

From @tonycoz

On Wed\, Sep 28\, 2011 at 06​:57​:24PM -0600\, Jim Cromie wrote​:

On Wed\, Sep 28\, 2011 at 5​:54 PM\, Tony Cook \tony@&#8203;develop\-help\.com wrote​:

On Wed\, Sep 28\, 2011 at 04​:50​:49PM -0600\, Jim Cromie wrote​:

1 - fixes gmake-isms  by adding those ?= conditionals only on linux\, which uses gmake by default

2 - emit valgrind* targets only on linux.

valgrind is available on darwin* too.

whats the $osname​:$osver value to use in the cases ?

No need.

Thanks\, applied as cfe76a0a8e5b6f21601522c788686e820ba933dd and 45ed6be318926c8b79c4ab96ac82eac9f727de66.

I've added Darwin in 0961731461727bea7a75cf92326539ddb48cbfce.

I've updated the URL and platforms in perlhacktips.pod in 0061d4fab85ba13ecc6cb1c4657841dbb1b85efb.

Tony

p5pRT commented 12 years ago

@tonycoz - Status changed from 'open' to 'resolved'