Re: Avoid repeated PQfnumber() in pg_dump

2021-08-27 Thread Bossart, Nathan
On 8/27/21, 7:27 AM, "Daniel Gustafsson"  wrote:
> I took another look at this today and pushed it after verifying with a 
> pgindent
> run.  Thanks!

Thank you!

Nathan



Re: Avoid repeated PQfnumber() in pg_dump

2021-08-27 Thread Daniel Gustafsson
> On 23 Jul 2021, at 01:39, Bossart, Nathan  wrote:

> The patch looks good to me.  I am marking it as ready-for-committer.

I took another look at this today and pushed it after verifying with a pgindent
run.  Thanks! 

--
Daniel Gustafsson   https://vmware.com/





Re: Avoid repeated PQfnumber() in pg_dump

2021-07-22 Thread Bossart, Nathan
On 7/15/21, 12:48 PM, "Daniel Gustafsson"  wrote:
> While unlikely to be common, very wide tables aren’t unheard of.  Either way, 
> I
> think it has merit to pull out the PQfnumber before the loop even if it’s a
> wash performance wise for many users, as it’s a pattern used elsewhere in
> pg_dump.

+1

The patch looks good to me.  I am marking it as ready-for-committer.

Nathan



Re: Avoid repeated PQfnumber() in pg_dump

2021-07-15 Thread Daniel Gustafsson
> On 15 Jul 2021, at 04:51, houzj.f...@fujitsu.com wrote:
> On July 15, 2021 5:35 AM Daniel Gustafsson  wrote:

>> Out of curiosity, how many columns are "many columns"?
> 
> I tried dump 10 table definitions while each table has 1000 columns
> (maybe not real world case).

While unlikely to be common, very wide tables aren’t unheard of.  Either way, I
think it has merit to pull out the PQfnumber before the loop even if it’s a
wash performance wise for many users, as it’s a pattern used elsewhere in
pg_dump.

--
Daniel Gustafsson   https://vmware.com/





RE: Avoid repeated PQfnumber() in pg_dump

2021-07-14 Thread houzj.f...@fujitsu.com
On July 15, 2021 5:35 AM Daniel Gustafsson  wrote:
> > On 14 Jul 2021, at 10:54, houzj.f...@fujitsu.com wrote:
> 
> > Since PQfnumber() is not a cheap function, I think we'd better invoke
> > PQfnumber() out of the loop like the attatched patch.
> 
> Looks good on a quick readthrough, and I didn't see any other similar
> codepaths in pg_dump on top of what you've fixed.

Thanks for reviewing the patch.
Added to the CF: https://commitfest.postgresql.org/34/3254/

> > After applying this change, I can see about 8% performance gain in my
> > test environment when dump table definitions which have many columns.
> 
> Out of curiosity, how many columns are "many columns"?

I tried dump 10 table definitions while each table has 1000 columns
(maybe not real world case).

Best regards,
houzj




Re: Avoid repeated PQfnumber() in pg_dump

2021-07-14 Thread Daniel Gustafsson
> On 14 Jul 2021, at 10:54, houzj.f...@fujitsu.com wrote:

> Since PQfnumber() is not a cheap function, I think we'd better invoke
> PQfnumber() out of the loop like the attatched patch.

Looks good on a quick readthrough, and I didn't see any other similar codepaths
in pg_dump on top of what you've fixed.

> After applying this change, I can see about 8% performance gain in my test 
> environment
> when dump table definitions which have many columns.

Out of curiosity, how many columns are "many columns"?

--
Daniel Gustafsson   https://vmware.com/





Re: Avoid repeated PQfnumber() in pg_dump

2021-07-14 Thread Justin Pryzby
On Wed, Jul 14, 2021 at 08:54:32AM +, houzj.f...@fujitsu.com wrote:
> Since PQfnumber() is not a cheap function, I think we'd better invoke
> PQfnumber() out of the loop like the attatched patch.

+1

Please add to the next CF

-- 
Justin




Avoid repeated PQfnumber() in pg_dump

2021-07-14 Thread houzj.f...@fujitsu.com
Hi,

When reviewing some pg_dump related code, I found some existsing code
(getTableAttrs() and dumpEnumType()) invoke PQfnumber() repeatedly which seems
unnecessary.

Example
-
for (int j = 0; j < ntups; j++)
{
if (j + 1 != atoi(PQgetvalue(res, j, PQfnumber(res, 
"attnum"
-

Since PQfnumber() is not a cheap function, I think we'd better invoke
PQfnumber() out of the loop like the attatched patch.

After applying this change, I can see about 8% performance gain in my test 
environment
when dump table definitions which have many columns.

Best regards,
Hou zhijie


0001-Avoid-repeated-PQfnumber.patch
Description: 0001-Avoid-repeated-PQfnumber.patch