The performance impact seems to be irrelevant. After a few runs both on master and with the change using: build -p OvmfPkg\OvmfPkgIa32.dsc -a IA32 -t VS2015x86
I got an average build time of: No Change, Master build time Avg: 00:01:34 Header check change build time Avg: 00:01:34 Thanks, Christian >-----Original Message----- >From: [email protected] [mailto:[email protected]] On Behalf Of >Christian Rodriguez >Sent: Thursday, May 23, 2019 9:06 AM >To: [email protected] >Cc: Feng, Bob C <[email protected]>; Gao, Liming ><[email protected]>; Zhu, Yonghong <[email protected]> >Subject: [edk2-devel] [Patch V2] BaseTools: Add a checking for Sources >section in INF file > >BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1804 > >In V2: Enable check for all builds, move conditional to hash invalidation In >the >Edk2 INF spec 3.9, it states, All HII Unicode format files must be listed in >[Sources] section. Add a check to see if [Sources] section lists all the >"source" >type files of a module. Performance impact should be minimal with this patch >since information is already being fetched for Makefile purposes. All other >information is already cached in memory. No extra IO time is needed. > >Signed-off-by: Christian Rodriguez <[email protected]> >Cc: Bob Feng <[email protected]> >Cc: Liming Gao <[email protected]> >Cc: Yonghong Zhu <[email protected]> >--- > BaseTools/Source/Python/AutoGen/AutoGen.py | 6 +- > BaseTools/Source/Python/AutoGen/GenMake.py | 44 +++++++++++++ > BaseTools/Source/Python/Common/GlobalData.py | 3 +- > BaseTools/Source/Python/build/build.py | 65 ++++++++++++-------- > 4 files changed, 91 insertions(+), 27 deletions(-) > >diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py >b/BaseTools/Source/Python/AutoGen/AutoGen.py >index a843f294b9..0bc27fb2b3 100644 >--- a/BaseTools/Source/Python/AutoGen/AutoGen.py >+++ b/BaseTools/Source/Python/AutoGen/AutoGen.py >@@ -3989,7 +3989,8 @@ class ModuleAutoGen(AutoGen): > for LibraryAutoGen in self.LibraryAutoGenList: > LibraryAutoGen.CreateMakeFile() > >- if self.CanSkip(): >+ # Don't enable if hash feature enabled, CanSkip uses timestamps to >determine build skipping >+ if not GlobalData.gUseHashCache and self.CanSkip(): > return > > if len(self.CustomMakefile) == 0: >@@ -4032,7 +4033,8 @@ class ModuleAutoGen(AutoGen): > for LibraryAutoGen in self.LibraryAutoGenList: > LibraryAutoGen.CreateCodeFile() > >- if self.CanSkip(): >+ # Don't enable if hash feature enabled, CanSkip uses timestamps to >determine build skipping >+ if not GlobalData.gUseHashCache and self.CanSkip(): > return > > AutoGenList = [] >diff --git a/BaseTools/Source/Python/AutoGen/GenMake.py >b/BaseTools/Source/Python/AutoGen/GenMake.py >index 0e0f9fd9b0..212ca0fa7f 100644 >--- a/BaseTools/Source/Python/AutoGen/GenMake.py >+++ b/BaseTools/Source/Python/AutoGen/GenMake.py >@@ -905,6 +905,50 @@ cleanlib: > ForceIncludedFile, > self._AutoGenObject.IncludePathList + >self._AutoGenObject.BuildOptionIncPathList > ) >+ >+ # Check if header files are listed in metafile >+ # Get a list of unique module header source files from MetaFile >+ headerFilesInMetaFileSet = set() >+ for aFile in self._AutoGenObject.SourceFileList: >+ aFileName = str(aFile) >+ if not aFileName.endswith('.h'): >+ continue >+ headerFilesInMetaFileSet.add(aFileName.lower()) >+ >+ # Get a list of unique module autogen files >+ localAutoGenFileSet = set() >+ for aFile in self._AutoGenObject.AutoGenFileList: >+ localAutoGenFileSet.add(str(aFile).lower()) >+ >+ # Get a list of unique module dependency header files >+ # Exclude autogen files and files not in the source directory >+ headerFileDependencySet = set() >+ localSourceDir = str(self._AutoGenObject.SourceDir).lower() >+ for Dependency in FileDependencyDict.values(): >+ for aFile in Dependency: >+ aFileName = str(aFile).lower() >+ if not aFileName.endswith('.h'): >+ continue >+ if aFileName in localAutoGenFileSet: >+ continue >+ if localSourceDir not in aFileName: >+ continue >+ headerFileDependencySet.add(aFileName) >+ >+ # Ensure that gModuleBuildTracking has been initialized per >architecture >+ if self._AutoGenObject.Arch not in GlobalData.gModuleBuildTracking: >+ GlobalData.gModuleBuildTracking[self._AutoGenObject.Arch] = >+ dict() >+ >+ # Check if a module dependency header file is missing from the >module's >MetaFile >+ for aFile in headerFileDependencySet: >+ if aFile in headerFilesInMetaFileSet: >+ continue >+ if GlobalData.gUseHashCache: >+ >GlobalData.gModuleBuildTracking[self._AutoGenObject.Arch][self._AutoGen >Object] = 'FAIL_METAFILE' >+ EdkLogger.warn("build","Module MetaFile [Sources] is missing local >header!", >+ ExtraData = "Local Header: " + aFile + " not found in >" + >self._AutoGenObject.MetaFile.Path >+ ) >+ > DepSet = None > for File,Dependency in FileDependencyDict.items(): > if not Dependency: >diff --git a/BaseTools/Source/Python/Common/GlobalData.py >b/BaseTools/Source/Python/Common/GlobalData.py >index 95e28a988f..bd45a43728 100644 >--- a/BaseTools/Source/Python/Common/GlobalData.py >+++ b/BaseTools/Source/Python/Common/GlobalData.py >@@ -110,7 +110,8 @@ gEnableGenfdsMultiThread = False >gSikpAutoGenCache = set() > > # Dictionary for tracking Module build status as success or failure -# False > -> >Fail : True -> Success >+# Top Dict: Key: Arch Type Value: Dictionary >+# Second Dict: Key: AutoGen Obj Value: 'SUCCESS'\'FAIL'\'FAIL_METAFILE' > gModuleBuildTracking = dict() > > # Dictionary of booleans that dictate whether a module or diff --git >a/BaseTools/Source/Python/build/build.py >b/BaseTools/Source/Python/build/build.py >index a8b4ce7375..d147e02dc6 100644 >--- a/BaseTools/Source/Python/build/build.py >+++ b/BaseTools/Source/Python/build/build.py >@@ -625,8 +625,16 @@ class BuildTask: > BuildTask._ErrorFlag.set() > BuildTask._ErrorMessage = "%s broken\n %s [%s]" % \ > (threading.currentThread().getName(), > Command, >WorkingDir) >- if self.BuildItem.BuildObject in GlobalData.gModuleBuildTracking and >not >BuildTask._ErrorFlag.isSet(): >- GlobalData.gModuleBuildTracking[self.BuildItem.BuildObject] = True >+ >+ # Set the value used by hash invalidation flow in >GlobalData.gModuleBuildTracking to 'SUCCESS' >+ # If Module or Lib is being tracked, it did not fail header check >test, and >built successfully >+ if (self.BuildItem.BuildObject.Arch in GlobalData.gModuleBuildTracking >and >+ self.BuildItem.BuildObject in >GlobalData.gModuleBuildTracking[self.BuildItem.BuildObject.Arch] and >+ >GlobalData.gModuleBuildTracking[self.BuildItem.BuildObject.Arch][self.BuildI >tem.BuildObject] != 'FAIL_METAFILE' and >+ not BuildTask._ErrorFlag.isSet() >+ ): >+ >GlobalData.gModuleBuildTracking[self.BuildItem.BuildObject.Arch][self.BuildI >tem.BuildObject] = 'SUCCESS' >+ > # indicate there's a thread is available for another build task > BuildTask._RunningQueueLock.acquire() > BuildTask._RunningQueue.pop(self.BuildItem) >@@ -1154,27 +1162,30 @@ class Build(): > # > # > def invalidateHash(self): >- # GlobalData.gModuleBuildTracking contains only modules that cannot >be skipped by hash >- for moduleAutoGenObj in GlobalData.gModuleBuildTracking.keys(): >- # False == FAIL : True == Success >- # Skip invalidating for Successful module builds >- if GlobalData.gModuleBuildTracking[moduleAutoGenObj] == True: >- continue >+ # Only for hashing feature >+ if not GlobalData.gUseHashCache: >+ return >+ >+ # GlobalData.gModuleBuildTracking contains only modules or libs that >cannot be skipped by hash >+ for moduleAutoGenObjArch in GlobalData.gModuleBuildTracking.keys(): >+ for moduleAutoGenObj in >GlobalData.gModuleBuildTracking[moduleAutoGenObjArch].keys(): >+ # Skip invalidating for Successful Module/Lib builds >+ if >GlobalData.gModuleBuildTracking[moduleAutoGenObjArch][moduleAutoGe >nObj] == 'SUCCESS': >+ continue > >- # The module failed to build or failed to start building, from >this point >on >+ # The module failed to build, failed to start building, >+ or failed the header check test from this point on > >- # Remove .hash from build >- if GlobalData.gUseHashCache: >- ModuleHashFile = path.join(moduleAutoGenObj.BuildDir, >moduleAutoGenObj.Name + ".hash") >+ # Remove .hash from build >+ ModuleHashFile = >+ os.path.join(moduleAutoGenObj.BuildDir, moduleAutoGenObj.Name + >+ ".hash") > if os.path.exists(ModuleHashFile): > os.remove(ModuleHashFile) > >- # Remove .hash file from cache >- if GlobalData.gBinCacheDest: >- FileDir = path.join(GlobalData.gBinCacheDest, >moduleAutoGenObj.Arch, moduleAutoGenObj.SourceDir, >moduleAutoGenObj.MetaFile.BaseName) >- HashFile = path.join(FileDir, moduleAutoGenObj.Name + '.hash') >- if os.path.exists(HashFile): >- os.remove(HashFile) >+ # Remove .hash file from cache >+ if GlobalData.gBinCacheDest: >+ FileDir = os.path.join(GlobalData.gBinCacheDest, >moduleAutoGenObj.Arch, moduleAutoGenObj.SourceDir, >moduleAutoGenObj.MetaFile.BaseName) >+ HashFile = os.path.join(FileDir, moduleAutoGenObj.Name + >'.hash') >+ if os.path.exists(HashFile): >+ os.remove(HashFile) > > ## Build a module or platform > # >@@ -1829,9 +1840,11 @@ class Build(): > if self.Target == "genmake": > return True > self.BuildModules.append(Ma) >- # Initialize all modules in tracking to False >(FAIL) >- if Ma not in GlobalData.gModuleBuildTracking: >- GlobalData.gModuleBuildTracking[Ma] = False >+ # Initialize all modules in tracking to 'FAIL' >+ if Ma.Arch not in GlobalData.gModuleBuildTracking: >+ GlobalData.gModuleBuildTracking[Ma.Arch] = >dict() >+ if Ma not in >GlobalData.gModuleBuildTracking[Ma.Arch]: >+ GlobalData.gModuleBuildTracking[Ma.Arch][Ma] >= 'FAIL' > self.AutoGenTime += int(round((time.time() - > AutoGenStart))) > MakeStart = time.time() > for Ma in self.BuildModules: >@@ -1915,6 +1928,7 @@ class Build(): > # Save MAP buffer into MAP file. > # > self._SaveMapFile (MapBuffer, Wa) >+ self.invalidateHash() > > def _GenFfsCmd(self,ArchList): > # convert dictionary of Cmd:(Inf,Arch) @@ -2013,9 +2027,11 @@ class >Build(): > if self.Target == "genmake": > continue > self.BuildModules.append(Ma) >- # Initialize all modules in tracking to False (FAIL) >- if Ma not in GlobalData.gModuleBuildTracking: >- GlobalData.gModuleBuildTracking[Ma] = False >+ # Initialize all modules in tracking to 'FAIL' >+ if Ma.Arch not in GlobalData.gModuleBuildTracking: >+ GlobalData.gModuleBuildTracking[Ma.Arch] = dict() >+ if Ma not in GlobalData.gModuleBuildTracking[Ma.Arch]: >+ GlobalData.gModuleBuildTracking[Ma.Arch][Ma] = >'FAIL' > self.Progress.Stop("done!") > self.AutoGenTime += int(round((time.time() - > AutoGenStart))) > MakeStart = time.time() @@ -2103,6 +2119,7 @@ class > Build(): > # Save MAP buffer into MAP file. > # > self._SaveMapFile(MapBuffer, Wa) >+ self.invalidateHash() > > ## Generate GuidedSectionTools.txt in the FV directories. > # >-- >2.19.1.windows.1 > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#41284): https://edk2.groups.io/g/devel/message/41284 Mute This Topic: https://groups.io/mt/31732696/21656 Group Owner: [email protected] Unsubscribe: https://edk2.groups.io/g/devel/unsub [[email protected]] -=-=-=-=-=-=-=-=-=-=-=-
