On Thu, Dec 17, 2009 at 09:13:44PM +0000, Tim Bunce wrote:
> Any chance you could add a FileHandle.xs containing the NYTP_* i/o
> functions and the XS interface code for them?
Done and pushed.
> 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.
But it turned out that there need to be some games with DynaLoader to bootstrap
the second module.
> > + 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.
True, but I don't really understand typemaps. I had forgotten about this
comment when reworking my patches locally.
>
> > 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.
Not done yet.
> > + 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.)
I didn't *think* that it did, because when I tested things by merging the
same profile twice, and then dumping it with dump_profile_data(), I got an
identical structure, with (as best I could tell) all the times doubled.
> Overall it looks great and would make a wonderful addition to v3.
On the assumption that it was going in the right direction, and other people
(hello lurkers) might want to play with it (or even hack on it), I committed
it. For now, it's still guilty of violating Don't Repeat Yourself because
it re-implements the internals of the file format in Perl. I've not looked
at how to refactor the XS to provide a higher-level API for writing the output
file, and providing a Perl API to that for it to use. I'm back in the office
on Monday, and so far there's nothing more urgent than this to steal my time.
It has no tests yet - I was rather hoping that someone lurking would feel
like that would make a good project to pay with.
Nicholas Clark
--
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]
