Re: BUG #14897: Segfault on statitics SQL request
Andres Freund <andres@anarazel.de>
From: Andres Freund <andres@anarazel.de>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Dmitry Dolgov <9erthalion6@gmail.com>, Michael Paquier <michael.paquier@gmail.com>, Vincent Lachenal <vincent.lachenal@gmail.com>, PostgreSQL mailing lists <pgsql-bugs@postgresql.org>
Date: 2017-11-11T18:11:45Z
Lists: pgsql-bugs
On 2017-11-11 12:54:29 -0500, Tom Lane wrote: > Andres Freund <andres@anarazel.de> writes: > > On 2017-11-11 12:41:40 -0500, Tom Lane wrote: > >> Yeah, changing MAXALIGN is out of the question. I'm thinking about > >> another flag bit for MemoryContextAllocExtended. Do we need to think > >> about other use-cases besides int128? Should we just force 16-byte > >> alignment on all architectures, or does it need to be platform-specific? > > > I'm not sure we want to > > a) Rely on one alignment being enough for everybody. > > b) Additionally burden already hot code paths with a growing number of > > alignment flag tests, and the necessary math. > > Well, (a) we could have more flag bits later if there are other use-cases > that require even stricter alignment Right, that's my point. It's not a scalable interface. > and (b) I do not believe that MemoryContextAllocExtended is a hot code > path at present; there are not enough call sites. It's not that hot, true. > > How about a MemoryContextAllocAligned(context, size, alignto, flags) > > that passes on most flags but adds the necessary overhead to size, and > > padds the result appropriately? > > This'd result in hard-wiring the alignment requirement at call sites, > which I think might not be a great idea. Well, it'd require using a macro specifying the alignment (or if we were using a modern C dialect alignof()), not the alignment spelled out. > For example, one plausible future use-case is "align on cacheline > boundaries". Unless we go to threads that doesn't seem *that* likely, given what palloc's used for. > I think that would be better served by a flag like > MCXT_ALLOC_ALIGN_CACHELINE than by having the callers demand a > specific numeric alignment value --- it'd be a lot easier to make the > alignment match actual hardware requirements if it were being inserted > at one specific place. Specifying MCXT_ALLOC_ALIGN_CACHELINE rather than CACHELINE_ALIGNMENT doesn't seem to make it meaningfully harder to adjust. Or are you thinking of probing the hardware? Greetings, Andres Freund
Commits
-
Prevent int128 from requiring more than MAXALIGN alignment.
- d4e38489f98a 9.5.11 landed
- 619a8c47da72 10.2 landed
- 4a15f87d2277 9.6.7 landed
- 7518049980be 11.0 landed
-
Rearrange c.h to create a "compiler characteristics" section.
- cfc157078f2e 9.5.11 landed
- a8910506757c 10.2 landed
- 6c35b3aa465e 9.6.7 landed
- 91aec93e6089 11.0 landed