Re: [Patch] BaseTools: Optimize string concatenation

["Gao, Liming" <liming.gao@intel.com>]

Mike:
  This patch bases on edk2 master with Python27. Seemly, most people are not aware that Python3 migration (https://bugzilla.tianocore.org/show_bug.cgi?id=55) is added for next edk2-stable201903 tag planning. In BZ 55, I propose to keep Python2 and Python3 both, and new feature and patches are created based on Python3. I will send the mail to collect the feedback on this approach. 

Thanks
Liming
> -----Original Message-----
> From: Kinney, Michael D
> Sent: Friday, November 9, 2018 12:41 AM
> To: Feng, Bob C <bob.c.feng@intel.com>; edk2-devel@lists.01.org; Kinney, Michael D <michael.d.kinney@intel.com>
> Cc: Carsey, Jaben <jaben.carsey@intel.com>; Gao, Liming <liming.gao@intel.com>
> Subject: RE: [edk2] [Patch] BaseTools: Optimize string concatenation
> 
> Bob,
> 
> Is this for edk2/master or for the Python 3 conversion in the
> edk2-staging branch?  If it is for the edk-staging branch, then
> the Subject is not correct.
> 
> Thanks,
> 
> Mike
> 
> > -----Original Message-----
> > From: edk2-devel [mailto:edk2-devel-
> > bounces@lists.01.org] On Behalf Of BobCF
> > Sent: Thursday, November 8, 2018 2:16 AM
> > To: edk2-devel@lists.01.org
> > Cc: Carsey, Jaben <jaben.carsey@intel.com>; Gao, Liming
> > <liming.gao@intel.com>
> > Subject: [edk2] [Patch] BaseTools: Optimize string
> > concatenation
> >
> > 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[LibraryCla
> > ssName]
> > -                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(Librar
> > yClassName,Platform.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
> >
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel