Robert Haas [Mon, 29 Aug 2022 14:10:09 +0000 (10:10 -0400)]
docs: Fix up some out-of-date references to INHERIT/NOINHERIT.
Commit
e3ce2de09d814f8770b2e3b3c152b7671bcdb83f should have updated
these sections of the documentation, but failed to do so.
Patch by me, reviewed by Nathan Bossart.
Discussion: http://postgr.es/m/CA+TgmoaKMnde2W_=u7CqeCKi=FKnfbNQPwOR=c_3c8qD7b2nhQ@mail.gmail.com
David Rowley [Mon, 29 Aug 2022 11:20:25 +0000 (23:20 +1200)]
Add missing padding from MemoryChunk struct
Buildfarm animals skate, grison and mamba are Assert failing on the
pointer being given to repalloc not being MAXALIGNED.
c6e0fe1f2a made
changes in that area.
All of these animals are 32-bit with a MAXIMUM_ALIGNOF of 8 and a
SIZEOF_VOID_P of 4. I suspect that the pointer is not properly aligned due
to the lack of padding in the MemoryChunk struct.
Here we add the same type of padding that was previously used in
AllocChunkData and GenerationChunk that
c6e0fe1f2a neglected to add.
Discussion: https://postgr.es/m/CAA4eK1%2B1JyW5TiL%3DyV-3Uq1CrfnTyn0Xrk5uArt31Z%3D8rgPhXQ%40mail.gmail.com
John Naylor [Mon, 29 Aug 2022 10:25:59 +0000 (17:25 +0700)]
Fix broken cast on MSVC
Per buildfarm animal drongo, casting a vector type to the same type
causes a compile error. We still need the cast on ARM64, so invent a
wrapper function that does the casting only where necessary.
Discussion: https://www.postgresql.org/message-id/CAFBsxsEouaTwbmpqV%2BEW2%3DwFbhw2vHRe26NQTRcd0%3DNaOFDy7A%40mail.gmail.com
John Naylor [Mon, 29 Aug 2022 07:32:54 +0000 (14:32 +0700)]
Use ARM Advanced SIMD (NEON) intrinsics where available
NEON support is required on the Aarch64 architecture for standard
implementations. Hardware designers for specialized markets can choose
not to support it, but that's true of floating point as well, which
we assume is supported. As with x86, some SIMD support is available
on 32-bit platforms, but those are not interesting from a performance
standpoint and would require an inconvenient runtime check.
Nathan Bossart
Reviewed by John Naylor, Andres Freund, Thomas Munro, and Tom Lane
Discussion: https://www.postgresql.org/message-id/flat/CAFBsxsEyR9JkfbPcDXBRYEfdfC__OkwVGdwEAgY4Rv0cvw35EA%40mail.gmail.com#
aba7a64b11503494ffd8dd27067626a9
John Naylor [Mon, 29 Aug 2022 06:40:53 +0000 (13:40 +0700)]
Abstract some more architecture-specific details away from SIMD functionality
Add a typedef to represent vectors containing four 32-bit integers,
and add functions operating on them. Also separate out saturating
subtraction into its own function. The motivation for this is to
prepare for a future commit to add ARM NEON support.
Nathan Bossart
Reviewed by John Naylor and Tom Lane
Discussion: https://www.postgresql.org/message-id/flat/CAFBsxsEyR9JkfbPcDXBRYEfdfC__OkwVGdwEAgY4Rv0cvw35EA%40mail.gmail.com#
aba7a64b11503494ffd8dd27067626a9
David Rowley [Mon, 29 Aug 2022 05:15:00 +0000 (17:15 +1200)]
Improve performance of and reduce overheads of memory management
Whenever we palloc a chunk of memory, traditionally, we prefix the
returned pointer with a pointer to the memory context to which the chunk
belongs. This is required so that we're able to easily determine the
owning context when performing operations such as pfree() and repalloc().
For the AllocSet context, prior to this commit we additionally prefixed
the pointer to the owning context with the size of the chunk. This made
the header 16 bytes in size. This 16-byte overhead was required for all
AllocSet allocations regardless of the allocation size.
For the generation context, the problem was worse; in addition to the
pointer to the owning context and chunk size, we also stored a pointer to
the owning block so that we could track the number of freed chunks on a
block.
The slab allocator had a 16-byte chunk header.
The changes being made here reduce the chunk header size down to just 8
bytes for all 3 of our memory context types. For small to medium sized
allocations, this significantly increases the number of chunks that we can
fit on a given block which results in much more efficient use of memory.
Additionally, this commit completely changes the rule that pointers to
palloc'd memory must be directly prefixed by a pointer to the owning
memory context and instead, we now insist that they're directly prefixed
by an 8-byte value where the least significant 3-bits are set to a value
to indicate which type of memory context the pointer belongs to. Using
those 3 bits as an index (known as MemoryContextMethodID) to a new array
which stores the methods for each memory context type, we're now able to
pass the pointer given to functions such as pfree() and repalloc() to the
function specific to that context implementation to allow them to devise
their own methods of finding the memory context which owns the given
allocated chunk of memory.
The reason we're able to reduce the chunk header down to just 8 bytes is
because of the way we make use of the remaining 61 bits of the required
8-byte chunk header. Here we also implement a general-purpose MemoryChunk
struct which makes use of those 61 remaining bits to allow the storage of
a 30-bit value which the MemoryContext is free to use as it pleases, and
also the number of bytes which must be subtracted from the chunk to get a
reference to the block that the chunk is stored on (also 30 bits). The 1
additional remaining bit is to denote if the chunk is an "external" chunk
or not. External here means that the chunk header does not store the
30-bit value or the block offset. The MemoryContext can use these
external chunks at any time, but must use them if any of the two 30-bit
fields are not large enough for the value(s) that need to be stored in
them. When the chunk is marked as external, it is up to the MemoryContext
to devise its own means to determine the block offset.
Using 3-bits for the MemoryContextMethodID does mean we're limiting
ourselves to only having a maximum of 8 different memory context types.
We could reduce the bit space for the 30-bit value a little to make way
for more than 3 bits, but it seems like it might be better to do that only
if we ever need more than 8 context types. This would only be a problem
if some future memory context type which does not use MemoryChunk really
couldn't give up any of the 61 remaining bits in the chunk header.
With this MemoryChunk, each of our 3 memory context types can quickly
obtain a reference to the block any given chunk is located on. AllocSet
is able to find the context to which the chunk is owned, by first
obtaining a reference to the block by subtracting the block offset as is
stored in the 'hdrmask' field and then referencing the block's 'aset'
field. The Generation context uses the same method, but GenerationBlock
did not have a field pointing back to the owning context, so one is added
by this commit.
In aset.c and generation.c, all allocations larger than allocChunkLimit
are stored on dedicated blocks. When there's just a single chunk on a
block like this, it's easy to find the block from the chunk, we just
subtract the size of the block header from the chunk pointer. The size of
these chunks is also known as we store the endptr on the block, so we can
just subtract the pointer to the allocated memory from that. Because we
can easily find the owning block and the size of the chunk for these
dedicated blocks, we just always use external chunks for allocation sizes
larger than allocChunkLimit. For generation.c, this sidesteps the problem
of non-external MemoryChunks being unable to represent chunk sizes >= 1GB.
This is less of a problem for aset.c as we store the free list index in
the MemoryChunk's spare 30-bit field (the value of which will never be
close to using all 30-bits). We can easily reverse engineer the chunk size
from this when needed. Storing this saves AllocSetFree() from having to
make a call to AllocSetFreeIndex() to determine which free list to put the
newly freed chunk on.
For the slab allocator, this commit adds a new restriction that slab
chunks cannot be >= 1GB in size. If there happened to be any users of
slab.c which used chunk sizes this large, they really should be using
AllocSet instead.
Here we also add a restriction that normal non-dedicated blocks cannot be
1GB or larger. It's now not possible to pass a 'maxBlockSize' >= 1GB
during the creation of an AllocSet or Generation context. Allocations can
still be larger than 1GB, it's just these will always be on dedicated
blocks (which do not have the 1GB restriction).
Author: Andres Freund, David Rowley
Discussion: https://postgr.es/m/CAApHDvpjauCRXcgcaL6+e3eqecEHoeRm9D-kcbuvBitgPnW=vw@mail.gmail.com
Amit Kapila [Mon, 29 Aug 2022 02:40:10 +0000 (08:10 +0530)]
Fix the incorrect assertion introduced in commit
7f13ac8123.
It has been incorrectly assumed in commit
7f13ac8123 that we can either
purge all or none in the catalog modifying xids list retrieved from a
serialized snapshot. It is quite possible that some of the xids in that
array are old enough to be pruned but not others.
As per buildfarm
Author: Amit Kapila and Masahiko Sawada
Reviwed-by: Masahiko Sawada
Discussion: https://postgr.es/m/CAA4eK1LBtv6ayE+TvCcPmC-xse=DVg=SmbyQD1nv_AaqcpUJEg@mail.gmail.com
Tom Lane [Sun, 28 Aug 2022 14:44:52 +0000 (10:44 -0400)]
Doc: fix example of recursive query.
Compute total number of sub-parts correctly, per jason@banfelder.net
Simon Riggs
Discussion: https://postgr.es/m/
166161184718.
1235920.
6304070286124217754@wrigleys.postgresql.org
Peter Eisentraut [Sun, 28 Aug 2022 07:55:04 +0000 (09:55 +0200)]
Add more detail why repalloc and pfree do not accept NULL pointers
Per discussion, we choose not to change this. This just gives a
little bit more information.
Discussion: https://www.postgresql.org/message-id/flat/
cf26e970-8e92-59f1-247a-
aa265235075b%40enterprisedb.com
Michael Paquier [Sun, 28 Aug 2022 07:04:58 +0000 (16:04 +0900)]
Enable RandomizedBaseAddress (ASLR) on Windows with MSVC builds
This has as effect to add /DYNAMICBASE to the .dll and .exe files
generated by the builds, undoing
7f3e17b. Note that ASLR was already
enabled in MinGW as we have never added --disable-dynamicbase there.
This change will ease a bit the integration of arm64 with MSVC, as ASLR
support is mandatory in this case. So, thanks to this commit, we have
no need to make ASLR conditional depending on the architecture used for
the build.
Andres Freund has done a lot of testing with this option while working
on meson, without seeing /DYNAMICBASE as being a problem in the Windows
builds of the CI. Personally, not supporting anything older than
Windows 10 on HEAD makes me feel safer about this change, as we have
seen ASLR with being a problem in process invocation particularly with
Windows 8 and server 2012 back in 2014, even if Windows 10 was not
really a thing back then.
45e004f is also something that can help in
making the process invocation more stable. We are very early in the
development of Postgres 16, giving a lot of room to detect stability
issues if any.
Discussion: https://postgr.es/m/
20220826012907.gjw3jdqdgsts5y65@awork3.anarazel.de
Tom Lane [Sat, 27 Aug 2022 16:52:39 +0000 (12:52 -0400)]
Avoid casting away const in sepgsql's quote_object_name.
quote_identifier's API is designed on the assumption that it's
not worth worrying about a short-term memory leak when we have
to produce a quoted version of the given identifier. Whoever wrote
quote_object_name took it on themselves to override that judgment,
but the only way to do so is to cast away const someplace. We can
avoid that and substantially shorten the function by going along
with quote_identifier's opinion. AFAICS quote_object_name is not
used in any way where this would be unsustainable.
Per discussion of commit
45987aae2, which exposed that we had
a casting-away-const situation here.
Discussion: https://postgr.es/m/
20220827112304.GL2342@telsasoft.com
Tom Lane [Sat, 27 Aug 2022 16:16:21 +0000 (12:16 -0400)]
Doc: add comment about bug fixed in back branches as of
3f7323cbb.
While the bug I just fixed in the back branches doesn't exist in
HEAD, the requirement that MULTIEXPR SubPlans not share output
parameters still does. Add a comment to memorialize that, because
perhaps it could be an issue again someday.
Discussion: https://postgr.es/m/17596-
c5357f61427a81dc@postgresql.org
Alexander Korotkov [Sat, 27 Aug 2022 11:46:15 +0000 (14:46 +0300)]
Fix typo in comment for writetuple() function
Reported-by: David Rowley
Discussion: https://postgr.es/m/CAApHDvrZ9Ky2LcWwcKsbdYChA850JE5qS%3DkGJiTNWS8mbBXZHw%40mail.gmail.com
John Naylor [Sat, 27 Aug 2022 04:17:36 +0000 (11:17 +0700)]
Be more careful to avoid including system headers after perl.h
Commit
121d2d3d70 included simd.h into pg_wchar.h. This caused a problem
on Windows, since Perl has "#define free" (referring to globals), which
breaks the Windows' header. To fix, move the static inline function
definitions from plperl_helpers.h, into plperl.h, where we already
document the necessary inclusion order. Since those functions were the
only reason for the existence of plperl_helpers.h, remove it.
First reported by Justin Pryzby
Diagnosis and review by Andres Freund, patch by myself per suggestion
from Tom Lane
Discussion: https://www.postgresql.org/message-id/
20220826115546.GE2342%40telsasoft.com
Michael Paquier [Sat, 27 Aug 2022 06:21:31 +0000 (15:21 +0900)]
Use correct connection for cancellation in frontend's parallel slots
While waiting for slots to become available in wait_on_slots() in
parallel_slot.c, the cancellation always relied on the first connection
in the set to do the job. This could cause problems when this slot's
socket is gone as PQgetCancel() would return NULL in this case. Rather
than always using the first connection, this changes the logic to use
the first valid connection for the cancellation.
Author: Ranier Vilela
Reviewed-by: Justin Pryzby
Discussion: https://postgr.es/m/CAEudQAokk1h_pUwGXsYS4oVOuf35s1O2o3TXGHpV8=AWikvgHA@mail.gmail.com
Backpatch-through: 14
Peter Eisentraut [Fri, 26 Aug 2022 17:16:28 +0000 (19:16 +0200)]
Remove unneeded null pointer checks before PQfreemem()
PQfreemem() just calls free(), and the latter already checks for null
pointers.
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/flat/
cf26e970-8e92-59f1-247a-
aa265235075b%40enterprisedb.com
Peter Eisentraut [Fri, 26 Aug 2022 13:55:57 +0000 (15:55 +0200)]
Remove unnecessary casts in free() and pfree()
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/flat/
cf26e970-8e92-59f1-247a-
aa265235075b%40enterprisedb.com