Re: Use get_call_result_type() more widely

2022-12-20 Thread Bharath Rupireddy
On Wed, Dec 21, 2022 at 6:44 AM Michael Paquier wrote: > > I have applied v2-0001. Thanks for taking care of this. By seeing the impact that get_call_result_type() can have for the functions that are possibly called repeatedly, I couldn't resist sharing a patch (attached herewith) that adds a

Re: Use get_call_result_type() more widely

2022-12-20 Thread Michael Paquier
On Tue, Dec 20, 2022 at 11:12:09AM -0500, Tom Lane wrote: > On the whole, I'm content to leave the BlessTupleDesc calls in > these callers. They are cheap enough if the tupdesc is already > blessed. Yeah, agreed. I have applied v2-0001, after fixing one error in wparser.c where some of the

Re: Use get_call_result_type() more widely

2022-12-20 Thread Tom Lane
Bharath Rupireddy writes: > On Tue, Dec 20, 2022 at 1:41 PM Tom Lane wrote: >> Hmm ... at least one of the paths through internal_get_result_type >> is intentionally blessing the result tupdesc: >> but it's not clear if they all do, and the comments certainly >> aren't promising it. > It looks

Re: Use get_call_result_type() more widely

2022-12-20 Thread Bharath Rupireddy
On Tue, Dec 20, 2022 at 1:41 PM Tom Lane wrote: > > Michael Paquier writes: > > On Mon, Dec 19, 2022 at 07:41:27PM +0530, Bharath Rupireddy wrote: > >> 0002 - gets rid of an unnecessary call to BlessTupleDesc() > >> after get_call_result_type(). > > > Hmm. I am not sure whether this is right,

Re: Use get_call_result_type() more widely

2022-12-20 Thread Tom Lane
Michael Paquier writes: > On Mon, Dec 19, 2022 at 07:41:27PM +0530, Bharath Rupireddy wrote: >> 0002 - gets rid of an unnecessary call to BlessTupleDesc() >> after get_call_result_type(). > Hmm. I am not sure whether this is right, actually.. Hmm ... at least one of the paths through

Re: Use get_call_result_type() more widely

2022-12-19 Thread Michael Paquier
On Mon, Dec 19, 2022 at 07:41:27PM +0530, Bharath Rupireddy wrote: > I agree with the bucketization. Please see the attached patches. 0001 > - gets rid of explicit tuple desc creation using > get_call_result_type() for functions thought to be not-so-frequently > called. It looks like I am OK with

Re: Use get_call_result_type() more widely

2022-12-19 Thread Michael Paquier
On Mon, Dec 19, 2022 at 05:50:03PM -0500, Robert Haas wrote: > All right, well, I just work here. :-) Just to give some numbers. The original version of the patch doing the full switch removed 500 lines of code. The second version that switches the "non-critical" paths removes 200~ lines. --

Re: Use get_call_result_type() more widely

2022-12-19 Thread Robert Haas
On Mon, Dec 19, 2022 at 4:21 PM Tom Lane wrote: > Now that somebody's made an effort to identify which places are > potentially performance-critical, I don't see why we wouldn't use > the fruits of their labor. Yes, somebody else might draw the line > differently, but drawing a line at all seems

Re: Use get_call_result_type() more widely

2022-12-19 Thread Tom Lane
Robert Haas writes: > On Mon, Dec 19, 2022 at 2:07 PM Alvaro Herrera > wrote: >> On the other hand, the measurements have shown that going through the >> function is significantly slower. So I kinda like the judgement call >> that Michael and Bharath have made: change to use the function when

Re: Use get_call_result_type() more widely

2022-12-19 Thread Robert Haas
On Mon, Dec 19, 2022 at 2:07 PM Alvaro Herrera wrote: > On the other hand, the measurements have shown that going through the > function is significantly slower. So I kinda like the judgement call > that Michael and Bharath have made: change to use the function when > performance is not an

Re: Use get_call_result_type() more widely

2022-12-19 Thread Alvaro Herrera
On 2022-Dec-19, Robert Haas wrote: > Here's a modest proposal: let's do nothing about this. There's no > evidence of a real problem here, so we're going to be trying to judge > the performance benefits against the code size savings without any > real data indicating that either one is an issue. I

Re: Use get_call_result_type() more widely

2022-12-19 Thread Robert Haas
On Tue, Dec 13, 2022 at 10:43 AM Tom Lane wrote: > Saving code is nice, but I'd assume the result is slower, because > get_call_result_type has to do a pretty substantial amount of work > to get the data to construct the tupdesc from. Have you tried to > quantify how much overhead this'd add?

Re: Use get_call_result_type() more widely

2022-12-19 Thread Bharath Rupireddy
On Thu, Dec 15, 2022 at 11:41 AM Michael Paquier wrote: > > On Wed, Dec 14, 2022 at 11:14:59AM +0530, Bharath Rupireddy wrote: > > AFAICS, most of these functions have no direct source code callers, > > they're user-facing functions and not in a hot code path. I measured > > the test times of

Re: Use get_call_result_type() more widely

2022-12-14 Thread Michael Paquier
On Wed, Dec 14, 2022 at 11:14:59AM +0530, Bharath Rupireddy wrote: > AFAICS, most of these functions have no direct source code callers, > they're user-facing functions and not in a hot code path. I measured > the test times of these functions and I don't see much difference [1]. Thanks for the

Re: Use get_call_result_type() more widely

2022-12-13 Thread Bharath Rupireddy
On Tue, Dec 13, 2022 at 9:12 PM Tom Lane wrote: > > Bharath Rupireddy writes: > > A review comment in another thread [1] by Michael Paquier about the > > usage of get_call_result_type() instead of explicit building of > > TupleDesc made me think about using it more widely. Actually, the > >

Re: Use get_call_result_type() more widely

2022-12-13 Thread Tom Lane
Bharath Rupireddy writes: > A review comment in another thread [1] by Michael Paquier about the > usage of get_call_result_type() instead of explicit building of > TupleDesc made me think about using it more widely. Actually, the > get_call_result_type() looks at the function definitions to

Re: Use get_call_result_type() more widely

2022-12-13 Thread Michael Paquier
On Tue, Dec 13, 2022 at 01:06:48PM +0530, Bharath Rupireddy wrote: > A review comment in another thread [1] by Michael Paquier about the > usage of get_call_result_type() instead of explicit building of > TupleDesc made me think about using it more widely. Actually, the > get_call_result_type()

Use get_call_result_type() more widely

2022-12-12 Thread Bharath Rupireddy
Databases Amazon Web Services: https://aws.amazon.com From e1df78cc86e3d38a2814d6cd89f6b86a8de4a284 Mon Sep 17 00:00:00 2001 From: Bharath Rupireddy Date: Tue, 13 Dec 2022 07:04:22 + Subject: [PATCH v1] Use get_call_result_type() more widely Usage of get_call_result_type() in more places