Re: [edk2] [Patch] BaseTools: Optimize string concatenation

2018-12-13 Thread Gao, Liming
Leif:
  Agree your point to prepare the data before the patch instead of after the 
patch. 

Thanks
Liming
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Leif 
> Lindholm
> Sent: Thursday, December 13, 2018 6:09 PM
> To: Gao, Liming 
> Cc: Carsey, Jaben ; edk2-devel@lists.01.org
> Subject: Re: [edk2] [Patch] BaseTools: Optimize string concatenation
> 
> Hi Liming,
> 
> Yes, this is fine.
> But for future submissions, I would like for this sort of information
> to be provided at the time of posting.
> 
> Not the full breakdown, but "reduceces build time of platform XXX on
> hardware YYY by A%/B seconds".
> 
> Ideally, more than one platform and more than one hardware should be
> provided, but at least during this initial improvement phase I'm also
> happy for the assumption being that unless someone else complains,
> it's fine on others.
> 
> Regards,
> 
> Leif
> 
> On Thu, Dec 13, 2018 at 01:50:35AM +, Gao, Liming wrote:
> > Leif:
> >   Kabylake platform is the real Intel hardware.  The MinKabylake is the 
> > minimal feature of the Kabylake BIOS. Here is MinKabylake
> BIOS code https://github.com/tianocore/edk2-platforms/tree/devel-MinPlatform
> >   Bob adds the build performance data of MinKabylake into 
> > https://bugzilla.tianocore.org/show_bug.cgi?id=1288.
> >
> > The original build performance data:
> > Build Duration:   00:02:23
> > AutoGen Duration: 00:00:42
> > Make Duration:00:01:12
> > GenFds Duration:  00:00:27
> >
> > After apply the patch, clean build performance is reduced from 2:23 to 
> > 1:57. So, I think his patch improves build performance.
> > Build Duration:   00:01:57
> > AutoGen Duration: 00:00:23
> > Make Duration:00:01:12
> > GenFds Duration:  00:00:21
> >
> > Thanks
> > Liming
> > >-Original Message-
> > >From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Leif
> > >Lindholm
> > >Sent: Thursday, December 13, 2018 2:37 AM
> > >To: Feng, Bob C 
> > >Cc: Carsey, Jaben ; edk2-devel@lists.01.org; Gao,
> > >Liming 
> > >Subject: Re: [edk2] [Patch] BaseTools: Optimize string concatenation
> > >
> > >On Tue, Dec 11, 2018 at 08:48:19AM +, Feng, Bob C wrote:
> > >> Hi Leif,
> > >>
> > >> I understand your concern.
> > >>
> > >> I collected another performance data set based on open source
> > >> MinKabylake platform and updated the BZ
> > >> https://bugzilla.tianocore.org/show_bug.cgi?id=1288. The data looks
> > >> better than Ovmf. It enabled multiple SKU.
> > >>
> > >> Before I sent those patch, I did verify them on intel real
> > >> platforms. It improves the build performance. But it's not
> > >> convenient to share those data.
> > >
> > >So, I have two comments on this:
> > >1) How can it be inconvenient to share information on build times? I
> > >   don't even care what the names or codenames for those platforms
> > >   are. If you are unable to tell us why what you have done matters,
> > >   the code changes do not belong in the public tree.
> > >   Clearly having good performance numbers for public platforms is the
> > >   easiest solution for this problem.
> > >2) Submissions of improvements to build system performance should be
> > >   verified building real platforms. It should not be a question of
> > >   "find some other platform to get numbers from once we have improved
> > >   performance for building our confidential platforms".
> > >
> > >Regards,
> > >
> > >Leif
> > >>
> > >> Thanks,
> > >> Bob
> > >>
> > >> -Original Message-
> > >> From: Leif Lindholm [mailto:leif.lindh...@linaro.org]
> > >> Sent: Monday, December 10, 2018 8:36 PM
> > >> To: Feng, Bob C 
> > >> Cc: edk2-devel@lists.01.org; Carsey, Jaben ; Gao,
> > >Liming 
> > >> Subject: Re: [edk2] [Patch] BaseTools: Optimize string concatenation
> > >>
> > >> On Mon, Dec 10, 2018 at 12:09:23PM +, Feng, Bob C wrote:
> > >> > For the "customized deepcopy" and "cache for uni file parser" data,
> > >> > you can see the AutoGen is not slower. The whole Build Duration is
> > >> > longer because Make Duration is longer while Make Duration time
> > &

Re: [edk2] [Patch] BaseTools: Optimize string concatenation

2018-12-13 Thread Leif Lindholm
Hi Liming,

Yes, this is fine.
But for future submissions, I would like for this sort of information
to be provided at the time of posting.

Not the full breakdown, but "reduceces build time of platform XXX on
hardware YYY by A%/B seconds".

Ideally, more than one platform and more than one hardware should be
provided, but at least during this initial improvement phase I'm also
happy for the assumption being that unless someone else complains,
it's fine on others.

Regards,

Leif

On Thu, Dec 13, 2018 at 01:50:35AM +, Gao, Liming wrote:
> Leif:
>   Kabylake platform is the real Intel hardware.  The MinKabylake is the 
> minimal feature of the Kabylake BIOS. Here is MinKabylake BIOS code 
> https://github.com/tianocore/edk2-platforms/tree/devel-MinPlatform
>   Bob adds the build performance data of MinKabylake into 
> https://bugzilla.tianocore.org/show_bug.cgi?id=1288. 
> 
> The original build performance data:
> Build Duration:   00:02:23
> AutoGen Duration: 00:00:42
> Make Duration:00:01:12
> GenFds Duration:  00:00:27
> 
> After apply the patch, clean build performance is reduced from 2:23 to 1:57. 
> So, I think his patch improves build performance. 
> Build Duration:   00:01:57
> AutoGen Duration: 00:00:23
> Make Duration:00:01:12
> GenFds Duration:  00:00:21
> 
> Thanks
> Liming
> >-Original Message-
> >From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Leif
> >Lindholm
> >Sent: Thursday, December 13, 2018 2:37 AM
> >To: Feng, Bob C 
> >Cc: Carsey, Jaben ; edk2-devel@lists.01.org; Gao,
> >Liming 
> >Subject: Re: [edk2] [Patch] BaseTools: Optimize string concatenation
> >
> >On Tue, Dec 11, 2018 at 08:48:19AM +, Feng, Bob C wrote:
> >> Hi Leif,
> >>
> >> I understand your concern.
> >>
> >> I collected another performance data set based on open source
> >> MinKabylake platform and updated the BZ
> >> https://bugzilla.tianocore.org/show_bug.cgi?id=1288. The data looks
> >> better than Ovmf. It enabled multiple SKU.
> >>
> >> Before I sent those patch, I did verify them on intel real
> >> platforms. It improves the build performance. But it's not
> >> convenient to share those data.
> >
> >So, I have two comments on this:
> >1) How can it be inconvenient to share information on build times? I
> >   don't even care what the names or codenames for those platforms
> >   are. If you are unable to tell us why what you have done matters,
> >   the code changes do not belong in the public tree.
> >   Clearly having good performance numbers for public platforms is the
> >   easiest solution for this problem.
> >2) Submissions of improvements to build system performance should be
> >   verified building real platforms. It should not be a question of
> >   "find some other platform to get numbers from once we have improved
> >   performance for building our confidential platforms".
> >
> >Regards,
> >
> >Leif
> >>
> >> Thanks,
> >> Bob
> >>
> >> -Original Message-
> >> From: Leif Lindholm [mailto:leif.lindh...@linaro.org]
> >> Sent: Monday, December 10, 2018 8:36 PM
> >> To: Feng, Bob C 
> >> Cc: edk2-devel@lists.01.org; Carsey, Jaben ; Gao,
> >Liming 
> >> Subject: Re: [edk2] [Patch] BaseTools: Optimize string concatenation
> >>
> >> On Mon, Dec 10, 2018 at 12:09:23PM +, Feng, Bob C wrote:
> >> > For the "customized deepcopy" and "cache for uni file parser" data,
> >> > you can see the AutoGen is not slower. The whole Build Duration is
> >> > longer because Make Duration is longer while Make Duration time
> >> > depends on the external make, compiler and linker. So it's not the
> >> > patch make the build slow down.
> >> >
> >> > Yes,  it's not faster either. I think that because the Ovmf platform
> >> > is relatively simple.  From the build tool source code point of view,
> >> > the customized deepcopy will take effect if the platform enabled
> >> > multiple SKU or there are many expressions in metadata file to be
> >> > evaluated. And the "cache for uni file parser" needs there are many
> >> > uni files.  The Ovmf platform looks not a good platform to demo the
> >> > effect of this patch.
> >>
> >> But surely we should not introduce patches said to improve performance
> >when the only data we have available shows that 

Re: [edk2] [Patch] BaseTools: Optimize string concatenation

2018-12-12 Thread Leif Lindholm
On Tue, Dec 11, 2018 at 08:48:19AM +, Feng, Bob C wrote:
> Hi Leif,
> 
> I understand your concern.  
> 
> I collected another performance data set based on open source
> MinKabylake platform and updated the BZ
> https://bugzilla.tianocore.org/show_bug.cgi?id=1288. The data looks
> better than Ovmf. It enabled multiple SKU.
> 
> Before I sent those patch, I did verify them on intel real
> platforms. It improves the build performance. But it's not
> convenient to share those data.

So, I have two comments on this:
1) How can it be inconvenient to share information on build times? I
   don't even care what the names or codenames for those platforms
   are. If you are unable to tell us why what you have done matters,
   the code changes do not belong in the public tree.
   Clearly having good performance numbers for public platforms is the
   easiest solution for this problem.
2) Submissions of improvements to build system performance should be
   verified building real platforms. It should not be a question of
   "find some other platform to get numbers from once we have improved
   performance for building our confidential platforms".

Regards,

Leif
> 
> Thanks,
> Bob
> 
> -Original Message-
> From: Leif Lindholm [mailto:leif.lindh...@linaro.org] 
> Sent: Monday, December 10, 2018 8:36 PM
> To: Feng, Bob C 
> Cc: edk2-devel@lists.01.org; Carsey, Jaben ; Gao, 
> Liming 
> Subject: Re: [edk2] [Patch] BaseTools: Optimize string concatenation
> 
> On Mon, Dec 10, 2018 at 12:09:23PM +, Feng, Bob C wrote:
> > For the "customized deepcopy" and "cache for uni file parser" data, 
> > you can see the AutoGen is not slower. The whole Build Duration is 
> > longer because Make Duration is longer while Make Duration time 
> > depends on the external make, compiler and linker. So it's not the 
> > patch make the build slow down.
> > 
> > Yes,  it's not faster either. I think that because the Ovmf platform 
> > is relatively simple.  From the build tool source code point of view, 
> > the customized deepcopy will take effect if the platform enabled 
> > multiple SKU or there are many expressions in metadata file to be 
> > evaluated. And the "cache for uni file parser" needs there are many 
> > uni files.  The Ovmf platform looks not a good platform to demo the 
> > effect of this patch.
> 
> But surely we should not introduce patches said to improve performance when 
> the only data we have available shows that they slow things down?
> 
> If the performance data is not representative, then it is worthless.
> 
> Don't get me wrong - if you say "and for this secret platform I can't share 
> with you, it improves build performance by X", then I may be OK with a minor 
> slowdown on the platforms I do have available to test, if X is not minor.
> 
> But if the improvement is only theoretical, and we have no evidence that it 
> helps real platforms, it should not be committed.
> 
> Regards,
> 
> Leif
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [Patch] BaseTools: Optimize string concatenation

2018-12-11 Thread Feng, Bob C
Hi Leif,

I understand your concern.  

I collected another performance data set based on open source MinKabylake 
platform and updated the BZ 
https://bugzilla.tianocore.org/show_bug.cgi?id=1288. The data looks better than 
Ovmf. It enabled multiple SKU.

Before I sent those patch, I did verify them on intel real platforms. It 
improves the build performance. But it's not convenient to share those data.

Thanks,
Bob

-Original Message-
From: Leif Lindholm [mailto:leif.lindh...@linaro.org] 
Sent: Monday, December 10, 2018 8:36 PM
To: Feng, Bob C 
Cc: edk2-devel@lists.01.org; Carsey, Jaben ; Gao, 
Liming 
Subject: Re: [edk2] [Patch] BaseTools: Optimize string concatenation

On Mon, Dec 10, 2018 at 12:09:23PM +, Feng, Bob C wrote:
> For the "customized deepcopy" and "cache for uni file parser" data, 
> you can see the AutoGen is not slower. The whole Build Duration is 
> longer because Make Duration is longer while Make Duration time 
> depends on the external make, compiler and linker. So it's not the 
> patch make the build slow down.
> 
> Yes,  it's not faster either. I think that because the Ovmf platform 
> is relatively simple.  From the build tool source code point of view, 
> the customized deepcopy will take effect if the platform enabled 
> multiple SKU or there are many expressions in metadata file to be 
> evaluated. And the "cache for uni file parser" needs there are many 
> uni files.  The Ovmf platform looks not a good platform to demo the 
> effect of this patch.

But surely we should not introduce patches said to improve performance when the 
only data we have available shows that they slow things down?

If the performance data is not representative, then it is worthless.

Don't get me wrong - if you say "and for this secret platform I can't share 
with you, it improves build performance by X", then I may be OK with a minor 
slowdown on the platforms I do have available to test, if X is not minor.

But if the improvement is only theoretical, and we have no evidence that it 
helps real platforms, it should not be committed.

Regards,

Leif
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [Patch] BaseTools: Optimize string concatenation

2018-12-10 Thread Leif Lindholm
On Mon, Dec 10, 2018 at 12:09:23PM +, Feng, Bob C wrote:
> For the "customized deepcopy" and "cache for uni file parser" data,
> you can see the AutoGen is not slower. The whole Build Duration is
> longer because Make Duration is longer while Make Duration time
> depends on the external make, compiler and linker. So it's not the
> patch make the build slow down.
> 
> Yes,  it's not faster either. I think that because the Ovmf platform
> is relatively simple.  From the build tool source code point of
> view, the customized deepcopy will take effect if the platform
> enabled multiple SKU or there are many expressions in metadata file
> to be evaluated. And the "cache for uni file parser" needs there are
> many uni files.  The Ovmf platform looks not a good platform to demo
> the effect of this patch.

But surely we should not introduce patches said to improve performance
when the only data we have available shows that they slow things down?

If the performance data is not representative, then it is worthless.

Don't get me wrong - if you say "and for this secret platform I can't
share with you, it improves build performance by X", then I may be OK
with a minor slowdown on the platforms I do have available to test, if
X is not minor.

But if the improvement is only theoretical, and we have no evidence
that it helps real platforms, it should not be committed.

Regards,

Leif
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [Patch] BaseTools: Optimize string concatenation

2018-12-10 Thread Feng, Bob C
Hi Leif,

For the "customized deepcopy" and "cache for uni file parser" data, you can see 
the AutoGen is not slower. The whole Build Duration is longer because Make 
Duration is longer while Make Duration time depends on the external make, 
compiler and linker. So it's not the patch make the build slow down.

Yes,  it's not faster either. I think that because the Ovmf platform is 
relatively simple.  From the build tool source code point of view, the 
customized deepcopy will take effect if the platform enabled multiple SKU or 
there are many expressions in metadata file to be evaluated. And the "cache for 
uni file parser" needs there are many uni files.  The Ovmf platform looks not a 
good platform to demo the effect of this patch.


Thanks,
Bob

-Original Message-
From: Leif Lindholm [mailto:leif.lindh...@linaro.org] 
Sent: Monday, December 10, 2018 6:48 PM
To: Feng, Bob C 
Cc: edk2-devel@lists.01.org; Carsey, Jaben ; Gao, 
Liming 
Subject: Re: [edk2] [Patch] BaseTools: Optimize string concatenation

Hi Bob,

Thanks.

I am a little bit confused by the "customized deepcopy" and "cache for uni file 
parser" data. These look like they are slowing down rather than speeding up the 
build.

Regards,

Leif

On Wed, Dec 05, 2018 at 02:51:58AM +, Feng, Bob C wrote:
> Hi,
> 
> I have added the performance data in BZ
> https://bugzilla.tianocore.org/show_bug.cgi?id=1288 . Please have a 
> review.
> 
> Thanks,
> Bob
> 
> 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Feng, Bob C
> Sent: Sunday, November 11, 2018 8:41 AM
> To: Leif Lindholm 
> Cc: Carsey, Jaben ; edk2-devel@lists.01.org; 
> Gao, Liming 
> Subject: Re: [edk2] [Patch] BaseTools: Optimize string concatenation
> 
> Hi Leif,
> 
> I use my desktop to do the benchmark.
> CPU: Intel(R) Core(TM) i7-6700 CPU @ 3.40GHz
> Memory: 16G
> OS: Win10
> 
> I'll add the performance detailed data to BZ.
> 
> -Bob
> 
> -Original Message-
> From: Leif Lindholm [mailto:leif.lindh...@linaro.org]
> Sent: Friday, November 9, 2018 7:49 PM
> To: Feng, Bob C 
> Cc: edk2-devel@lists.01.org; Carsey, Jaben ; 
> Gao, Liming 
> Subject: Re: [edk2] [Patch] BaseTools: Optimize string concatenation
> 
> Hi Bob,
> 
> On Fri, Nov 09, 2018 at 03:25:04AM +, Feng, Bob C wrote:
> > Yes. I should show the data.
> > 
> > My unites scripts is as below. The parameter lines is a string list 
> > which size is 43395. The test result is
> > 
> > ''.join(String list) time:  0.042262 String += String time  :  
> > 3.822699
> > 
> > def TestPlus(lines):
> > str_target = ""
> > 
> > for line in lines:
> > str_target += line
> > 
> > return str_target
> > 
> > def TestJoin(lines):
> > str_target = []
> > 
> > for line in lines:
> > str_target.append(line)
> > 
> > return "".join(str_target)
> > 
> > def CompareStrCat():
> > lines = GetStrings()
> > print (len(lines))
> > 
> > begin = time.perf_counter()
> > for _ in range(10):
> > TestJoin(lines)
> > end = time.perf_counter() - begin
> > print ("''.join(String list) time: %f" % end)
> > 
> > begin = time.perf_counter()
> > for _ in range(10):
> > TestPlus(lines)
> > end = time.perf_counter() - begin
> > print ("String += String time: %f" % end)
> > 
> > For build OvmfX64, it's not very effective, it saves 2~3 second in 
> > Parse/AutoGen phase, because OvmfX64 is relatively simple. It does 
> > not enable much features such as Multiple SKU and structure PCD by 
> > default and there is no big size Autogen.c/Autogen.h/Makefile 
> > generated either. but for the complex platform, this patch will be 
> > much effective. The unites above simulates a real case that there is 
> > a
> > 43395 lines of Autogen.c generated.
> 
> I beg to differ. Shaving 2-3 seconds off the autogen phase is a substantial 
> improvement.
> 
> However, on my wimpy 24-core Cortex-A53 system, the effect is not noticeable 
> (fluctuates between 1:56-1:58 whether with or without this patch).
> 
> And even on my x86 workstation, I see no measurable difference (12-13s).
> What is the hardware you are benchmarking on?
> 
> It does not appear to be detrimental to performance on any of my platforms, 
> but I would like to see some more measurements, and I would like to see that 
> logged with some more detail in bugzilla.
> 
> Regards,

Re: [edk2] [Patch] BaseTools: Optimize string concatenation

2018-12-10 Thread Leif Lindholm
Hi Bob,

Thanks.

I am a little bit confused by the "customized deepcopy" and "cache for
uni file parser" data. These look like they are slowing down rather
than speeding up the build.

Regards,

Leif

On Wed, Dec 05, 2018 at 02:51:58AM +, Feng, Bob C wrote:
> Hi,
> 
> I have added the performance data in BZ
> https://bugzilla.tianocore.org/show_bug.cgi?id=1288 . Please have a
> review.
> 
> Thanks,
> Bob
> 
> 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Feng, 
> Bob C
> Sent: Sunday, November 11, 2018 8:41 AM
> To: Leif Lindholm 
> Cc: Carsey, Jaben ; edk2-devel@lists.01.org; Gao, 
> Liming 
> Subject: Re: [edk2] [Patch] BaseTools: Optimize string concatenation
> 
> Hi Leif,
> 
> I use my desktop to do the benchmark.
> CPU: Intel(R) Core(TM) i7-6700 CPU @ 3.40GHz
> Memory: 16G
> OS: Win10
> 
> I'll add the performance detailed data to BZ.
> 
> -Bob
> 
> -Original Message-
> From: Leif Lindholm [mailto:leif.lindh...@linaro.org]
> Sent: Friday, November 9, 2018 7:49 PM
> To: Feng, Bob C 
> Cc: edk2-devel@lists.01.org; Carsey, Jaben ; Gao, 
> Liming 
> Subject: Re: [edk2] [Patch] BaseTools: Optimize string concatenation
> 
> Hi Bob,
> 
> On Fri, Nov 09, 2018 at 03:25:04AM +, Feng, Bob C wrote:
> > Yes. I should show the data.
> > 
> > My unites scripts is as below. The parameter lines is a string list 
> > which size is 43395. The test result is
> > 
> > ''.join(String list) time:  0.042262
> > String += String time  :  3.822699
> > 
> > def TestPlus(lines):
> > str_target = ""
> > 
> > for line in lines:
> > str_target += line
> > 
> > return str_target
> > 
> > def TestJoin(lines):
> > str_target = []
> > 
> > for line in lines:
> > str_target.append(line)
> > 
> > return "".join(str_target)
> > 
> > def CompareStrCat():
> > lines = GetStrings()
> > print (len(lines))
> > 
> > begin = time.perf_counter()
> > for _ in range(10):
> > TestJoin(lines)
> > end = time.perf_counter() - begin
> > print ("''.join(String list) time: %f" % end)
> > 
> > begin = time.perf_counter()
> > for _ in range(10):
> > TestPlus(lines)
> > end = time.perf_counter() - begin
> > print ("String += String time: %f" % end)
> > 
> > For build OvmfX64, it's not very effective, it saves 2~3 second in 
> > Parse/AutoGen phase, because OvmfX64 is relatively simple. It does not 
> > enable much features such as Multiple SKU and structure PCD by default 
> > and there is no big size Autogen.c/Autogen.h/Makefile generated 
> > either. but for the complex platform, this patch will be much 
> > effective. The unites above simulates a real case that there is a
> > 43395 lines of Autogen.c generated.
> 
> I beg to differ. Shaving 2-3 seconds off the autogen phase is a substantial 
> improvement.
> 
> However, on my wimpy 24-core Cortex-A53 system, the effect is not noticeable 
> (fluctuates between 1:56-1:58 whether with or without this patch).
> 
> And even on my x86 workstation, I see no measurable difference (12-13s).
> What is the hardware you are benchmarking on?
> 
> It does not appear to be detrimental to performance on any of my platforms, 
> but I would like to see some more measurements, and I would like to see that 
> logged with some more detail in bugzilla.
> 
> Regards,
> 
> Leif
> 
> > Since this patch mostly effect the Parser/AutoGen phase, I just use 
> > "build genmake" to show the improvement data.
> > The final result for clean build is:
> > Current code:  17 seconds
> > After patch:  15 seconds
> > 
> > Details:
> > Current data:
> > 
> > d:\edk2 (master -> origin)
> > λ build genmake -p OvmfPkg\OvmfPkgIa32X64.dsc -a IA32 -a X64 -t
> > VS2015x86 Build environment: Windows-10-10.0.10240 Build start time: 
> > 10:12:32, Nov.09 2018
> > 
> > WORKSPACE= d:\edk2
> > ECP_SOURCE   = d:\edk2\edkcompatibilitypkg
> > EDK_SOURCE   = d:\edk2\edkcompatibilitypkg
> > EFI_SOURCE   = d:\edk2\edkcompatibilitypkg
> > EDK_TOOLS_PATH   = d:\edk2\basetools
> > EDK_TOOLS_BIN= d:\edk2\basetools\bin\win32
> > CONF_PATH= d:\edk2\conf
> > 
> > Architecture(s)  = IA32 X64
> > Build target = DEBUG
> > Toolchain= VS2015x86
&

Re: [edk2] [Patch] BaseTools: Optimize string concatenation

2018-12-04 Thread Feng, Bob C
Hi,

I have added the performance data in BZ 
https://bugzilla.tianocore.org/show_bug.cgi?id=1288 . Please have a review.

Thanks,
Bob


-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Feng, 
Bob C
Sent: Sunday, November 11, 2018 8:41 AM
To: Leif Lindholm 
Cc: Carsey, Jaben ; edk2-devel@lists.01.org; Gao, 
Liming 
Subject: Re: [edk2] [Patch] BaseTools: Optimize string concatenation

Hi Leif,

I use my desktop to do the benchmark.
CPU: Intel(R) Core(TM) i7-6700 CPU @ 3.40GHz
Memory: 16G
OS: Win10

I'll add the performance detailed data to BZ.

-Bob

-Original Message-
From: Leif Lindholm [mailto:leif.lindh...@linaro.org]
Sent: Friday, November 9, 2018 7:49 PM
To: Feng, Bob C 
Cc: edk2-devel@lists.01.org; Carsey, Jaben ; Gao, 
Liming 
Subject: Re: [edk2] [Patch] BaseTools: Optimize string concatenation

Hi Bob,

On Fri, Nov 09, 2018 at 03:25:04AM +, Feng, Bob C wrote:
> Yes. I should show the data.
> 
> My unites scripts is as below. The parameter lines is a string list 
> which size is 43395. The test result is
> 
> ''.join(String list) time:  0.042262
> String += String time  :  3.822699
> 
> def TestPlus(lines):
> str_target = ""
> 
> for line in lines:
> str_target += line
> 
> return str_target
> 
> def TestJoin(lines):
> str_target = []
> 
> for line in lines:
> str_target.append(line)
> 
> return "".join(str_target)
> 
> def CompareStrCat():
> lines = GetStrings()
> print (len(lines))
> 
> begin = time.perf_counter()
> for _ in range(10):
> TestJoin(lines)
> end = time.perf_counter() - begin
> print ("''.join(String list) time: %f" % end)
> 
> begin = time.perf_counter()
> for _ in range(10):
> TestPlus(lines)
> end = time.perf_counter() - begin
> print ("String += String time: %f" % end)
> 
> For build OvmfX64, it's not very effective, it saves 2~3 second in 
> Parse/AutoGen phase, because OvmfX64 is relatively simple. It does not 
> enable much features such as Multiple SKU and structure PCD by default 
> and there is no big size Autogen.c/Autogen.h/Makefile generated 
> either. but for the complex platform, this patch will be much 
> effective. The unites above simulates a real case that there is a
> 43395 lines of Autogen.c generated.

I beg to differ. Shaving 2-3 seconds off the autogen phase is a substantial 
improvement.

However, on my wimpy 24-core Cortex-A53 system, the effect is not noticeable 
(fluctuates between 1:56-1:58 whether with or without this patch).

And even on my x86 workstation, I see no measurable difference (12-13s).
What is the hardware you are benchmarking on?

It does not appear to be detrimental to performance on any of my platforms, but 
I would like to see some more measurements, and I would like to see that logged 
with some more detail in bugzilla.

Regards,

Leif

> Since this patch mostly effect the Parser/AutoGen phase, I just use 
> "build genmake" to show the improvement data.
> The final result for clean build is:
> Current code:  17 seconds
> After patch:  15 seconds
> 
> Details:
> Current data:
> 
> d:\edk2 (master -> origin)
> λ build genmake -p OvmfPkg\OvmfPkgIa32X64.dsc -a IA32 -a X64 -t
> VS2015x86 Build environment: Windows-10-10.0.10240 Build start time: 
> 10:12:32, Nov.09 2018
> 
> WORKSPACE= d:\edk2
> ECP_SOURCE   = d:\edk2\edkcompatibilitypkg
> EDK_SOURCE   = d:\edk2\edkcompatibilitypkg
> EFI_SOURCE   = d:\edk2\edkcompatibilitypkg
> EDK_TOOLS_PATH   = d:\edk2\basetools
> EDK_TOOLS_BIN= d:\edk2\basetools\bin\win32
> CONF_PATH= d:\edk2\conf
> 
> Architecture(s)  = IA32 X64
> Build target = DEBUG
> Toolchain= VS2015x86
> 
> Active Platform  = d:\edk2\OvmfPkg\OvmfPkgIa32X64.dsc
> Flash Image Definition   = d:\edk2\OvmfPkg\OvmfPkgIa32X64.fdf
> 
> Processing meta-data ... done!
> Generating code . done!
> Generating makefile . done!
> Generating code .. done!
> Generating makefile .. done!
> 
> - Done -
> Build end time: 10:12:49, Nov.09 2018
> Build total time: 00:00:17
> 
> After applying this patch:
> 
> d:\edk2 (master -> origin)
> λ build genmake -p OvmfPkg\OvmfPkgIa32X64.dsc -a IA32 -a X64 -
> Build environment: Windows-10-10.0.10240  
> Build start time: 10:11:41, Nov.09 2018   
>   
> WORKSPACE= d:\edk2
> ECP_SOURCE 

Re: [edk2] [Patch] BaseTools: Optimize string concatenation

2018-11-10 Thread Feng, Bob C
Hi Leif,

I use my desktop to do the benchmark.
CPU: Intel(R) Core(TM) i7-6700 CPU @ 3.40GHz
Memory: 16G
OS: Win10

I'll add the performance detailed data to BZ.

-Bob

-Original Message-
From: Leif Lindholm [mailto:leif.lindh...@linaro.org] 
Sent: Friday, November 9, 2018 7:49 PM
To: Feng, Bob C 
Cc: edk2-devel@lists.01.org; Carsey, Jaben ; Gao, 
Liming 
Subject: Re: [edk2] [Patch] BaseTools: Optimize string concatenation

Hi Bob,

On Fri, Nov 09, 2018 at 03:25:04AM +, Feng, Bob C wrote:
> Yes. I should show the data.
> 
> My unites scripts is as below. The parameter lines is a string list 
> which size is 43395. The test result is
> 
> ''.join(String list) time:  0.042262
> String += String time  :  3.822699
> 
> def TestPlus(lines):
> str_target = ""
> 
> for line in lines:
> str_target += line
> 
> return str_target
> 
> def TestJoin(lines):
> str_target = []
> 
> for line in lines:
> str_target.append(line)
> 
> return "".join(str_target)
> 
> def CompareStrCat():
> lines = GetStrings()
> print (len(lines))
> 
> begin = time.perf_counter()
> for _ in range(10):
> TestJoin(lines)
> end = time.perf_counter() - begin
> print ("''.join(String list) time: %f" % end)
> 
> begin = time.perf_counter()
> for _ in range(10):
> TestPlus(lines)
> end = time.perf_counter() - begin
> print ("String += String time: %f" % end)
> 
> For build OvmfX64, it's not very effective, it saves 2~3 second in 
> Parse/AutoGen phase, because OvmfX64 is relatively simple. It does not 
> enable much features such as Multiple SKU and structure PCD by default 
> and there is no big size Autogen.c/Autogen.h/Makefile generated 
> either. but for the complex platform, this patch will be much 
> effective. The unites above simulates a real case that there is a 
> 43395 lines of Autogen.c generated.

I beg to differ. Shaving 2-3 seconds off the autogen phase is a substantial 
improvement.

However, on my wimpy 24-core Cortex-A53 system, the effect is not noticeable 
(fluctuates between 1:56-1:58 whether with or without this patch).

And even on my x86 workstation, I see no measurable difference (12-13s).
What is the hardware you are benchmarking on?

It does not appear to be detrimental to performance on any of my platforms, but 
I would like to see some more measurements, and I would like to see that logged 
with some more detail in bugzilla.

Regards,

Leif

> Since this patch mostly effect the Parser/AutoGen phase, I just use 
> "build genmake" to show the improvement data.
> The final result for clean build is:
> Current code:  17 seconds
> After patch:  15 seconds
> 
> Details:
> Current data:
> 
> d:\edk2 (master -> origin)
> λ build genmake -p OvmfPkg\OvmfPkgIa32X64.dsc -a IA32 -a X64 -t 
> VS2015x86 Build environment: Windows-10-10.0.10240 Build start time: 
> 10:12:32, Nov.09 2018
> 
> WORKSPACE= d:\edk2
> ECP_SOURCE   = d:\edk2\edkcompatibilitypkg
> EDK_SOURCE   = d:\edk2\edkcompatibilitypkg
> EFI_SOURCE   = d:\edk2\edkcompatibilitypkg
> EDK_TOOLS_PATH   = d:\edk2\basetools
> EDK_TOOLS_BIN= d:\edk2\basetools\bin\win32
> CONF_PATH= d:\edk2\conf
> 
> Architecture(s)  = IA32 X64
> Build target = DEBUG
> Toolchain= VS2015x86
> 
> Active Platform  = d:\edk2\OvmfPkg\OvmfPkgIa32X64.dsc
> Flash Image Definition   = d:\edk2\OvmfPkg\OvmfPkgIa32X64.fdf
> 
> Processing meta-data ... done!
> Generating code . done!
> Generating makefile . done!
> Generating code .. done!
> Generating makefile .. done!
> 
> - Done -
> Build end time: 10:12:49, Nov.09 2018
> Build total time: 00:00:17
> 
> After applying this patch:
> 
> d:\edk2 (master -> origin)
> λ build genmake -p OvmfPkg\OvmfPkgIa32X64.dsc -a IA32 -a X64 -
> Build environment: Windows-10-10.0.10240  
> Build start time: 10:11:41, Nov.09 2018   
>   
> WORKSPACE= d:\edk2
> ECP_SOURCE   = d:\edk2\edkcompatibilitypkg
> EDK_SOURCE   = d:\edk2\edkcompatibilitypkg
> EFI_SOURCE   = d:\edk2\edkcompatibilitypkg
> EDK_TOOLS_PATH   = d:\edk2\basetools  
> EDK_TOOLS_BIN= d:\edk2\basetools\bin\win32
> CONF_PATH= d:\edk2\conf   
>   
>  

Re: [edk2] [Patch] BaseTools: Optimize string concatenation

2018-11-09 Thread Kinney, Michael D
Liming,

If we can support both Python2 and Python3 equally,
then I agree that these types of performance enhancements
can be added to edk2/master after the stable tag.

Let's make sure we enter a BZ for each performance
improvement and as Leif has asked, put evidence of the
performance improvement in the BZ.

Mike

> -Original Message-
> From: Gao, Liming
> Sent: Friday, November 9, 2018 6:17 AM
> To: Kinney, Michael D ;
> Feng, Bob C ; edk2-
> de...@lists.01.org
> Cc: Carsey, Jaben 
> Subject: RE: [edk2] [Patch] BaseTools: Optimize string
> concatenation
> 
> 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 ; edk2-
> de...@lists.01.org; Kinney, Michael D
> 
> > Cc: Carsey, Jaben ; Gao,
> Liming 
> > 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-
> > > boun...@lists.01.org] On Behalf Of BobCF
> > > Sent: Thursday, November 8, 2018 2:16 AM
> > > To: edk2-devel@lists.01.org
> > > Cc: Carsey, Jaben ; Gao,
> Liming
> > > 
> > > 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 
> > > Cc: Liming Gao 
> > > Cc: Jaben Carsey 
> > > ---
> > >  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):
> > >  

Re: [edk2] [Patch] BaseTools: Optimize string concatenation

2018-11-09 Thread Gao, Liming
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 ; edk2-devel@lists.01.org; Kinney, 
> Michael D 
> Cc: Carsey, Jaben ; Gao, Liming 
> 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-
> > boun...@lists.01.org] On Behalf Of BobCF
> > Sent: Thursday, November 8, 2018 2:16 AM
> > To: edk2-devel@lists.01.org
> > Cc: Carsey, Jaben ; Gao, Liming
> > 
> > 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 
> > Cc: Liming Gao 
> > Cc: Jaben Carsey 
> > ---
> >  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 Cre

Re: [edk2] [Patch] BaseTools: Optimize string concatenation

2018-11-09 Thread Leif Lindholm
g\OvmfPkgIa32X64.fdf 
>   
> Processing meta-data . done!  
> Generating code . done!   
> Generating makefile . done!   
> Generating code .. done!  
> Generating makefile .. done!  
>   
> - Done -          
> Build end time: 10:11:56, Nov.09 2018 
> Build total time: 00:00:15
> 
> 
> Thanks,
> Bob
> 
> 
> -Original Message-
> From: Leif Lindholm [mailto:leif.lindh...@linaro.org] 
> Sent: Friday, November 9, 2018 12:53 AM
> To: Feng, Bob C 
> Cc: edk2-devel@lists.01.org; Carsey, Jaben ; Gao, 
> Liming 
> Subject: Re: [edk2] [Patch] BaseTools: Optimize string concatenation
> 
> On Thu, Nov 08, 2018 at 06:16:25PM +0800, BobCF wrote:
> > 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.
> 
> Do you have any numbers on the level of improvement seen?
> 
> Either for the individual scripts when called identically, or (if
> measurable) on the build of an entire platform (say OvmfX64?).
> 
> Regards,
> 
> Leif
> 
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: BobCF 
> > Cc: Liming Gao 
> > Cc: Jaben Carsey 
> > ---
> >  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):
>

Re: [edk2] [Patch] BaseTools: Optimize string concatenation

2018-11-08 Thread Feng, Bob C
Hi Leif,

Yes. I should show the data.

My unites scripts is as below. The parameter lines is a string list which size 
is 43395. The test result is

''.join(String list) time:  0.042262
String += String time  :  3.822699

def TestPlus(lines):
str_target = ""

for line in lines:
str_target += line

return str_target

def TestJoin(lines):
str_target = []

for line in lines:
str_target.append(line)

return "".join(str_target)

def CompareStrCat():
lines = GetStrings()
print (len(lines))

begin = time.perf_counter()
for _ in range(10):
TestJoin(lines)
end = time.perf_counter() - begin
print ("''.join(String list) time: %f" % end)

begin = time.perf_counter()
for _ in range(10):
TestPlus(lines)
end = time.perf_counter() - begin
print ("String += String time: %f" % end)

For build OvmfX64, it's not very effective, it saves 2~3 second in 
Parse/AutoGen phase, because OvmfX64 is relatively simple. It does not enable 
much features such as Multiple SKU and structure PCD by default and there is no 
big size Autogen.c/Autogen.h/Makefile generated either. but for the complex 
platform, this patch will be much effective. The unites above simulates a real 
case that there is a 43395 lines of Autogen.c generated.

Since this patch mostly effect the Parser/AutoGen phase, I just use "build 
genmake" to show the improvement data. 
The final result for clean build is:
Current code:  17 seconds
After patch:  15 seconds

Details:
Current data:

d:\edk2 (master -> origin)
λ build genmake -p OvmfPkg\OvmfPkgIa32X64.dsc -a IA32 -a X64 -t VS2015x86
Build environment: Windows-10-10.0.10240
Build start time: 10:12:32, Nov.09 2018

WORKSPACE= d:\edk2
ECP_SOURCE   = d:\edk2\edkcompatibilitypkg
EDK_SOURCE   = d:\edk2\edkcompatibilitypkg
EFI_SOURCE   = d:\edk2\edkcompatibilitypkg
EDK_TOOLS_PATH   = d:\edk2\basetools
EDK_TOOLS_BIN= d:\edk2\basetools\bin\win32
CONF_PATH= d:\edk2\conf

Architecture(s)  = IA32 X64
Build target = DEBUG
Toolchain= VS2015x86

Active Platform  = d:\edk2\OvmfPkg\OvmfPkgIa32X64.dsc
Flash Image Definition   = d:\edk2\OvmfPkg\OvmfPkgIa32X64.fdf

Processing meta-data ... done!
Generating code . done!
Generating makefile . done!
Generating code .. done!
Generating makefile .. done!

- Done -
Build end time: 10:12:49, Nov.09 2018
Build total time: 00:00:17

After applying this patch:

d:\edk2 (master -> origin)
λ build genmake -p OvmfPkg\OvmfPkgIa32X64.dsc -a IA32 -a X64 -
Build environment: Windows-10-10.0.10240  
Build start time: 10:11:41, Nov.09 2018   
  
WORKSPACE= d:\edk2
ECP_SOURCE   = d:\edk2\edkcompatibilitypkg
EDK_SOURCE   = d:\edk2\edkcompatibilitypkg
EFI_SOURCE   = d:\edk2\edkcompatibilitypkg
EDK_TOOLS_PATH   = d:\edk2\basetools  
EDK_TOOLS_BIN= d:\edk2\basetools\bin\win32
CONF_PATH= d:\edk2\conf   
  
  
Architecture(s)  = IA32 X64   
Build target = DEBUG  
Toolchain= VS2015x86  
  
Active Platform  = d:\edk2\OvmfPkg\OvmfPkgIa32X64.dsc 
Flash Image Definition   = d:\edk2\OvmfPkg\OvmfPkgIa32X64.fdf 
  
Processing meta-data . done!  
Generating code . done!   
Generating makefile . done!   
Generating code .. done!  
Generating makefile .. done!  
  
- Done -  
Build end time: 10:11:56, Nov.09 2018 
Build total time: 00:00:15


Thanks,
Bob


-Original Message-
From: Leif Lindholm [mailto:leif.lindh...@linaro.org] 
Sent: Friday, November 9, 2018 12:53 AM
To: Feng, Bob C 
Cc: edk2-devel@lists.01.org; Carsey, Jaben ; Gao, 
Liming 
Subject: Re: [edk2] [Patch] BaseTools: Optimize string concatenation

On Thu, Nov 08, 2018 at 06:16:25PM +0800, BobCF wrote:
> 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 

Re: [edk2] [Patch] BaseTools: Optimize string concatenation

2018-11-08 Thread Leif Lindholm
On Thu, Nov 08, 2018 at 06:16:25PM +0800, BobCF wrote:
> 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.

Do you have any numbers on the level of improvement seen?

Either for the individual scripts when called identically, or (if
measurable) on the build of an entire platform (say OvmfX64?).

Regards,

Leif

> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: BobCF 
> Cc: Liming Gao 
> Cc: Jaben Carsey 
> ---
>  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
>  #
>  # 

Re: [edk2] [Patch] BaseTools: Optimize string concatenation

2018-11-08 Thread Kinney, Michael D
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-
> boun...@lists.01.org] On Behalf Of BobCF
> Sent: Thursday, November 8, 2018 2:16 AM
> To: edk2-devel@lists.01.org
> Cc: Carsey, Jaben ; Gao, Liming
> 
> 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 
> Cc: Liming Gao 
> Cc: Jaben Carsey 
> ---
>  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
>  #
>  # 

Re: [edk2] [Patch] BaseTools: Optimize string concatenation

2018-11-08 Thread Carsey, Jaben
Reviewed-by: Jaben Carsey 

> -Original Message-
> From: Feng, Bob C
> Sent: Thursday, November 08, 2018 2:16 AM
> To: edk2-devel@lists.01.org
> Cc: Gao, Liming ; Carsey, Jaben
> 
> 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 
> Cc: Liming Gao 
> Cc: Jaben Carsey 
> ---
>  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
>  #
>  # 

[edk2] [Patch] BaseTools: Optimize string concatenation

2018-11-08 Thread BobCF
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 
Cc: Liming Gao 
Cc: Jaben Carsey 
---
 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
 
 #
 #