[edk2] [Patch] BaseTools: Optimize string concatenation

Feng, Bob C bob.c.feng at intel.com
Tue Dec 11 00:48:19 PST 2018


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.lindholm at linaro.org] 
Sent: Monday, December 10, 2018 8:36 PM
To: Feng, Bob C <bob.c.feng at intel.com>
Cc: edk2-devel at lists.01.org; Carsey, Jaben <jaben.carsey at intel.com>; Gao, Liming <liming.gao at intel.com>
Subject: Re: [edk2] [Patch] BaseTools: Optimize string concatenation

On Mon, Dec 10, 2018 at 12:09:23PM +0000, 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


More information about the edk2-devel mailing list