From: "Michael D Kinney" <michael.d.kinney@intel.com>
To: "devel@edk2.groups.io" <devel@edk2.groups.io>,
"S, Ashraf Ali" <ashraf.ali.s@intel.com>
Cc: "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>,
"Kinney, Michael D" <michael.d.kinney@intel.com>
Subject: Re: [edk2-devel] [PATCH] BaseTools: Optimize GenerateByteArrayValue and CollectPlatformGuids APIs
Date: Sat, 20 Jan 2024 02:05:43 +0000 [thread overview]
Message-ID: <CO1PR11MB4929FA7EFA0E69F3A23C8D38D2772@CO1PR11MB4929.namprd11.prod.outlook.com> (raw)
In-Reply-To: <bd7ae66998dbf42a379d680b5cac4fd16d713dbe.1705478090.git.ashraf.ali.s@intel.com>
Hi Ashraf,
What is captured in the file?
What PCD/VPD changes will invalidate the cache? Just the number and
type of PCD/VPD elements or their default values/sizes?
How was this tested? Were all conditions that invalidate the cache
tested? I ask because incremental build is a very important feature
and if there is any logic error in the cache management of a file like
this, it will cause unexpected behavior and developers will not trust
incremental builds.
Mike
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ashraf
> Ali S
> Sent: Tuesday, January 16, 2024 11:55 PM
> 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: [edk2-devel] [PATCH] 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
> PcdRecordList 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 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 PcdRecordList.json is not exists, PcdRecordList
> 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 PCD/VPD.
> if there is a change in VPD/PCD then recreate the PcdRecordList.json.
> and rest of the flow remains same.
> if there is no change in VPD/PCD read the output.txt and return the data
>
> 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 | 72 +++++++++++++++----
> 2 files changed, 64 insertions(+), 24 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..740b8e22be 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'
> +PcdRecordListName = 'PcdRecordList.json'
>
> PcdMainCHeader = '''
> /**
> @@ -1599,7 +1602,7 @@ class DscBuildData(PlatformBuildClassObject):
> S_pcd_set = DscBuildData.OverrideByComm(S_pcd_set)
>
> # Create a tool to caculate structure pcd value
> - Str_Pcd_Values = self.GenerateByteArrayValue(S_pcd_set)
> + Str_Pcd_Values = self.GenerateByteArrayValue(S_pcd_set,
> RecordList)
>
> if Str_Pcd_Values:
> for (skuname, StoreName, PcdGuid, PcdName, PcdValue) in
> Str_Pcd_Values:
> @@ -2750,12 +2753,61 @@ class DscBuildData(PlatformBuildClassObject):
> ccflags.add(item)
> i +=1
> return ccflags
> - def GenerateByteArrayValue (self, StructuredPcds):
> +
> + 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 GenerateByteArrayValue (self, StructuredPcds, PcdRecordList):
> #
> # Generate/Compile/Run C application to determine if there are
> any flexible array members
> #
> if not StructuredPcds:
> return
> + #
> + # If the output path doesn't exists then create it
> + #
> + if not os.path.exists(self.OutputPath):
> + os.makedirs(self.OutputPath)
> +
> + PcdRecordListPath = os.path.join(self.OutputPath, self._Arch,
> PcdRecordListName)
> + PcdRecordOutputValueFile = os.path.join(self.OutputPath,
> self._Arch, 'Output.txt')
> +
> + if not os.path.exists(os.path.dirname(PcdRecordListPath)):
> + os.makedirs(os.path.dirname(PcdRecordListPath))
> + #
> + # Check if the PcdRecordList.json exists or not
> + # if exits then it might be a incremental build then check if
> the PcdRecord list 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(PcdRecordListPath):
> + with open(PcdRecordListPath, 'r') as file:
> + file_content_str = file.read()
> + if file_content_str:
> + # Use json.loads to convert the string back to a
> list
> + file_content = json.loads(file_content_str)
> + # Check if all PcdRecordList in record_set are
> present in file_content
> + # and if there are no extra PcdRecordList in
> file_content
> + if set(map(tuple, PcdRecordList)) == set(map(tuple,
> file_content)):
> + return
> self.GetStructurePcdSet(PcdRecordOutputValueFile)
> + #
> + # 1st build, create the PcdRecordList.json
> + # update the record as PCD Input has been changed in
> incremental build
> + #
> + with open(PcdRecordListPath, 'w') as file:
> + json.dump(PcdRecordList, file)
>
> InitByteValue = ""
> CApp = PcdMainCHeader
> @@ -2832,8 +2884,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)
>
> @@ -3042,17 +3092,11 @@ 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()
> + # Copy output file for each Arch
> + shutil.copyfile(OutputValueFile, PcdRecordOutputValueFile)
>
> - 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
> + # return 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 (#114108): https://edk2.groups.io/g/devel/message/114108
Mute This Topic: https://groups.io/mt/103781798/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
next prev parent reply other threads:[~2024-01-20 2:05 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-01-17 7:54 [edk2-devel] [PATCH] BaseTools: Optimize GenerateByteArrayValue and CollectPlatformGuids APIs Ashraf Ali S
2024-01-20 2:05 ` Michael D Kinney [this message]
2024-01-21 12:13 ` Ashraf Ali S
2024-01-23 1:27 ` Michael D Kinney
2024-01-23 4:23 ` Feng, Bob C
2024-02-02 11:11 ` Ashraf Ali S
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CO1PR11MB4929FA7EFA0E69F3A23C8D38D2772@CO1PR11MB4929.namprd11.prod.outlook.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox