* Re: [edk2-devel] [Patch V2] BaseTools: Add a checking for Sources section in INF file
[not found] <15A15B842F0124CD.29177@groups.io>
@ 2019-05-23 16:13 ` Christian Rodriguez
0 siblings, 0 replies; only message in thread
From: Christian Rodriguez @ 2019-05-23 16:13 UTC (permalink / raw)
To: devel@edk2.groups.io, Rodriguez, Christian
Cc: Feng, Bob C, Gao, Liming, Zhu, Yonghong
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: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
>Christian Rodriguez
>Sent: Thursday, May 23, 2019 9:06 AM
>To: devel@edk2.groups.io
>Cc: Feng, Bob C <bob.c.feng@intel.com>; Gao, Liming
><liming.gao@intel.com>; Zhu, Yonghong <yonghong.zhu@intel.com>
>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 <christian.rodriguez@intel.com>
>Cc: Bob Feng <bob.c.feng@intel.com>
>Cc: Liming Gao <liming.gao@intel.com>
>Cc: Yonghong Zhu <yonghong.zhu@intel.com>
>---
> 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
>
>
>
^ permalink raw reply [flat|nested] only message in thread