Re: Can we remove extra memset in BloomInitPage, GinInitPage and SpGistInitPage when we have it in PageInit?

On Tue, 6 Apr 2021 at 19:14, Bharath Rupireddy
<bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
>
> On Tue, Apr 6, 2021 at 6:09 PM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
> >
> > On Mon, Mar 22, 2021 at 10:58:17AM +0530, Mahendra Singh Thalor wrote:
> > > Your changes look to fine me and I am also not getting any failure. I
> > > think we should back-patch all the branches.
> > >
> > > Patch is applying to all the branches(till v95) and there is no failure.
> >
> > Er, no. This is just some duplicated code with no extra effect. I
> > have no objection to simplify a bit the whole on readability and
> > consistency grounds (will do so tomorrow), including the removal of
> > the commented-out memset call in gistinitpage, but this is not
> > something that should be backpatched.
>
> +1 to not backport this patch because it's not a bug or not even a
> critical issue. Having said that removal of these unnecessary memsets
> would not only be better for readability and consistency but also can
> reduce few extra function call costs(although minimal) while adding
> new index pages.
>
> Please find the v3 patch that removed the commented-out memset call in
> gistinitpage.

Thanks Bharath for updated patch.

+++ b/src/backend/storage/page/bufpage.c
@@ -51,7 +51,7 @@ PageInit(Page page, Size pageSize, Size specialSize)
/* Make sure all fields of page are zero, as well as unused space */
MemSet(p, 0, pageSize);

- p->pd_flags = 0;
+ /* p->pd_flags = 0; done by above MemSet */

I think, for readability we can keep old code here or we can remove
new added comment also.

Apart from this, all other changes looks good to me.

--
Thanks and Regards
Mahendra Singh Thalor
EnterpriseDB: http://www.enterprisedb.com