On Thu, Dec 17, 2009 at 05:08:11PM +0000, Nicholas Clark wrote:
> On Thu, Dec 17, 2009 at 01:38:43PM +0000, Tim Bunce wrote:
> > On Thu, Dec 17, 2009 at 10:52:35AM +0000, Nicholas Clark wrote:
> > > It moves the call to normalize_eval_seqn() after the callback runs.
> > > With this change, it's possible to write a program to output a
> > > byte-for-byte
> > > identical file via the callback interface.
> >
> > I'm curious about why you might want to do that.
>
> Ah, well. Because you can test whether you've read in and written out *one*
> file correctly.
>
> At which point it's not a huge step to read in multiple files, and write one
> out...
>
> Which I believe might be useful.
Beautifully understated :)
> Appended is the summary, attached are all the commits. Should I push these?
> (I'm using git-svn)
>
> Nicholas Clark
>
> +MODULE = Devel::NYTProf PACKAGE = Devel::NYTProf::FileHandle
NYTProf.xs is getting rather bloated. I'd love to get the NYTP_* i/o
functions moved out. (I'd even wondered about an libnytpio.so that other
tools could link to.)
Any chance you could add a FileHandle.xs containing the NYTP_* i/o
functions and the XS interface code for them?
I figure the FileHandle.o could be included in the link that creates the
NYTProf.so to avoid having two .so and problems accessing the function
pointers. Making the functions non-static and adding a .h should suffice.
> + if(!sv_isa(handle, "Devel::NYTProf::FileHandle"))
> + croak("handle is not a Devel::NYTProf::FileHandle");
> + fh = (NYTP_file)SvPVX(SvRV(handle));
A typemap entry could neatly handle that repeated boilerplate code.
> diff --git a/bin/nytprofmerge b/bin/nytprofmerge
> +my %dispatcher =
> + ATTRIBUTE => sub {
> + my (undef, $key, $value) = @_;
> + $out->write(":$key=$value\n");
> + },
Attributes probably need a little more logic.
Some should be output immediately:
ticks_per_sec, profiler_start_time, profiler_end_time
For the rest I'd suggest doing
$attr->{$key}{$value}++;
here.
In the code to finalize the file, output any attributes that only have a
single distinct value. Any with more that one...
application: join with " and "
basetime: min(...)
clock_id: croak
complete: warn, output 0
perl_version: warn, output min
profiler_start_time: output min
profiler_end_time: output max
profiler_duration: discard
xs_version: croak
...others...: croak
> + NEW_FID => sub {
> + my (undef, $fid, $eval_fid, $eval_line, $flags, $size, $mtime, $name)
> = @_;
> + my ($new_fid, $new_eval_fid);
> + if($eval_fid) {
> + $new_eval_fid = $fids{$eval_fid};
> + confess("unknown eval_fid $eval_fid") unless defined $new_eval_fid;
> + $new_fid = $next_fid++;
> + $fids{$fid} = $new_fid;
> + } else {
> + $new_eval_fid = $eval_fid;
> + $new_fid = $file_to_fid{$name};
I'd add $size and $mtime to the key (not used yet, but this'll make them
more important).
> + return if defined $new_fid;
> +
> + $new_fid = $next_fid++;
> + $fids{$fid} = $new_fid;
> + $file_to_fid{$name} = $fid;
That looks like it'll assign different fids to the same source file in
different profiles. I think most people would much rather the profiles
for a given source file are merged. (It might seem to 'work' currently
but that would just be a side-effect of the reporting code.)
> + }
> + $out->write('@');
> + $out->output_int($new_fid, $new_eval_fid, $eval_line, $flags, $size,
> $mtime);
> + $out->output_str($name);
> + },
Overall it looks great and would make a wonderful addition to v3.
Thank you!
Tim.
--
You've received this message because you are subscribed to
the Devel::NYTProf Development User group.
Group hosted at: http://groups.google.com/group/develnytprof-dev
Project hosted at: http://perl-devel-nytprof.googlecode.com
CPAN distribution: http://search.cpan.org/dist/Devel-NYTProf
To post, email: [email protected]
To unsubscribe, email: [email protected]
