Date: Mon, 15 Oct 2018 16:45:03 -0700
From: Andres Freund <andres@anarazel.de>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-hackers@postgresql.org
Subject: Re: Large writable variables
Message-ID: <20181015234503.rtgz2bokftnkrba5@alap3.anarazel.de>
References: <20181015200754.7y7zfuzsoux2c4ya@alap3.anarazel.de>
 <18430.1539635786@sss.pgh.pa.us>
 <15484.1539646909@sss.pgh.pa.us>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <15484.1539646909@sss.pgh.pa.us>
Precedence: bulk

Hi,

I've a patch making .data variables constant, fwiw. Will post in a
bit. Just so we don't duplicate work too much.


On 2018-10-15 19:41:49 -0400, Tom Lane wrote:
> I wrote:
> > Andres Freund <andres@anarazel.de> writes:
> >> 0000000008560224 0000000000023440 b tzdefrules_s
> >> 0000000008536704 0000000000023440 b gmtmem.7009
> 
> > I think that tzdefrules_s is not used in common cases (though I could be
> > wrong about that), so we could win by alloc-on-first-use.  The same might
> > be true for gmtmem, but there's a sticking point: there is no provision
> > for failure there, so I'm unsure how we avoid crashing on OOM.
> 
> So my intuition was exactly backwards on this.
> 
> 1. tzdefrules_s is filled in the postmaster, and if it's not filled there
> it'd be filled by every child process, so there's zero point in converting
> it to malloc-on-the-fly style.  This is because tzparse() always wants
> it filled.  That's actually a tad annoying, because it looks to me like
> with many timezone settings the data will not get used, but I'm hesitant
> to mess with the IANA code's logic enough to improve that.  Maybe I'll try
> submitting an upstream patch and see what they think of it first.

Ok, that makes sense.


> 2. gmtmem, on the other hand, is only used if somebody calls pg_gmtime,
> which already has a perfectly good error-report convention, cf gmtime(3).
> We have exactly one caller, which was not bothering to test for error :-(
> and would dump core on failure.  And that caller isn't reached in most
> processes, at least not in the regression tests.  So this side of it is
> easy to improve.
> 
> Hence I propose the attached patch for point 2, which I'd want to
> backpatch to keep our copies of the IANA code in sync.

That makes sense, too.

Greetings,

Andres Freund