Skip to content

Commit bb9c37f

Browse files
ashutosh-bapatCommitfest Bot
authored andcommitted
Refactor CalculateShmemSize()
This function calls many functions which return the amount of shared memory required for different shared memory data structures. Up until now, the returned total of these sizes was used to create a single shared memory segment. But starting the previous patch, we create multiple shared memory segments each of which contain one shared memory structure related to shared buffers and one main memory segment containing rest of the structures. Since CalculateShmemSize() is called for every shared memory segment, and its return value is added to the memory required for all the shared memory segments, we end up allocating more memory than required. Instead, CalculateShmemSize() is called only once. Each of its callees are expected to a. return the size required from the main segment b. add sizes to the AnonymousMappings corresponding to the other memory segments. For individual modules to add memory to their respective AnonymousMappings, we need to know the different mappings upfront. Hence ANON_MAPPINGS replaces next_free_segment. TODOs: 1. This change however requires that the AnonymousMappings array and macros defining identifiers of each of the segments be platform-independent. This patch doesn't achieve that goal for all the platforms for example windows. We need to fix that. 2. If postgres is invoked with -C shared_memory_size, it reports 0. That's because it report the GUC values before share memory sizes are set in AnonymousMappings. Fix that too. 3. Eliminate this assymetry in CalculateShmemSize(). See TODO in prologue of CalculateShmemSize(). 4. This is one way to avoid requesting more memory in each segment. But there may be other ways to design CalculateShmemSize(). Need to think and implement it better. Author: Ashutosh Bapat
1 parent 8115712 commit bb9c37f

File tree

10 files changed

+125
-107
lines changed

10 files changed

+125
-107
lines changed

src/backend/port/sysv_shmem.c

Lines changed: 13 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -94,21 +94,7 @@ typedef enum
9494
unsigned long UsedShmemSegID = 0;
9595
void *UsedShmemSegAddr = NULL;
9696

97-
typedef struct AnonymousMapping
98-
{
99-
int shmem_segment;
100-
Size shmem_size; /* Size of the actually used memory */
101-
Size shmem_reserved; /* Size of the reserved mapping */
102-
Pointer shmem; /* Pointer to the start of the mapped memory */
103-
Pointer seg_addr; /* SysV shared memory for the header */
104-
unsigned long seg_id; /* IPC key */
105-
int segment_fd; /* fd for the backing anon file */
106-
} AnonymousMapping;
107-
108-
static AnonymousMapping Mappings[ANON_MAPPINGS];
109-
110-
/* Keeps track of used mapping segments */
111-
static int next_free_segment = 0;
97+
AnonymousMapping Mappings[ANON_MAPPINGS];
11298

11399
/*
114100
* Anonymous mapping layout we use looks like this:
@@ -168,7 +154,7 @@ static IpcMemoryState PGSharedMemoryAttach(IpcMemoryId shmId,
168154
void *attachAt,
169155
PGShmemHeader **addr);
170156

171-
static const char*
157+
const char*
172158
MappingName(int shmem_segment)
173159
{
174160
switch (shmem_segment)
@@ -193,7 +179,7 @@ MappingName(int shmem_segment)
193179
static void
194180
DebugMappings()
195181
{
196-
for(int i = 0; i < next_free_segment; i++)
182+
for(int i = 0; i < ANON_MAPPINGS; i++)
197183
{
198184
AnonymousMapping m = Mappings[i];
199185
elog(DEBUG1, "Mapping[%s]: addr %p, size %zu",
@@ -851,9 +837,6 @@ PrepareHugePages()
851837
{
852838
void *ptr = MAP_FAILED;
853839

854-
/* Reset to handle reinitialization */
855-
next_free_segment = 0;
856-
857840
/* Complain if hugepages demanded but we can't possibly support them */
858841
#if !defined(MAP_HUGETLB)
859842
if (huge_pages == HUGE_PAGES_ON)
@@ -876,8 +859,7 @@ PrepareHugePages()
876859
*/
877860
for(int segment = 0; segment < ANON_MAPPINGS; segment++)
878861
{
879-
int numSemas;
880-
Size segment_size = CalculateShmemSize(&numSemas, segment);
862+
Size segment_size = Mappings[segment].shmem_req_size;
881863

882864
if (segment_size % hugepagesize != 0)
883865
segment_size += hugepagesize - (segment_size % hugepagesize);
@@ -909,7 +891,7 @@ PrepareHugePages()
909891
static void
910892
AnonymousShmemDetach(int status, Datum arg)
911893
{
912-
for(int i = 0; i < next_free_segment; i++)
894+
for(int i = 0; i < ANON_MAPPINGS; i++)
913895
{
914896
AnonymousMapping m = Mappings[i];
915897

@@ -927,7 +909,7 @@ AnonymousShmemDetach(int status, Datum arg)
927909
/*
928910
* PGSharedMemoryCreate
929911
*
930-
* Create a shared memory segment of the given size and initialize its
912+
* Create a shared memory segment for the given mapping and initialize its
931913
* standard header. Also, register an on_shmem_exit callback to release
932914
* the storage.
933915
*
@@ -937,15 +919,14 @@ AnonymousShmemDetach(int status, Datum arg)
937919
* postmaster or backend.
938920
*/
939921
PGShmemHeader *
940-
PGSharedMemoryCreate(Size size,
922+
PGSharedMemoryCreate(AnonymousMapping *mapping,
941923
PGShmemHeader **shim)
942924
{
943925
IpcMemoryKey NextShmemSegID;
944926
void *memAddress;
945927
PGShmemHeader *hdr;
946928
struct stat statbuf;
947929
Size sysvsize, total_reserved;
948-
AnonymousMapping *mapping = &Mappings[next_free_segment];
949930

950931
/*
951932
* We use the data directory's ID info (inode and device numbers) to
@@ -965,13 +946,12 @@ PGSharedMemoryCreate(Size size,
965946
errmsg("huge pages not supported with the current \"shared_memory_type\" setting")));
966947

967948
/* Room for a header? */
968-
Assert(size > MAXALIGN(sizeof(PGShmemHeader)));
949+
Assert(mapping->shmem_req_size > MAXALIGN(sizeof(PGShmemHeader)));
969950

970951
/* Prepare the mapping information */
971-
mapping->shmem_size = size;
972-
mapping->shmem_segment = next_free_segment;
952+
mapping->shmem_size = mapping->shmem_req_size;
973953
total_reserved = (Size) MaxAvailableMemory * BLCKSZ;
974-
mapping->shmem_reserved = total_reserved * SHMEM_RESIZE_RATIO[next_free_segment];
954+
mapping->shmem_reserved = total_reserved * SHMEM_RESIZE_RATIO[mapping->shmem_segment];
975955

976956
/* Round up to be a multiple of BLCKSZ */
977957
mapping->shmem_reserved = mapping->shmem_reserved + BLCKSZ -
@@ -982,8 +962,6 @@ PGSharedMemoryCreate(Size size,
982962
/* On success, mapping data will be modified. */
983963
CreateAnonymousSegment(mapping);
984964

985-
next_free_segment++;
986-
987965
/* Register on-exit routine to unmap the anonymous segment */
988966
on_shmem_exit(AnonymousShmemDetach, (Datum) 0);
989967

@@ -992,7 +970,7 @@ PGSharedMemoryCreate(Size size,
992970
}
993971
else
994972
{
995-
sysvsize = size;
973+
sysvsize = mapping->shmem_req_size;
996974

997975
/* huge pages are only available with mmap */
998976
SetConfigOption("huge_pages_status", "off",
@@ -1005,7 +983,7 @@ PGSharedMemoryCreate(Size size,
1005983
* loop simultaneously. (CreateDataDirLockFile() does not entirely ensure
1006984
* that, but prefer fixing it over coping here.)
1007985
*/
1008-
NextShmemSegID = statbuf.st_ino + next_free_segment;
986+
NextShmemSegID = statbuf.st_ino + mapping->shmem_segment;
1009987

1010988
for (;;)
1011989
{
@@ -1214,7 +1192,7 @@ PGSharedMemoryNoReAttach(void)
12141192
void
12151193
PGSharedMemoryDetach(void)
12161194
{
1217-
for(int i = 0; i < next_free_segment; i++)
1195+
for(int i = 0; i < ANON_MAPPINGS; i++)
12181196
{
12191197
AnonymousMapping m = Mappings[i];
12201198

src/backend/port/win32_shmem.c

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ EnableLockPagesPrivilege(int elevel)
204204
* standard header.
205205
*/
206206
PGShmemHeader *
207-
PGSharedMemoryCreate(Size size,
207+
PGSharedMemoryCreate(AnonymousMapping *mapping,
208208
PGShmemHeader **shim)
209209
{
210210
void *memAddress;
@@ -216,7 +216,7 @@ PGSharedMemoryCreate(Size size,
216216
DWORD size_high;
217217
DWORD size_low;
218218
SIZE_T largePageSize = 0;
219-
Size orig_size = size;
219+
Size size = mapping->shmem_req_size;
220220
DWORD flProtect = PAGE_READWRITE;
221221
DWORD desiredAccess;
222222

@@ -304,7 +304,7 @@ PGSharedMemoryCreate(Size size,
304304
* Use the original size, not the rounded-up value, when
305305
* falling back to non-huge pages.
306306
*/
307-
size = orig_size;
307+
size = mapping->shmem_req_size;
308308
flProtect = PAGE_READWRITE;
309309
goto retry;
310310
}
@@ -391,6 +391,7 @@ PGSharedMemoryCreate(Size size,
391391
hdr->totalsize = size;
392392
hdr->freeoffset = MAXALIGN(sizeof(PGShmemHeader));
393393
hdr->dsm_control = 0;
394+
mapping->shmem_size = size;
394395

395396
/* Save info for possible future use */
396397
UsedShmemSegAddr = memAddress;

src/backend/postmaster/postmaster.c

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -963,13 +963,6 @@ PostmasterMain(int argc, char *argv[])
963963
*/
964964
process_shmem_requests();
965965

966-
/*
967-
* Now that loadable modules have had their chance to request additional
968-
* shared memory, determine the value of any runtime-computed GUCs that
969-
* depend on the amount of shared memory required.
970-
*/
971-
InitializeShmemGUCs();
972-
973966
/*
974967
* Now that modules have been loaded, we can process any custom resource
975968
* managers specified in the wal_consistency_checking GUC.
@@ -1005,6 +998,13 @@ PostmasterMain(int argc, char *argv[])
1005998
*/
1006999
CreateSharedMemoryAndSemaphores();
10071000

1001+
/*
1002+
* Now that loadable modules have had their chance to request additional
1003+
* shared memory, determine the value of any runtime-computed GUCs that
1004+
* depend on the amount of shared memory required.
1005+
*/
1006+
InitializeShmemGUCs();
1007+
10081008
/*
10091009
* Estimate number of openable files. This must happen after setting up
10101010
* semaphores, because on some platforms semaphores count as open files.

src/backend/storage/buffer/buf_init.c

Lines changed: 19 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -158,48 +158,31 @@ BufferManagerShmemInit(void)
158158
* data.
159159
*/
160160
Size
161-
BufferManagerShmemSize(int shmem_segment)
161+
BufferManagerShmemSize(void)
162162
{
163-
Size size = 0;
163+
size_t size;
164164

165-
if (shmem_segment == MAIN_SHMEM_SEGMENT)
166-
return size;
165+
/* size of buffer descriptors, plus alignment padding */
166+
size = add_size(0, mul_size(NBuffers, sizeof(BufferDescPadded)));
167+
size = add_size(size, PG_CACHE_LINE_SIZE);
168+
Mappings[BUFFER_DESCRIPTORS_SHMEM_SEGMENT].shmem_req_size = size;
167169

168-
if (shmem_segment == BUFFER_DESCRIPTORS_SHMEM_SEGMENT)
169-
{
170-
/* size of buffer descriptors */
171-
size = add_size(size, mul_size(NBuffers, sizeof(BufferDescPadded)));
172-
/* to allow aligning buffer descriptors */
173-
size = add_size(size, PG_CACHE_LINE_SIZE);
174-
}
170+
/* size of data pages, plus alignment padding */
171+
size = add_size(0, PG_IO_ALIGN_SIZE);
172+
size = add_size(size, mul_size(NBuffers, BLCKSZ));
173+
Mappings[BUFFERS_SHMEM_SEGMENT].shmem_req_size = size;
175174

176-
if (shmem_segment == BUFFERS_SHMEM_SEGMENT)
177-
{
178-
/* size of data pages, plus alignment padding */
179-
size = add_size(size, PG_IO_ALIGN_SIZE);
180-
size = add_size(size, mul_size(NBuffers, BLCKSZ));
181-
}
175+
/* size of stuff controlled by freelist.c */
176+
Mappings[STRATEGY_SHMEM_SEGMENT].shmem_req_size = StrategyShmemSize();
182177

183-
if (shmem_segment == STRATEGY_SHMEM_SEGMENT)
184-
{
185-
/* size of stuff controlled by freelist.c */
186-
size = add_size(size, StrategyShmemSize());
187-
}
178+
/* size of I/O condition variables, plus alignment padding */
179+
size = add_size(0, mul_size(NBuffers,
180+
sizeof(ConditionVariableMinimallyPadded)));
181+
size = add_size(size, PG_CACHE_LINE_SIZE);
182+
Mappings[BUFFER_IOCV_SHMEM_SEGMENT].shmem_req_size = size;
188183

189-
if (shmem_segment == BUFFER_IOCV_SHMEM_SEGMENT)
190-
{
191-
/* size of I/O condition variables */
192-
size = add_size(size, mul_size(NBuffers,
193-
sizeof(ConditionVariableMinimallyPadded)));
194-
/* to allow aligning the above */
195-
size = add_size(size, PG_CACHE_LINE_SIZE);
196-
}
197-
198-
if (shmem_segment == CHECKPOINT_BUFFERS_SHMEM_SEGMENT)
199-
{
200-
/* size of checkpoint sort array in bufmgr.c */
201-
size = add_size(size, mul_size(NBuffers, sizeof(CkptSortItem)));
202-
}
184+
/* size of checkpoint sort array in bufmgr.c */
185+
Mappings[CHECKPOINT_BUFFERS_SHMEM_SEGMENT].shmem_req_size = mul_size(NBuffers, sizeof(CkptSortItem));
203186

204187
return size;
205188
}

0 commit comments

Comments
 (0)