public inbox for [email protected]help / color / mirror / Atom feed
Re: Missed compiler optimization issue in function select_rtable_names_for_explain 3+ messages / 2 participants [nested] [flat]
* Re: Missed compiler optimization issue in function select_rtable_names_for_explain @ 2024-05-22 10:55 Daniel Gustafsson <[email protected]> 2024-05-22 16:53 ` Re: Missed compiler optimization issue in function select_rtable_names_for_explain Tom Lane <[email protected]> 0 siblings, 1 reply; 3+ messages in thread From: Daniel Gustafsson @ 2024-05-22 10:55 UTC (permalink / raw) To: XChy <[email protected]>; +Cc: pgsql-general > On 22 May 2024, at 12:12, XChy <[email protected]> wrote: > >> How is the memset in select_rtable_names_for_explain a dead-store? Even memset calls could be optimized away from the EXPLAIN codepath I have a feeling it >> would have to be many in a tight loop for it to be measurable even? > For the first question, I don't mean that the memset is the dead store. Gotcha > I mean that the stores with value "0" after the memset are dead: > ``` > dpns.subplans = NIL; > dpns.ctes = NIL; > dpns.appendrels = NULL; > ``` > since the memset has written zeroes to the object "dpns", and these members are known to be zero. They are known to be zero, but that's not entirely equivalent though is it? NIL is defined as ((List *) NULL) and NULL is typically defined as ((void *) 0), so sizeof(0) would be the size of an int and sizeof(NULL) would be the size of a void pointer. -- Daniel Gustafsson ^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: Missed compiler optimization issue in function select_rtable_names_for_explain 2024-05-22 10:55 Re: Missed compiler optimization issue in function select_rtable_names_for_explain Daniel Gustafsson <[email protected]> @ 2024-05-22 16:53 ` Tom Lane <[email protected]> 2024-05-22 18:14 ` Re: Missed compiler optimization issue in function select_rtable_names_for_explain Daniel Gustafsson <[email protected]> 0 siblings, 1 reply; 3+ messages in thread From: Tom Lane @ 2024-05-22 16:53 UTC (permalink / raw) To: Daniel Gustafsson <[email protected]>; +Cc: XChy <[email protected]>; pgsql-general Daniel Gustafsson <[email protected]> writes: > They are known to be zero, but that's not entirely equivalent though is it? > NIL is defined as ((List *) NULL) and NULL is typically defined as ((void *) > 0), so sizeof(0) would be the size of an int and sizeof(NULL) would be the size > of a void pointer. There are other places where we assume that a memset-to-zero will produce null pointers, so I don't think that that objection has a lot of force. My real answer is that this is our coding style and we are not going to change it: our normal convention is to initialize struct fields in declaration order, and that's what we're doing here. If some particular version of some particular compiler fails to make an entirely-negligible optimization in consequence, that is not something we are going to care about. Maybe we would care if the missed optimization were a serious performance loss in a very hot code path. But this is neither. I'd also say that this is pretty clearly a compiler bug. If it'd normally optimize away a null-pointer-store following a memset-to-zero, but does not in memset(&dpns, 0, sizeof(dpns)); dpns.rtable = rtable; dpns.subplans = NIL; dpns.ctes = NIL; dpns.appendrels = NULL; that would seem to indicate that the optimizer doesn't really understand that dpns.rtable is a distinct field from the others. How is that our problem? (I kind of wonder if what's actually blocking the optimization is the casts inside the NIL macros. Still not our problem.) regards, tom lane ^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: Missed compiler optimization issue in function select_rtable_names_for_explain 2024-05-22 10:55 Re: Missed compiler optimization issue in function select_rtable_names_for_explain Daniel Gustafsson <[email protected]> 2024-05-22 16:53 ` Re: Missed compiler optimization issue in function select_rtable_names_for_explain Tom Lane <[email protected]> @ 2024-05-22 18:14 ` Daniel Gustafsson <[email protected]> 0 siblings, 0 replies; 3+ messages in thread From: Daniel Gustafsson @ 2024-05-22 18:14 UTC (permalink / raw) To: Tom Lane <[email protected]>; +Cc: XChy <[email protected]>; pgsql-general > On 22 May 2024, at 18:53, Tom Lane <[email protected]> wrote: > > Daniel Gustafsson <[email protected]> writes: >> They are known to be zero, but that's not entirely equivalent though is it? >> NIL is defined as ((List *) NULL) and NULL is typically defined as ((void *) >> 0), so sizeof(0) would be the size of an int and sizeof(NULL) would be the size >> of a void pointer. > > There are other places where we assume that a memset-to-zero will > produce null pointers, so I don't think that that objection has > a lot of force. It wasn't really an objection, but (perhaps a badly worded) an attempt to understand the proposal. > My real answer is that this is our coding style > and we are not going to change it: our normal convention is to > initialize struct fields in declaration order, and that's what > we're doing here. If some particular version of some particular > compiler fails to make an entirely-negligible optimization in > consequence, that is not something we are going to care about. +1 -- Daniel Gustafsson ^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2024-05-22 18:14 UTC | newest] Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2024-05-22 10:55 Re: Missed compiler optimization issue in function select_rtable_names_for_explain Daniel Gustafsson <[email protected]> 2024-05-22 16:53 ` Tom Lane <[email protected]> 2024-05-22 18:14 ` Daniel Gustafsson <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox