Re: [edk2-devel] [PATCH V4] BaseTools: Optimize GenerateByteArrayValue and CollectPlatformGuids APIs

["Yuwei Chen" <yuwei.chen@intel.com>]

Reviewed-by: Yuwei Chen <yuwei.chen@intel.com>

> -----Original Message-----
> From: S, Ashraf Ali <ashraf.ali.s@intel.com>
> Sent: Thursday, February 8, 2024 1:21 AM
> To: devel@edk2.groups.io
> Cc: S, Ashraf Ali <ashraf.ali.s@intel.com>; Chen, Christine
> <yuwei.chen@intel.com>; Rebecca Cran <rebecca@bsdio.com>; Liming Gao
> <gaoliming@byosoft.com.cn>; Feng, Bob C <bob.c.feng@intel.com>; Chan,
> Amy <amy.chan@intel.com>; Chaganty, Rangasai V
> <rangasai.v.chaganty@intel.com>; Solanki, Digant H
> <digant.h.solanki@intel.com>
> Subject: [PATCH V4] BaseTools: Optimize GenerateByteArrayValue and
> CollectPlatformGuids APIs
> 
> During the Incremental build GenerateByteArrayValue used to generate the
> ByteArrayValue even when there is no change in the PCD/VPDs. which is time
> consuming API based on the number of PCD/VPDs and SKU IDs.
> 
> The optimization is that GenerateByteArrayValue is used to store the
> StructuredPcdsData in a JSON file for each of the arch. and during the
> Incremental build this API will check, if there is any change in the Structured
> PCD/VPDs then rest of the flow remains the same.
> if there is no change then it will return the provious build data.
> 
> Flow:
> during the 1st build StructuredPcdsData.json is not exists, StructuredPcdsData
> will be dumped to json file. and it will copy the output.txt as well.
> Note: as the output.txt are different for different Arch, so it will be stored in
> the Arch folder.
> During the Incremental build check if there is any change in Structured
> PCD/VPD. if there is a change in Structured VPD/PCD then recreate the
> StructuredPcdsData.json, and rest of the flow remains same.
> if there is no change in VPD/PCD read the output.txt and return the data
> 
> Unit Test:
> Test1: Modified the Structured Pcds default from DEC file. current flow is
> executing.
> Test2: Override the default value of the PCD from DEC file. current flow is
> executing.
> Test3: Modified/Override the PCD from DSC file. current flow executing
> Test4: Modified/Override the FDF from DSC file. current flow executing
> Test5: update the default value from Command Line.current flow executing
> Test6: Build without change in PCD in DSC, FDF, DEC and Command Line the
> proposed changes will be executing, and the return data remains the same
> with and without the changes.
> Test7: Build with and without modified the include headers of Structured
> PCDs. if there is any change in those Structured PCD header then current flow
> will be executed.
> 
> With these changes it's helping to save around ~2.5min to ~3.5min of
> Incremental build time in my build environment.
> 
> Sample PR: https://github.com/tianocore/edk2-basetools/pull/113
> 
> Cc: Yuwei Chen <yuwei.chen@intel.com>
> Cc: Rebecca Cran <rebecca@bsdio.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Bob Feng <bob.c.feng@intel.com>
> Cc: Amy Chan <amy.chan@intel.com>
> Cc: Sai Chaganty <rangasai.v.chaganty@intel.com>
> Cc: Digant H Solanki <digant.h.solanki@intel.com>
> Signed-off-by: Ashraf Ali S <ashraf.ali.s@intel.com>
> ---
>  .../Source/Python/AutoGen/WorkspaceAutoGen.py |  16 +-
>  .../Source/Python/Workspace/DscBuildData.py   | 181 +++++++++++++-----
>  2 files changed, 139 insertions(+), 58 deletions(-)
> 
> diff --git a/BaseTools/Source/Python/AutoGen/WorkspaceAutoGen.py
> b/BaseTools/Source/Python/AutoGen/WorkspaceAutoGen.py
> index 160e3a3cd3..eec9280c8e 100644
> --- a/BaseTools/Source/Python/AutoGen/WorkspaceAutoGen.py
> +++ b/BaseTools/Source/Python/AutoGen/WorkspaceAutoGen.py
> @@ -160,22 +160,18 @@ class WorkspaceAutoGen(AutoGen):
> 
>      def CollectPlatformGuids(self):
>          oriInfList = []
> -        oriPkgSet = set()
> -        PlatformPkg = set()
> +        pkgSet = set()
>          for Arch in self.ArchList:
>              Platform = self.BuildDatabase[self.MetaFile, Arch, self.BuildTarget,
> self.ToolChain]
>              oriInfList = Platform.Modules
>              for ModuleFile in oriInfList:
>                  ModuleData = self.BuildDatabase[ModuleFile, Platform._Arch,
> Platform._Target, Platform._Toolchain]
> -                oriPkgSet.update(ModuleData.Packages)
> -                for Pkg in oriPkgSet:
> -                    Guids = Pkg.Guids
> -                    GlobalData.gGuidDict.update(Guids)
> +                pkgSet.update(ModuleData.Packages)
>              if Platform.Packages:
> -                PlatformPkg.update(Platform.Packages)
> -                for Pkg in PlatformPkg:
> -                    Guids = Pkg.Guids
> -                    GlobalData.gGuidDict.update(Guids)
> +                pkgSet.update(Platform.Packages)
> +        for Pkg in pkgSet:
> +            Guids = Pkg.Guids
> +            GlobalData.gGuidDict.update(Guids)
> 
>      @cached_property
>      def FdfProfile(self):
> diff --git a/BaseTools/Source/Python/Workspace/DscBuildData.py
> b/BaseTools/Source/Python/Workspace/DscBuildData.py
> index 4768099343..ddcd157c73 100644
> --- a/BaseTools/Source/Python/Workspace/DscBuildData.py
> +++ b/BaseTools/Source/Python/Workspace/DscBuildData.py
> @@ -37,6 +37,8 @@ from functools import reduce  from Common.Misc
> import SaveFileOnChange  from Workspace.BuildClassObject import
> PlatformBuildClassObject, StructurePcd, PcdClassObject,
> ModuleBuildClassObject  from collections import OrderedDict, defaultdict
> +import json
> +import shutil
> 
>  def _IsFieldValueAnArray (Value):
>      Value = Value.strip()
> @@ -56,6 +58,7 @@ def _IsFieldValueAnArray (Value):
> 
>  PcdValueInitName = 'PcdValueInit'
>  PcdValueCommonName = 'PcdValueCommon'
> +StructuredPcdsDataName = 'StructuredPcdsData.json'
> 
>  PcdMainCHeader = '''
>  /**
> @@ -2750,6 +2753,63 @@ class DscBuildData(PlatformBuildClassObject):
>                  ccflags.add(item)
>              i +=1
>          return ccflags
> +
> +    def GetStructurePcdSet (self, OutputValueFile):
> +        if not os.path.isfile(OutputValueFile):
> +            EdkLogger.error("GetStructurePcdSet", FILE_NOT_FOUND, "Output.txt
> doesn't exist", ExtraData=OutputValueFile)
> +            return []
> +        File = open (OutputValueFile, 'r')
> +        FileBuffer = File.readlines()
> +        File.close()
> +
> +        #start update structure pcd final value
> +        StructurePcdSet = []
> +        for Pcd in FileBuffer:
> +            PcdValue = Pcd.split ('|')
> +            PcdInfo = PcdValue[0].split ('.')
> +            StructurePcdSet.append((PcdInfo[0], PcdInfo[1], PcdInfo[2],
> PcdInfo[3], PcdValue[2].strip()))
> +        return StructurePcdSet
> +
> +    def GetBuildOptionsValueList(self):
> +        CC_FLAGS = LinuxCFLAGS
> +        if sys.platform == "win32":
> +            CC_FLAGS = WindowsCFLAGS
> +        BuildOptions = OrderedDict()
> +        for Options in self.BuildOptions:
> +            if Options[2] != EDKII_NAME:
> +                continue
> +            Family = Options[0]
> +            if Family and Family != self.ToolChainFamily:
> +                continue
> +            Target, Tag, Arch, Tool, Attr = Options[1].split("_")
> +            if Tool != 'CC':
> +                continue
> +            if Attr != "FLAGS":
> +                continue
> +            if Target == TAB_STAR or Target == self._Target:
> +                if Tag == TAB_STAR or Tag == self._Toolchain:
> +                    if 'COMMON' not in BuildOptions:
> +                        BuildOptions['COMMON'] = set()
> +                    if Arch == TAB_STAR:
> +                        BuildOptions['COMMON']|=
> self.ParseCCFlags(self.BuildOptions[Options])
> +                    if Arch in self.SupArchList:
> +                        if Arch not in BuildOptions:
> +                            BuildOptions[Arch] = set()
> +                        BuildOptions[Arch] |=
> + self.ParseCCFlags(self.BuildOptions[Options])
> +
> +        if BuildOptions:
> +            ArchBuildOptions = {arch:flags for arch,flags in BuildOptions.items() if
> arch != 'COMMON'}
> +            if len(ArchBuildOptions.keys()) == 1:
> +                BuildOptions['COMMON'] |= (list(ArchBuildOptions.values())[0])
> +            elif len(ArchBuildOptions.keys()) > 1:
> +                CommonBuildOptions = reduce(lambda x,y: x&y,
> ArchBuildOptions.values())
> +                BuildOptions['COMMON'] |= CommonBuildOptions
> +            ValueList = [item for item in BuildOptions['COMMON'] if
> item.startswith((r"/U","-U"))]
> +            ValueList.extend([item for item in BuildOptions['COMMON'] if
> item.startswith((r"/D", "-D"))])
> +            CC_FLAGS += " ".join(ValueList)
> +        return CC_FLAGS
> +
> +
>      def GenerateByteArrayValue (self, StructuredPcds):
>          #
>          # Generate/Compile/Run C application to determine if there are any
> flexible array members @@ -2757,6 +2817,66 @@ class
> DscBuildData(PlatformBuildClassObject):
>          if not StructuredPcds:
>              return
> 
> +        StructuredPcdsData = {}
> +        StoredStructuredPcdObjectPaths = {}
> +        SkipPcdValueInit = False
> +
> +        CC_FLAGS = self.GetBuildOptionsValueList()
> +
> +        for PcdName in StructuredPcds:
> +            Pcd = StructuredPcds[PcdName]
> +            TokenSpaceGuidCName = Pcd.TokenSpaceGuidCName
> +            TokenCName = Pcd.TokenCName
> +
> +            # Create a key using TokenSpaceGuidCName and TokenCName
> +            StructuredPcdsData[f"{TokenSpaceGuidCName}_{TokenCName}"] = {
> +                "DefaultValueFromDec": Pcd.DefaultValueFromDec,
> +                "DefaultValues": Pcd.DefaultValues,
> +                "PcdFieldValueFromComm": Pcd.PcdFieldValueFromComm,
> +                "PcdFieldValueFromFdf": Pcd.PcdFieldValueFromFdf,
> +                "DefaultFromDSC": Pcd.DefaultFromDSC,
> +                "PcdFiledValueFromDscComponent":
> Pcd.PcdFiledValueFromDscComponent
> +            }
> +
> +        # Store the CC Flags
> +        StructuredPcdsData["CC_FLAGS"] = CC_FLAGS
> +        #
> +        # If the output path doesn't exists then create it
> +        #
> +        if not os.path.exists(self.OutputPath):
> +            os.makedirs(self.OutputPath)
> +
> +        StructuredPcdsDataPath = os.path.join(self.OutputPath, self._Arch,
> StructuredPcdsDataName)
> +        PcdRecordOutputValueFile = os.path.join(self.OutputPath,
> + self._Arch, 'Output.txt')
> +
> +        if not os.path.exists(os.path.dirname(StructuredPcdsDataPath)):
> +            os.makedirs(os.path.dirname(StructuredPcdsDataPath))
> +        #
> +        # Check if the StructuredPcdsData.json exists or not
> +        # if exits then it might be a incremental build then check if the
> StructuredPcdsData has been changed or not.
> +        # if changed then proceed further, if not changed then return the stored
> data from earlier build
> +        #
> +        if os.path.isfile(StructuredPcdsDataPath):
> +            with open(StructuredPcdsDataPath, 'r') as file:
> +                StoredStructuredPcdsData = json.load(file)
> +                # OBJECTS will have the modified time, which needs to be checked
> later
> +                StoredStructuredPcdObjectPaths =
> + StoredStructuredPcdsData.pop("OBJECTS", {})
> +
> +                if StructuredPcdsData == StoredStructuredPcdsData:
> +                    SkipPcdValueInit = True
> +                    for filename, file_mtime in
> StoredStructuredPcdObjectPaths.items():
> +                        f_mtime = os.path.getmtime(filename)
> +                        #
> +                        # check if the include_file are modified or not,
> +                        # if modified then generate the PcdValueInit
> +                        #
> +                        if f_mtime != file_mtime:
> +                            SkipPcdValueInit = False
> +                            break
> +
> +        if SkipPcdValueInit:
> +            return self.GetStructurePcdSet(PcdRecordOutputValueFile)
> +
>          InitByteValue = ""
>          CApp = PcdMainCHeader
> 
> @@ -2832,8 +2952,6 @@ class DscBuildData(PlatformBuildClassObject):
> 
>          CApp = CApp + PcdMainCEntry + '\n'
> 
> -        if not os.path.exists(self.OutputPath):
> -            os.makedirs(self.OutputPath)
>          CAppBaseFileName = os.path.join(self.OutputPath, PcdValueInitName)
>          SaveFileOnChange(CAppBaseFileName + '.c', CApp, False)
> 
> @@ -2890,42 +3008,6 @@ class DscBuildData(PlatformBuildClassObject):
>                          IncSearchList.append(inc)
>          MakeApp = MakeApp + '\n'
> 
> -        CC_FLAGS = LinuxCFLAGS
> -        if sys.platform == "win32":
> -            CC_FLAGS = WindowsCFLAGS
> -        BuildOptions = OrderedDict()
> -        for Options in self.BuildOptions:
> -            if Options[2] != EDKII_NAME:
> -                continue
> -            Family = Options[0]
> -            if Family and Family != self.ToolChainFamily:
> -                continue
> -            Target, Tag, Arch, Tool, Attr = Options[1].split("_")
> -            if Tool != 'CC':
> -                continue
> -            if Attr != "FLAGS":
> -                continue
> -            if Target == TAB_STAR or Target == self._Target:
> -                if Tag == TAB_STAR or Tag == self._Toolchain:
> -                    if 'COMMON' not in BuildOptions:
> -                        BuildOptions['COMMON'] = set()
> -                    if Arch == TAB_STAR:
> -                        BuildOptions['COMMON']|=
> self.ParseCCFlags(self.BuildOptions[Options])
> -                    if Arch in self.SupArchList:
> -                        if Arch not in BuildOptions:
> -                            BuildOptions[Arch] = set()
> -                        BuildOptions[Arch] |=
> self.ParseCCFlags(self.BuildOptions[Options])
> -
> -        if BuildOptions:
> -            ArchBuildOptions = {arch:flags for arch,flags in BuildOptions.items() if
> arch != 'COMMON'}
> -            if len(ArchBuildOptions.keys()) == 1:
> -                BuildOptions['COMMON'] |= (list(ArchBuildOptions.values())[0])
> -            elif len(ArchBuildOptions.keys()) > 1:
> -                CommonBuildOptions = reduce(lambda x,y: x&y,
> ArchBuildOptions.values())
> -                BuildOptions['COMMON'] |= CommonBuildOptions
> -            ValueList = [item for item in BuildOptions['COMMON'] if
> item.startswith((r"/U","-U"))]
> -            ValueList.extend([item for item in BuildOptions['COMMON'] if
> item.startswith((r"/D", "-D"))])
> -            CC_FLAGS += " ".join(ValueList)
>          MakeApp += CC_FLAGS
> 
>          if sys.platform == "win32":
> @@ -2946,7 +3028,9 @@ class DscBuildData(PlatformBuildClassObject):
> 
> SearchPathList.append(os.path.normpath(mws.join(GlobalData.gGlobalDefin
> es["EDK_TOOLS_PATH"], "BaseTools/Source/C/Common")))
>          SearchPathList.extend(str(item) for item in IncSearchList)
>          IncFileList = GetDependencyList(IncludeFileFullPaths, SearchPathList)
> +        StructuredPcdsData["OBJECTS"] = {}
>          for include_file in IncFileList:
> +            StructuredPcdsData["OBJECTS"][include_file] =
> + os.path.getmtime(include_file)
>              MakeApp += "$(OBJECTS) : %s\n" % include_file
>          if sys.platform == "win32":
>              PcdValueCommonPath =
> os.path.normpath(mws.join(GlobalData.gGlobalDefines["EDK_TOOLS_PATH"]
> , "Source\C\Common\PcdValueCommon.c"))
> @@ -3042,17 +3126,18 @@ class DscBuildData(PlatformBuildClassObject):
>              if returncode != 0:
>                  EdkLogger.warn('Build', COMMAND_FAILURE, 'Can not collect
> output from command: %s\n%s\n%s\n' % (Command, StdOut, StdErr))
> 
> -        #start update structure pcd final value
> -        File = open (OutputValueFile, 'r')
> -        FileBuffer = File.readlines()
> -        File.close()
> +        #
> +        # In 1st build create the StructuredPcdsData.json
> +        # update the record as PCD Input has been changed if its incremental
> build
> +        #
> +        with open(StructuredPcdsDataPath, 'w') as file:
> +            json.dump(StructuredPcdsData, file, indent=2)
> 
> -        StructurePcdSet = []
> -        for Pcd in FileBuffer:
> -            PcdValue = Pcd.split ('|')
> -            PcdInfo = PcdValue[0].split ('.')
> -            StructurePcdSet.append((PcdInfo[0], PcdInfo[1], PcdInfo[2], PcdInfo[3],
> PcdValue[2].strip()))
> -        return StructurePcdSet
> +        # Copy update output file for each Arch
> +        shutil.copyfile(OutputValueFile, PcdRecordOutputValueFile)
> +
> +        #start update structure pcd final value
> +        return self.GetStructurePcdSet(OutputValueFile)
> 
>      @staticmethod
>      def NeedUpdateOutput(OutputFile, ValueCFile, StructureInput):
> --
> 2.39.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115261): https://edk2.groups.io/g/devel/message/115261
Mute This Topic: https://groups.io/mt/104222804/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-