public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [edk2-devel] [PATCH] BaseTools: Optimize GenerateByteArrayValue and CollectPlatformGuids APIs
@ 2024-01-17  7:54 Ashraf Ali S
  2024-01-20  2:05 ` Michael D Kinney
  0 siblings, 1 reply; 6+ messages in thread
From: Ashraf Ali S @ 2024-01-17  7:54 UTC (permalink / raw)
  To: devel
  Cc: Ashraf Ali S, Yuwei Chen, Rebecca Cran, Liming Gao, Bob Feng,
	Amy Chan, Sai Chaganty, Digant H Solanki

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 (#113934): https://edk2.groups.io/g/devel/message/113934
Mute This Topic: https://groups.io/mt/103781798/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [edk2-devel] [PATCH] BaseTools: Optimize GenerateByteArrayValue and CollectPlatformGuids APIs
  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
  2024-01-21 12:13   ` Ashraf Ali S
  0 siblings, 1 reply; 6+ messages in thread
From: Michael D Kinney @ 2024-01-20  2:05 UTC (permalink / raw)
  To: devel@edk2.groups.io, S, Ashraf Ali
  Cc: Chen, Christine, Rebecca Cran, Liming Gao, Feng, Bob C, Chan, Amy,
	Chaganty, Rangasai V, Solanki, Digant H, Kinney, Michael D

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]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [edk2-devel] [PATCH] BaseTools: Optimize GenerateByteArrayValue and CollectPlatformGuids APIs
  2024-01-20  2:05 ` Michael D Kinney
@ 2024-01-21 12:13   ` Ashraf Ali S
  2024-01-23  1:27     ` Michael D Kinney
  0 siblings, 1 reply; 6+ messages in thread
From: Ashraf Ali S @ 2024-01-21 12:13 UTC (permalink / raw)
  To: Kinney, Michael D, devel@edk2.groups.io
  Cc: Chen, Christine, Rebecca Cran, Liming Gao, Feng, Bob C, Chan, Amy,
	Chaganty, Rangasai V, Solanki, Digant H

Hi., @Kinney, Michael D

The main API which is modified in this Patch is GenerateByteArrayValue.

This API is used to return the list of SKUID.TokenSpaceGuid.VpdName|VPD STRUCT|Binary Data which will be stored in Output.txt
Example: SAMPLESKUID.STANDARD.gEfiMdePkgTokenSpaceGuid.SampleVpd|SAMPLE_STRUCT[]|{0x01,0x01,0x05,0x09,0x02}

This VPD/PCD is coming from either the DEC file or the DSC file.

GenerateByteArrayValue API is used to create the PcdValueInit.exe and it's equivalent C and Make File.

When there is no change in DEC or DSC VPD/PCD still it used to do the same process again in the incremental build which is time consuming.
In my project this API is used to take 3min of time with High end server (64Threads) and 3.5Min in Laptop (16 threads with performance mode enabled) only for GenerateByteArrayValue API.

This  patch will check if there are any change in the DEC/DSC VPDs/PCDs2, if there is any change in VPD the current flow is not affect. (it will create the PcdRecordList.json and copy Output.txt for Arch folder with the project. Ex: Build\{Project}Pkg\DEBUG_VS2019\PcdValueInit\{IA32/X64/Any}\PcdRecordList.json and Build\{Project}Pkg\DEBUG_VS2019\PcdValueInit\{IA32/X64/Any}\Output.txt)
If there is no change the DSC/DEC VPDs it will read the data from Output.txt and return the same data. (we will compare the Input structure and PcdRecordList.json) if there is any mismatch then it there change in VPDs if not then no change in VPDs).

Unit testing from my side as follows:
* Build the Firmware.
	* PcdRecordList.json and Output.txt will be created based on the Arch.
* Build the Firmware again without any change.
	* It will read the Output.txt and return the data. (it will match Input PCD/VPD list and compare with existing PcdRecordList.json)
	* I verified the return length and content GenerateByteArrayValue (it's same with and without my changes)
* Build the Firmware again my modifying the VPDs in DSC file
	* Change in VPDs, it will regenerate PcdRecordList.json file
	*  it will create the Exe file and Output.txt
	* New Output.txt will be copied to above path.
* Build the Firmware again by modifying the VPDs in DEC file.
	* Change in VPDs, it will regenerate PcdRecordList.json file
	*  it will create the Exe file and Output.txt
	* New Output.txt will be copied to above path.
* Build the Firmware again without modifying the code.
	* It will read the Output.txt and return the data.
	* I verified the return length and content GenerateByteArrayValue (it's same with and without my changes)


There is no impact on the Boot/cache.
This patch is used to reduce the incremental build time by checking if the VPDs/PCDs are changed or not, if not changed then it will return the previous stored data.

Thanks.,
S, Ashraf Ali

-----Original Message-----
From: Kinney, Michael D <michael.d.kinney@intel.com> 
Sent: Saturday, January 20, 2024 7:36 AM
To: 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

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 (#114124): https://edk2.groups.io/g/devel/message/114124
Mute This Topic: https://groups.io/mt/103781798/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [edk2-devel] [PATCH] BaseTools: Optimize GenerateByteArrayValue and CollectPlatformGuids APIs
  2024-01-21 12:13   ` Ashraf Ali S
@ 2024-01-23  1:27     ` Michael D Kinney
  2024-01-23  4:23       ` Feng, Bob C
  0 siblings, 1 reply; 6+ messages in thread
From: Michael D Kinney @ 2024-01-23  1:27 UTC (permalink / raw)
  To: S, Ashraf Ali, devel@edk2.groups.io
  Cc: Chen, Christine, Rebecca Cran, Liming Gao, Feng, Bob C, Chan, Amy,
	Chaganty, Rangasai V, Solanki, Digant H, Kinney, Michael D

Hi Ashraf,

The PcdValueInit feature is not limited to only PCDs of type VPD.  It is
for any structured PCDs.  Did you test PCDs with all types (e.g. PcdsFixedAtBuild
PcdsPactahbleInModule, PcdsDynamicHii, PcdsDynamicDatabase, PcdsDynamicVpd,
PcdsDynamicExHii, PcdsDynamicExDatabase, PcdsDynamicExVpd)

And did you DSC test cases cover both changes in PCD types and changes in PCD values?

Thanks,

Mike

> -----Original Message-----
> From: S, Ashraf Ali <ashraf.ali.s@intel.com>
> Sent: Sunday, January 21, 2024 4:14 AM
> To: Kinney, Michael D <michael.d.kinney@intel.com>; devel@edk2.groups.io
> 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>
> Subject: RE: [edk2-devel] [PATCH] BaseTools: Optimize
> GenerateByteArrayValue and CollectPlatformGuids APIs
> 
> Hi., @Kinney, Michael D
> 
> The main API which is modified in this Patch is GenerateByteArrayValue.
> 
> This API is used to return the list of SKUID.TokenSpaceGuid.VpdName|VPD
> STRUCT|Binary Data which will be stored in Output.txt
> Example:
> SAMPLESKUID.STANDARD.gEfiMdePkgTokenSpaceGuid.SampleVpd|SAMPLE_STRUCT[]|
> {0x01,0x01,0x05,0x09,0x02}
> 
> This VPD/PCD is coming from either the DEC file or the DSC file.
> 
> GenerateByteArrayValue API is used to create the PcdValueInit.exe and
> it's equivalent C and Make File.
> 
> When there is no change in DEC or DSC VPD/PCD still it used to do the
> same process again in the incremental build which is time consuming.
> In my project this API is used to take 3min of time with High end server
> (64Threads) and 3.5Min in Laptop (16 threads with performance mode
> enabled) only for GenerateByteArrayValue API.
> 
> This  patch will check if there are any change in the DEC/DSC
> VPDs/PCDs2, if there is any change in VPD the current flow is not
> affect. (it will create the PcdRecordList.json and copy Output.txt for
> Arch folder with the project. Ex:
> Build\{Project}Pkg\DEBUG_VS2019\PcdValueInit\{IA32/X64/Any}\PcdRecordLis
> t.json and
> Build\{Project}Pkg\DEBUG_VS2019\PcdValueInit\{IA32/X64/Any}\Output.txt)
> If there is no change the DSC/DEC VPDs it will read the data from
> Output.txt and return the same data. (we will compare the Input
> structure and PcdRecordList.json) if there is any mismatch then it there
> change in VPDs if not then no change in VPDs).
> 
> Unit testing from my side as follows:
> * Build the Firmware.
> 	* PcdRecordList.json and Output.txt will be created based on the
> Arch.
> * Build the Firmware again without any change.
> 	* It will read the Output.txt and return the data. (it will match
> Input PCD/VPD list and compare with existing PcdRecordList.json)
> 	* I verified the return length and content GenerateByteArrayValue
> (it's same with and without my changes)
> * Build the Firmware again my modifying the VPDs in DSC file
> 	* Change in VPDs, it will regenerate PcdRecordList.json file
> 	*  it will create the Exe file and Output.txt
> 	* New Output.txt will be copied to above path.
> * Build the Firmware again by modifying the VPDs in DEC file.
> 	* Change in VPDs, it will regenerate PcdRecordList.json file
> 	*  it will create the Exe file and Output.txt
> 	* New Output.txt will be copied to above path.
> * Build the Firmware again without modifying the code.
> 	* It will read the Output.txt and return the data.
> 	* I verified the return length and content GenerateByteArrayValue
> (it's same with and without my changes)
> 
> 
> There is no impact on the Boot/cache.
> This patch is used to reduce the incremental build time by checking if
> the VPDs/PCDs are changed or not, if not changed then it will return the
> previous stored data.
> 
> Thanks.,
> S, Ashraf Ali
> 
> -----Original Message-----
> From: Kinney, Michael D <michael.d.kinney@intel.com>
> Sent: Saturday, January 20, 2024 7:36 AM
> To: 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
> 
> 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 (#114160): https://edk2.groups.io/g/devel/message/114160
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]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [edk2-devel] [PATCH] BaseTools: Optimize GenerateByteArrayValue and CollectPlatformGuids APIs
  2024-01-23  1:27     ` Michael D Kinney
@ 2024-01-23  4:23       ` Feng, Bob C
  2024-02-02 11:11         ` Ashraf Ali S
  0 siblings, 1 reply; 6+ messages in thread
From: Feng, Bob C @ 2024-01-23  4:23 UTC (permalink / raw)
  To: Kinney, Michael D, S, Ashraf Ali, devel@edk2.groups.io
  Cc: Chen, Christine, Rebecca Cran, Liming Gao, Chan, Amy,
	Chaganty, Rangasai V, Solanki, Digant H

Hi Ashraf,

Besides DSC, PCD value also comes from the build tool command line option, DEC and INF. This patch looks only check the PCD changes from DSC, it's not cover all the cases. 

Thanks,
Bob

-----Original Message-----
From: Kinney, Michael D <michael.d.kinney@intel.com> 
Sent: Tuesday, January 23, 2024 9:28 AM
To: S, Ashraf Ali <ashraf.ali.s@intel.com>; devel@edk2.groups.io
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

Hi Ashraf,

The PcdValueInit feature is not limited to only PCDs of type VPD.  It is for any structured PCDs.  Did you test PCDs with all types (e.g. PcdsFixedAtBuild PcdsPactahbleInModule, PcdsDynamicHii, PcdsDynamicDatabase, PcdsDynamicVpd, PcdsDynamicExHii, PcdsDynamicExDatabase, PcdsDynamicExVpd)

And did you DSC test cases cover both changes in PCD types and changes in PCD values?

Thanks,

Mike

> -----Original Message-----
> From: S, Ashraf Ali <ashraf.ali.s@intel.com>
> Sent: Sunday, January 21, 2024 4:14 AM
> To: Kinney, Michael D <michael.d.kinney@intel.com>; 
> devel@edk2.groups.io
> 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>
> Subject: RE: [edk2-devel] [PATCH] BaseTools: Optimize 
> GenerateByteArrayValue and CollectPlatformGuids APIs
> 
> Hi., @Kinney, Michael D
> 
> The main API which is modified in this Patch is GenerateByteArrayValue.
> 
> This API is used to return the list of 
> SKUID.TokenSpaceGuid.VpdName|VPD
> STRUCT|Binary Data which will be stored in Output.txt
> Example:
> SAMPLESKUID.STANDARD.gEfiMdePkgTokenSpaceGuid.SampleVpd|SAMPLE_STRUCT[
> ]|
> {0x01,0x01,0x05,0x09,0x02}
> 
> This VPD/PCD is coming from either the DEC file or the DSC file.
> 
> GenerateByteArrayValue API is used to create the PcdValueInit.exe and 
> it's equivalent C and Make File.
> 
> When there is no change in DEC or DSC VPD/PCD still it used to do the 
> same process again in the incremental build which is time consuming.
> In my project this API is used to take 3min of time with High end 
> server
> (64Threads) and 3.5Min in Laptop (16 threads with performance mode
> enabled) only for GenerateByteArrayValue API.
> 
> This  patch will check if there are any change in the DEC/DSC 
> VPDs/PCDs2, if there is any change in VPD the current flow is not 
> affect. (it will create the PcdRecordList.json and copy Output.txt for 
> Arch folder with the project. Ex:
> Build\{Project}Pkg\DEBUG_VS2019\PcdValueInit\{IA32/X64/Any}\PcdRecordL
> is
> t.json and
> Build\{Project}Pkg\DEBUG_VS2019\PcdValueInit\{IA32/X64/Any}\Output.txt
> ) If there is no change the DSC/DEC VPDs it will read the data from 
> Output.txt and return the same data. (we will compare the Input 
> structure and PcdRecordList.json) if there is any mismatch then it 
> there change in VPDs if not then no change in VPDs).
> 
> Unit testing from my side as follows:
> * Build the Firmware.
> 	* PcdRecordList.json and Output.txt will be created based on the 
> Arch.
> * Build the Firmware again without any change.
> 	* It will read the Output.txt and return the data. (it will match 
> Input PCD/VPD list and compare with existing PcdRecordList.json)
> 	* I verified the return length and content GenerateByteArrayValue 
> (it's same with and without my changes)
> * Build the Firmware again my modifying the VPDs in DSC file
> 	* Change in VPDs, it will regenerate PcdRecordList.json file
> 	*  it will create the Exe file and Output.txt
> 	* New Output.txt will be copied to above path.
> * Build the Firmware again by modifying the VPDs in DEC file.
> 	* Change in VPDs, it will regenerate PcdRecordList.json file
> 	*  it will create the Exe file and Output.txt
> 	* New Output.txt will be copied to above path.
> * Build the Firmware again without modifying the code.
> 	* It will read the Output.txt and return the data.
> 	* I verified the return length and content GenerateByteArrayValue 
> (it's same with and without my changes)
> 
> 
> There is no impact on the Boot/cache.
> This patch is used to reduce the incremental build time by checking if 
> the VPDs/PCDs are changed or not, if not changed then it will return 
> the previous stored data.
> 
> Thanks.,
> S, Ashraf Ali
> 
> -----Original Message-----
> From: Kinney, Michael D <michael.d.kinney@intel.com>
> Sent: Saturday, January 20, 2024 7:36 AM
> To: 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
> 
> 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 (#114176): https://edk2.groups.io/g/devel/message/114176
Mute This Topic: https://groups.io/mt/103781798/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [edk2-devel] [PATCH] BaseTools: Optimize GenerateByteArrayValue and CollectPlatformGuids APIs
  2024-01-23  4:23       ` Feng, Bob C
@ 2024-02-02 11:11         ` Ashraf Ali S
  0 siblings, 0 replies; 6+ messages in thread
From: Ashraf Ali S @ 2024-02-02 11:11 UTC (permalink / raw)
  To: Feng, Bob C, Kinney, Michael D, devel@edk2.groups.io
  Cc: Chen, Christine, Rebecca Cran, Liming Gao, Chan, Amy,
	Chaganty, Rangasai V, Solanki, Digant H

[-- Attachment #1: Type: text/plain, Size: 17875 bytes --]

Hi., @Kinney, Michael D @Feng, Bob C

Thanks for your inputs. After further going through the code, it's not checking if the PCD is changed from Command Line or FDF or the DEC file.

I have send the updated patch. [PATCH v2] BaseTools: Optimize GenerateByteArrayValue and CollectPlatformGuids APIs

Sample PR also I have triggered : https://github.com/tianocore/edk2-basetools/pull/113/files
Unit test details I have added in V2 patch.

Thanks.,
S, Ashraf Ali

-----Original Message-----
From: Feng, Bob C <bob.c.feng@intel.com> 
Sent: Tuesday, January 23, 2024 9:54 AM
To: Kinney, Michael D <michael.d.kinney@intel.com>; S, Ashraf Ali <ashraf.ali.s@intel.com>; devel@edk2.groups.io
Cc: Chen, Christine <yuwei.chen@intel.com>; Rebecca Cran <rebecca@bsdio.com>; Liming Gao <gaoliming@byosoft.com.cn>; Chan, Amy <amy.chan@intel.com>; Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; Solanki, Digant H <digant.h.solanki@intel.com>
Subject: RE: [edk2-devel] [PATCH] BaseTools: Optimize GenerateByteArrayValue and CollectPlatformGuids APIs

Hi Ashraf,

Besides DSC, PCD value also comes from the build tool command line option, DEC and INF. This patch looks only check the PCD changes from DSC, it's not cover all the cases. 

Thanks,
Bob

-----Original Message-----
From: Kinney, Michael D <michael.d.kinney@intel.com>
Sent: Tuesday, January 23, 2024 9:28 AM
To: S, Ashraf Ali <ashraf.ali.s@intel.com>; devel@edk2.groups.io
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

Hi Ashraf,

The PcdValueInit feature is not limited to only PCDs of type VPD.  It is for any structured PCDs.  Did you test PCDs with all types (e.g. PcdsFixedAtBuild PcdsPactahbleInModule, PcdsDynamicHii, PcdsDynamicDatabase, PcdsDynamicVpd, PcdsDynamicExHii, PcdsDynamicExDatabase, PcdsDynamicExVpd)

And did you DSC test cases cover both changes in PCD types and changes in PCD values?

Thanks,

Mike

> -----Original Message-----
> From: S, Ashraf Ali <ashraf.ali.s@intel.com>
> Sent: Sunday, January 21, 2024 4:14 AM
> To: Kinney, Michael D <michael.d.kinney@intel.com>; 
> devel@edk2.groups.io
> 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>
> Subject: RE: [edk2-devel] [PATCH] BaseTools: Optimize 
> GenerateByteArrayValue and CollectPlatformGuids APIs
> 
> Hi., @Kinney, Michael D
> 
> The main API which is modified in this Patch is GenerateByteArrayValue.
> 
> This API is used to return the list of 
> SKUID.TokenSpaceGuid.VpdName|VPD
> STRUCT|Binary Data which will be stored in Output.txt
> Example:
> SAMPLESKUID.STANDARD.gEfiMdePkgTokenSpaceGuid.SampleVpd|SAMPLE_STRUCT[
> ]|
> {0x01,0x01,0x05,0x09,0x02}
> 
> This VPD/PCD is coming from either the DEC file or the DSC file.
> 
> GenerateByteArrayValue API is used to create the PcdValueInit.exe and 
> it's equivalent C and Make File.
> 
> When there is no change in DEC or DSC VPD/PCD still it used to do the 
> same process again in the incremental build which is time consuming.
> In my project this API is used to take 3min of time with High end 
> server
> (64Threads) and 3.5Min in Laptop (16 threads with performance mode
> enabled) only for GenerateByteArrayValue API.
> 
> This  patch will check if there are any change in the DEC/DSC 
> VPDs/PCDs2, if there is any change in VPD the current flow is not 
> affect. (it will create the PcdRecordList.json and copy Output.txt for 
> Arch folder with the project. Ex:
> Build\{Project}Pkg\DEBUG_VS2019\PcdValueInit\{IA32/X64/Any}\PcdRecordL
> is
> t.json and
> Build\{Project}Pkg\DEBUG_VS2019\PcdValueInit\{IA32/X64/Any}\Output.txt
> ) If there is no change the DSC/DEC VPDs it will read the data from 
> Output.txt and return the same data. (we will compare the Input 
> structure and PcdRecordList.json) if there is any mismatch then it 
> there change in VPDs if not then no change in VPDs).
> 
> Unit testing from my side as follows:
> * Build the Firmware.
> 	* PcdRecordList.json and Output.txt will be created based on the 
> Arch.
> * Build the Firmware again without any change.
> 	* It will read the Output.txt and return the data. (it will match 
> Input PCD/VPD list and compare with existing PcdRecordList.json)
> 	* I verified the return length and content GenerateByteArrayValue 
> (it's same with and without my changes)
> * Build the Firmware again my modifying the VPDs in DSC file
> 	* Change in VPDs, it will regenerate PcdRecordList.json file
> 	*  it will create the Exe file and Output.txt
> 	* New Output.txt will be copied to above path.
> * Build the Firmware again by modifying the VPDs in DEC file.
> 	* Change in VPDs, it will regenerate PcdRecordList.json file
> 	*  it will create the Exe file and Output.txt
> 	* New Output.txt will be copied to above path.
> * Build the Firmware again without modifying the code.
> 	* It will read the Output.txt and return the data.
> 	* I verified the return length and content GenerateByteArrayValue 
> (it's same with and without my changes)
> 
> 
> There is no impact on the Boot/cache.
> This patch is used to reduce the incremental build time by checking if 
> the VPDs/PCDs are changed or not, if not changed then it will return 
> the previous stored data.
> 
> Thanks.,
> S, Ashraf Ali
> 
> -----Original Message-----
> From: Kinney, Michael D <michael.d.kinney@intel.com>
> Sent: Saturday, January 20, 2024 7:36 AM
> To: 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
> 
> 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 (#115044): https://edk2.groups.io/g/devel/message/115044
Mute This Topic: https://groups.io/mt/103781798/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: message/rfc822, Size: 15925 bytes --]

From: "S, Ashraf Ali" <ashraf.ali.s@intel.com>
To: "devel@edk2.groups.io" <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 v2] BaseTools: Optimize GenerateByteArrayValue and CollectPlatformGuids APIs
Date: Fri, 2 Feb 2024 10:57:46 +0000
Message-ID: <86248b9bd1c4dcb5e8f6fadb0420f3111fa18541.1706871442.git.ashraf.ali.s@intel.com>

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.

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   | 84 ++++++++++++++++---
 2 files changed, 78 insertions(+), 22 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..3b4a75d86a 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,23 @@ 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 GenerateByteArrayValue (self, StructuredPcds):
         #
         # Generate/Compile/Run C application to determine if there are any flexible array members
@@ -2757,6 +2777,54 @@ class DscBuildData(PlatformBuildClassObject):
         if not StructuredPcds:
             return

+        StructuredPcdsData = {}
+
+        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
+            }
+
+        #
+        # 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)
+                if StructuredPcdsData == StoredStructuredPcdsData:
+                    return self.GetStructurePcdSet(PcdRecordOutputValueFile)
+                else:
+                    # update the record as PCD Input has been changed in incremental build
+                    with open(StructuredPcdsDataPath, 'w') as file:
+                        json.dump(StructuredPcdsData, file, indent=2)
+        else:
+            #
+            # 1st build, create the StructuredPcdsData.json
+            #
+            with open(StructuredPcdsDataPath, 'w') as file:
+                json.dump(StructuredPcdsData, file, indent=2)
+
         InitByteValue = ""
         CApp = PcdMainCHeader

@@ -2832,8 +2900,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 +3108,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 update 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
+        #start update structure pcd final value
+        return self.GetStructurePcdSet(OutputValueFile)

     @staticmethod
     def NeedUpdateOutput(OutputFile, ValueCFile, StructureInput):
--
2.39.1.windows.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2024-02-02 11:11 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox