You've already forked pgbackrest
							
							
				mirror of
				https://github.com/pgbackrest/pgbackrest.git
				synced 2025-10-30 23:37:45 +02:00 
			
		
		
		
	Improve memory usage of unlogged relation detection in manifest build.
This loop was using a lot of memory without freeing it at intervals. Rewrite to use char arrays when possible to reduce memory that needs to be allocated and freed.
This commit is contained in:
		| @@ -40,6 +40,22 @@ | ||||
|                         <p>Fix issue with <id>=</id> character in file or database names.</p> | ||||
|                     </release-item> | ||||
|                 </release-bug-list> | ||||
|  | ||||
|                 <release-improvement-list> | ||||
|                     <release-item> | ||||
|                         <release-item-contributor-list> | ||||
|                             <release-item-ideator id="MannerMan"/> | ||||
|                             <release-item-ideator id="brad.nicholson"/> | ||||
|                             <release-item-reviewer id="cynthia.shang"/> | ||||
|                             <release-item-reviewer id="stephen.frost"/> | ||||
|                             <!-- Actually tester, but we don't have a tag for that yet --> | ||||
|                             <release-item-reviewer id="brad.nicholson"/> | ||||
|                             <release-item-reviewer id="MannerMan"/> | ||||
|                         </release-item-contributor-list> | ||||
|  | ||||
|                         <p>Improve memory usage of unlogged relation detection in manifest build.</p> | ||||
|                     </release-item> | ||||
|                 </release-improvement-list> | ||||
|             </release-core-list> | ||||
|  | ||||
|             <release-doc-list> | ||||
| @@ -8765,6 +8781,11 @@ | ||||
|             <contributor-id type="github">mhagander</contributor-id> | ||||
|         </contributor> | ||||
|  | ||||
|         <contributor id="MannerMan"> | ||||
|             <contributor-name-display>Oscar</contributor-name-display> | ||||
|             <contributor-id type="github">MannerMan</contributor-id> | ||||
|         </contributor> | ||||
|  | ||||
|         <contributor id="marc.cousin"> | ||||
|             <contributor-name-display>Marc Cousin</contributor-name-display> | ||||
|             <contributor-id type="github">marco44</contributor-id> | ||||
|   | ||||
| @@ -982,37 +982,53 @@ manifestNewBuild( | ||||
|             { | ||||
|                 RegExp *relationExp = regExpNew(strNewFmt("^" DB_PATH_EXP "/" RELATION_EXP "$", strZ(buildData.tablespaceId))); | ||||
|                 unsigned int fileIdx = 0; | ||||
|                 const String *lastRelationFileId = NULL; | ||||
|                 char lastRelationFileId[21] = "";                   // Large enough for a 64-bit unsigned integer | ||||
|                 bool lastRelationFileIdUnlogged = false; | ||||
|  | ||||
| #ifdef DEBUG_MEM | ||||
|                 // Record the temp context size before the loop begins | ||||
|                 size_t sizeBegin = memContextSize(memContextCurrent()); | ||||
| #endif | ||||
|  | ||||
|                 while (fileIdx < manifestFileTotal(this)) | ||||
|                 { | ||||
|                     // If this file looks like a relation.  Note that this never matches on _init forks. | ||||
|                     const ManifestFile *file = manifestFile(this, fileIdx); | ||||
|  | ||||
|                     // If this file looks like a relation.  Note that this never matches on _init forks. | ||||
|                     if (regExpMatch(relationExp, file->name)) | ||||
|                     { | ||||
|                         String *fileName = strBase(file->name); | ||||
|                         String *relationFileId = strNew(""); | ||||
|                         // Get the filename (without path) | ||||
|                         const char *fileName = strBaseZ(file->name); | ||||
|                         size_t fileNameSize = strlen(fileName); | ||||
|  | ||||
|                         // Strip off the numeric part of the relation | ||||
|                         for (unsigned int nameIdx = 0; nameIdx < strSize(fileName); nameIdx++) | ||||
|                         { | ||||
|                             char nameChr = strZ(fileName)[nameIdx]; | ||||
|                         char relationFileId[sizeof(lastRelationFileId)]; | ||||
|                         unsigned int nameIdx = 0; | ||||
|  | ||||
|                             if (!isdigit(nameChr)) | ||||
|                         for (; nameIdx < fileNameSize; nameIdx++) | ||||
|                         { | ||||
|                             if (!isdigit(fileName[nameIdx])) | ||||
|                                 break; | ||||
|  | ||||
|                             strCatChr(relationFileId, nameChr); | ||||
|                             relationFileId[nameIdx] = fileName[nameIdx]; | ||||
|                         } | ||||
|  | ||||
|                         relationFileId[nameIdx] = '\0'; | ||||
|  | ||||
|                         // The filename must have characters | ||||
|                         ASSERT(relationFileId[0] != '\0'); | ||||
|  | ||||
|                         // Store the last relation so it does not need to be found everytime | ||||
|                         if (lastRelationFileId == NULL || !strEq(lastRelationFileId, relationFileId)) | ||||
|                         if (strcmp(lastRelationFileId, relationFileId) != 0) | ||||
|                         { | ||||
|                             // Determine if the relation is unlogged | ||||
|                             const String *relationInit = strNewFmt("%s/%s_init", strZ(strPath(file->name)), strZ(relationFileId)); | ||||
|                             lastRelationFileId = relationFileId; | ||||
|                             String *relationInit = strNewFmt( | ||||
|                                 "%.*s%s_init", (int)(strSize(file->name) - fileNameSize), strZ(file->name), relationFileId); | ||||
|                             lastRelationFileIdUnlogged = manifestFileFindDefault(this, relationInit, NULL) != NULL; | ||||
|                             strFree(relationInit); | ||||
|  | ||||
|                             // Save the file id so we don't need to do the lookup next time if if doesn't change | ||||
|                             strcpy(lastRelationFileId, relationFileId); | ||||
|                         } | ||||
|  | ||||
|                         // If relation is unlogged then remove it | ||||
| @@ -1025,6 +1041,11 @@ manifestNewBuild( | ||||
|  | ||||
|                     fileIdx++; | ||||
|                 } | ||||
|  | ||||
| #ifdef DEBUG_MEM | ||||
|                 // Make sure that the temp context did not grow too much during the loop | ||||
|                 ASSERT(memContextSize(memContextCurrent()) - sizeBegin < 256); | ||||
| #endif | ||||
|             } | ||||
|         } | ||||
|         MEM_CONTEXT_TEMP_END(); | ||||
|   | ||||
| @@ -506,6 +506,7 @@ sub run | ||||
|                 my $strTestFlags = | ||||
|                     (($self->{bOptimize} && ($self->{bProfile} || $bPerformance)) ? '-O2' : $strNoOptimizeFlags) . | ||||
|                     (!$self->{bDebugTestTrace} && $self->{bDebug} ? ' -DDEBUG_TEST_TRACE' : '') . | ||||
|                     ' -DDEBUG_MEM' . | ||||
|                     (vmCoverageC($self->{oTest}->{&TEST_VM}) && $self->{bCoverageUnit} ? ' -fprofile-arcs -ftest-coverage' : '') . | ||||
|                     ($self->{oTest}->{&TEST_VM} eq VM_NONE ? '' : " -DTEST_CONTAINER_REQUIRED") . | ||||
|                     ($self->{oTest}->{&TEST_CTESTDEF} ? " $self->{oTest}->{&TEST_CTESTDEF}" : ''); | ||||
|   | ||||
		Reference in New Issue
	
	Block a user