Re: [Patch] BaseTools: Optimize string concatenation

["Carsey, Jaben" <jaben.carsey@intel.com>]

Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>

> -----Original Message-----
> From: Feng, Bob C
> Sent: Thursday, November 08, 2018 2:16 AM
> To: edk2-devel@lists.01.org
> Cc: Gao, Liming <liming.gao@intel.com>; Carsey, Jaben
> <jaben.carsey@intel.com>
> Subject: [Patch] BaseTools: Optimize string concatenation
> Importance: High
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=1288
> 
> This patch is one of build tool performance improvement
> series patches.
> 
> This patch is going to use join function instead of
> string += string2 statement.
> 
> Current code use string += string2 in a loop to combine
> a string. while creating a string list in a loop and using
> "".join(stringlist) after the loop will be much faster.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: BobCF <bob.c.feng@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Jaben Carsey <jaben.carsey@intel.com>
> ---
>  BaseTools/Source/Python/AutoGen/StrGather.py  | 39 +++++++++++++----
> --
>  BaseTools/Source/Python/Common/Misc.py        | 21 +++++-----
>  .../Source/Python/Workspace/InfBuildData.py   |  4 +-
>  .../Python/Workspace/WorkspaceCommon.py       | 11 ++----
>  4 files changed, 44 insertions(+), 31 deletions(-)
> 
> diff --git a/BaseTools/Source/Python/AutoGen/StrGather.py
> b/BaseTools/Source/Python/AutoGen/StrGather.py
> index 361d499076..d34a9e9447 100644
> --- a/BaseTools/Source/Python/AutoGen/StrGather.py
> +++ b/BaseTools/Source/Python/AutoGen/StrGather.py
> @@ -135,11 +135,11 @@ def AscToHexList(Ascii):
>  # @param UniGenCFlag      UniString is generated into AutoGen C file when it
> is set to True
>  #
>  # @retval Str:           A string of .h file content
>  #
>  def CreateHFileContent(BaseName, UniObjectClass, IsCompatibleMode,
> UniGenCFlag):
> -    Str = ''
> +    Str = []
>      ValueStartPtr = 60
>      Line = COMMENT_DEFINE_STR + ' ' + LANGUAGE_NAME_STRING_NAME
> + ' ' * (ValueStartPtr - len(DEFINE_STR +
> LANGUAGE_NAME_STRING_NAME)) + DecToHexStr(0, 4) +
> COMMENT_NOT_REFERENCED
>      Str = WriteLine(Str, Line)
>      Line = COMMENT_DEFINE_STR + ' ' +
> PRINTABLE_LANGUAGE_NAME_STRING_NAME + ' ' * (ValueStartPtr -
> len(DEFINE_STR + PRINTABLE_LANGUAGE_NAME_STRING_NAME)) +
> DecToHexStr(1, 4) + COMMENT_NOT_REFERENCED
>      Str = WriteLine(Str, Line)
> @@ -164,16 +164,16 @@ def CreateHFileContent(BaseName,
> UniObjectClass, IsCompatibleMode, UniGenCFlag):
>                      Line = COMMENT_DEFINE_STR + ' ' + Name + ' ' +
> DecToHexStr(Token, 4) + COMMENT_NOT_REFERENCED
>                  else:
>                      Line = COMMENT_DEFINE_STR + ' ' + Name + ' ' * (ValueStartPtr -
> len(DEFINE_STR + Name)) + DecToHexStr(Token, 4) +
> COMMENT_NOT_REFERENCED
>                  UnusedStr = WriteLine(UnusedStr, Line)
> 
> -    Str = ''.join([Str, UnusedStr])
> +    Str.extend( UnusedStr)
> 
>      Str = WriteLine(Str, '')
>      if IsCompatibleMode or UniGenCFlag:
>          Str = WriteLine(Str, 'extern unsigned char ' + BaseName + 'Strings[];')
> -    return Str
> +    return "".join(Str)
> 
>  ## Create a complete .h file
>  #
>  # Create a complet .h file with file header and file content
>  #
> @@ -185,11 +185,11 @@ def CreateHFileContent(BaseName,
> UniObjectClass, IsCompatibleMode, UniGenCFlag):
>  # @retval Str:           A string of complete .h file
>  #
>  def CreateHFile(BaseName, UniObjectClass, IsCompatibleMode,
> UniGenCFlag):
>      HFile = WriteLine('', CreateHFileContent(BaseName, UniObjectClass,
> IsCompatibleMode, UniGenCFlag))
> 
> -    return HFile
> +    return "".join(HFile)
> 
>  ## Create a buffer to store all items in an array
>  #
>  # @param BinBuffer   Buffer to contain Binary data.
>  # @param Array:      The array need to be formatted
> @@ -209,11 +209,11 @@ def CreateBinBuffer(BinBuffer, Array):
>  #
>  def CreateArrayItem(Array, Width = 16):
>      MaxLength = Width
>      Index = 0
>      Line = '  '
> -    ArrayItem = ''
> +    ArrayItem = []
> 
>      for Item in Array:
>          if Index < MaxLength:
>              Line = Line + Item + ',  '
>              Index = Index + 1
> @@ -221,11 +221,11 @@ def CreateArrayItem(Array, Width = 16):
>              ArrayItem = WriteLine(ArrayItem, Line)
>              Line = '  ' + Item + ',  '
>              Index = 1
>      ArrayItem = Write(ArrayItem, Line.rstrip())
> 
> -    return ArrayItem
> +    return "".join(ArrayItem)
> 
>  ## CreateCFileStringValue
>  #
>  # Create a line with string value
>  #
> @@ -236,11 +236,11 @@ def CreateArrayItem(Array, Width = 16):
> 
>  def CreateCFileStringValue(Value):
>      Value = [StringBlockType] + Value
>      Str = WriteLine('', CreateArrayItem(Value))
> 
> -    return Str
> +    return "".join(Str)
> 
>  ## GetFilteredLanguage
>  #
>  # apply get best language rules to the UNI language code list
>  #
> @@ -438,11 +438,11 @@ def CreateCFileContent(BaseName,
> UniObjectClass, IsCompatibleMode, UniBinBuffer,
>      #
>      # Join package data
>      #
>      AllStr = Write(AllStr, Str)
> 
> -    return AllStr
> +    return "".join(AllStr)
> 
>  ## Create end of .c file
>  #
>  # Create end of .c file
>  #
> @@ -465,11 +465,11 @@ def CreateCFileEnd():
>  #
>  def CreateCFile(BaseName, UniObjectClass, IsCompatibleMode, FilterInfo):
>      CFile = ''
>      CFile = WriteLine(CFile, CreateCFileContent(BaseName, UniObjectClass,
> IsCompatibleMode, None, FilterInfo))
>      CFile = WriteLine(CFile, CreateCFileEnd())
> -    return CFile
> +    return "".join(CFile)
> 
>  ## GetFileList
>  #
>  # Get a list for all files
>  #
> @@ -572,17 +572,34 @@ def GetStringFiles(UniFilList, SourceFileList,
> IncludeList, IncludePathList, Ski
> 
>  #
>  # Write an item
>  #
>  def Write(Target, Item):
> -    return ''.join([Target, Item])
> +    if isinstance(Target,str):
> +        Target = [Target]
> +    if not Target:
> +        Target = []
> +    if isinstance(Item,list):
> +        Target.extend(Item)
> +    else:
> +        Target.append(Item)
> +    return Target
> 
>  #
>  # Write an item with a break line
>  #
>  def WriteLine(Target, Item):
> -    return ''.join([Target, Item, '\n'])
> +    if isinstance(Target,str):
> +        Target = [Target]
> +    if not Target:
> +        Target = []
> +    if isinstance(Item, list):
> +        Target.extend(Item)
> +    else:
> +        Target.append(Item)
> +    Target.append('\n')
> +    return Target
> 
>  # This acts like the main() function for the script, unless it is 'import'ed into
> another
>  # script.
>  if __name__ == '__main__':
>      EdkLogger.info('start')
> diff --git a/BaseTools/Source/Python/Common/Misc.py
> b/BaseTools/Source/Python/Common/Misc.py
> index 80236db160..8dcbe141ae 100644
> --- a/BaseTools/Source/Python/Common/Misc.py
> +++ b/BaseTools/Source/Python/Common/Misc.py
> @@ -777,21 +777,21 @@ class TemplateString(object):
> 
>              return "".join(StringList)
> 
>      ## Constructor
>      def __init__(self, Template=None):
> -        self.String = ''
> +        self.String = []
>          self.IsBinary = False
>          self._Template = Template
>          self._TemplateSectionList = self._Parse(Template)
> 
>      ## str() operator
>      #
>      #   @retval     string  The string replaced
>      #
>      def __str__(self):
> -        return self.String
> +        return "".join(self.String)
> 
>      ## Split the template string into fragments per the ${BEGIN} and ${END}
> flags
>      #
>      #   @retval     list    A list of TemplateString.Section objects
>      #
> @@ -835,13 +835,16 @@ class TemplateString(object):
>      #   @param      Dictionary      The placeholder dictionaries
>      #
>      def Append(self, AppendString, Dictionary=None):
>          if Dictionary:
>              SectionList = self._Parse(AppendString)
> -            self.String += "".join(S.Instantiate(Dictionary) for S in SectionList)
> +            self.String.append( "".join(S.Instantiate(Dictionary) for S in
> SectionList))
>          else:
> -            self.String += AppendString
> +            if isinstance(AppendString,list):
> +                self.String.extend(AppendString)
> +            else:
> +                self.String.append(AppendString)
> 
>      ## Replace the string template with dictionary of placeholders
>      #
>      #   @param      Dictionary      The placeholder dictionaries
>      #
> @@ -1741,27 +1744,21 @@ class PathClass(object):
>      #
>      # @retval False The two PathClass are different
>      # @retval True  The two PathClass are the same
>      #
>      def __eq__(self, Other):
> -        if isinstance(Other, type(self)):
> -            return self.Path == Other.Path
> -        else:
> -            return self.Path == str(Other)
> +        return self.Path == str(Other)
> 
>      ## Override __cmp__ function
>      #
>      # Customize the comparsion operation of two PathClass
>      #
>      # @retval 0     The two PathClass are different
>      # @retval -1    The first PathClass is less than the second PathClass
>      # @retval 1     The first PathClass is Bigger than the second PathClass
>      def __cmp__(self, Other):
> -        if isinstance(Other, type(self)):
> -            OtherKey = Other.Path
> -        else:
> -            OtherKey = str(Other)
> +        OtherKey = str(Other)
> 
>          SelfKey = self.Path
>          if SelfKey == OtherKey:
>              return 0
>          elif SelfKey > OtherKey:
> diff --git a/BaseTools/Source/Python/Workspace/InfBuildData.py
> b/BaseTools/Source/Python/Workspace/InfBuildData.py
> index 44d44d24eb..d615cccdf7 100644
> --- a/BaseTools/Source/Python/Workspace/InfBuildData.py
> +++ b/BaseTools/Source/Python/Workspace/InfBuildData.py
> @@ -612,11 +612,13 @@ class InfBuildData(ModuleBuildClassObject):
>          for Record in RecordList:
>              Lib = Record[0]
>              Instance = Record[1]
>              if Instance:
>                  Instance = NormPath(Instance, self._Macros)
> -            RetVal[Lib] = Instance
> +                RetVal[Lib] = Instance
> +            else:
> +                RetVal[Lib] = None
>          return RetVal
> 
>      ## Retrieve library names (for Edk.x style of modules)
>      @cached_property
>      def Libraries(self):
> diff --git a/BaseTools/Source/Python/Workspace/WorkspaceCommon.py
> b/BaseTools/Source/Python/Workspace/WorkspaceCommon.py
> index 8d8a3e2789..55d01fa4b2 100644
> --- a/BaseTools/Source/Python/Workspace/WorkspaceCommon.py
> +++ b/BaseTools/Source/Python/Workspace/WorkspaceCommon.py
> @@ -126,17 +126,14 @@ def GetModuleLibInstances(Module, Platform,
> BuildDatabase, Arch, Target, Toolcha
>      while len(LibraryConsumerList) > 0:
>          M = LibraryConsumerList.pop()
>          for LibraryClassName in M.LibraryClasses:
>              if LibraryClassName not in LibraryInstance:
>                  # override library instance for this module
> -                if LibraryClassName in
> Platform.Modules[str(Module)].LibraryClasses:
> -                    LibraryPath =
> Platform.Modules[str(Module)].LibraryClasses[LibraryClassName]
> -                else:
> -                    LibraryPath = Platform.LibraryClasses[LibraryClassName,
> ModuleType]
> -                if LibraryPath is None or LibraryPath == "":
> -                    LibraryPath = M.LibraryClasses[LibraryClassName]
> -                    if LibraryPath is None or LibraryPath == "":
> +                LibraryPath =
> Platform.Modules[str(Module)].LibraryClasses.get(LibraryClassName,Platfor
> m.LibraryClasses[LibraryClassName, ModuleType])
> +                if LibraryPath is None:
> +                    LibraryPath = M.LibraryClasses.get(LibraryClassName)
> +                    if LibraryPath is None:
>                          if FileName:
>                              EdkLogger.error("build", RESOURCE_NOT_AVAILABLE,
>                                              "Instance of library class [%s] is not found" %
> LibraryClassName,
>                                              File=FileName,
>                                              ExtraData="in [%s] [%s]\n\tconsumed by module
> [%s]" % (str(M), Arch, str(Module)))
> --
> 2.19.1.windows.1