From: | Tim Bunce <Tim(dot)Bunce(at)pobox(dot)com> |
---|---|
To: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
Cc: | Tim Bunce <Tim(dot)Bunce(at)pobox(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | PL/Perl function naming (was: release notes) |
Date: | 2010-06-15 09:16:17 |
Message-ID: | 20100615091617.GD31960@timac.local |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
[Sorry for the delay. I'd stopped checking the pgsql mailing lists.
Thanks to David Wheeler and Josh Berkus for the heads-up.]
On Sun, May 30, 2010 at 06:58:32PM -0400, Andrew Dunstan wrote:
>
> Tim Bunce wrote:
> >On Mon, May 17, 2010 at 11:34:37AM -0400, Tom Lane wrote:
> >>Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> >>>Why do the release notes say this, under plperl:
> >>> * PL/Perl subroutines are now given names (Tim Bunce)
> >>> This is for the use of profiling and code coverage tools. DIDN'T
> >>> THEY HAVE NAMES BEFORE?
> >>> If whoever put this in the release notes had bothered
> >>>to ask I am sure we would have been happy to explain.
> >>You don't need to complain, just fix it. The point of the comment is
> >>that more explanation is needed.
> >
> >I think you can just delete it. It's too esoteric to be worth noting.
> >
> >Tim.
> >
> >p.s. It also turned to be insufficiently useful for NYTProf since it
> >doesn't also update some internals to record the 'filename' and line
> >number range of the sub. So PostgreSQL::PLPerl::NYTProf works around
> >that by wrapping mkfuncsrc() to edit the generated perl code to include
> >a subname in the usual "sub $name { ... }" style. I may offer a patch
> >for 9.1 to to make it work that way.
>
> I put this aside to think about it.
>
> If the "feature" is not any use should we rip it out? We pretty much
> included it because you said it was what you needed for the
> profiler.
Yes, it can go.
*** a/src/pl/plperl/plperl.c
--- b/src/pl/plperl/plperl.c
*************** plperl_create_sub(plperl_proc_desc *prod
*** 1319,1328 ****
(errmsg("didn't get a CODE ref from compiling %s",
prodesc->proname)));
- /* give the subroutine a proper name in the main:: symbol table */
- CvGV(SvRV(subref)) = (GV *) newSV(0);
- gv_init(CvGV(SvRV(subref)), PL_defstash, subname, strlen(subname), TRUE);
-
prodesc->reference = subref;
return;
> I'm seriously nervous about adding function names - making functions
> directly callable like that could be a major footgun recipe, since
> now we are free to hide some stuff in the calling mechanism, and can
> (and have in the past) changed that to suit our needs, e.g. when we
> added trigger support via a hidden function argument. That has been
> safe precisely because nobody has had a handle on the subroutine
> which would enable it to be called directly from perl code. There
> have been suggestions in the past of using this for other features,
> so we should not assume that the calling mechanism is fixed in stone.
Agreed. It is a very hard to use footgun though because the oid is
included in the name. It's certainly not something anyone would shoot
themselves with by accident.
[Speaking of calling conventions: I never did get time for some decent
performance optimization. I'd like to get rid of the explicit extra
trigger data argument and corresponding "local $_TD=shift;" prologue.
That could be done much faster in C.]
> Perhaps we could decorate the subs with attributes, or is that not
> doable for anonymous subs?
Perhaps. I'll look into it when I get around to working on the PL/Perl
NYTProf again. For the profiler it would be enough to only enable the
naming when the appropriate perl debugging flag bits are set, so it
wouldn't happen normally.
Tim.
From | Date | Subject | |
---|---|---|---|
Next Message | Fujii Masao | 2010-06-15 09:40:39 | debug log in pg_archivecleanup |
Previous Message | Fujii Masao | 2010-06-15 08:45:13 | Re: Proposal for 9.1: WAL streaming from WAL buffers |