<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:dt="uuid:C2F41010-65B3-11d1-A29F-00AA00C14882" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40"> <head> <meta http-equiv="Content-Type" content="text/html; charset=us-ascii"> <meta name="Generator" content="Microsoft Word 15 (filtered medium)"> <style></style> </head> <body lang="EN-US" link="#0563C1" vlink="#954F72"> <div class="WordSection1"> <a name="_MailEndCompose">Hi Christian,<o:p></o:p></a> For the extra IO accesses for duplicated library, I plan to introduce the CopyFileOnChange() function to solve it. Below is the CopyFileOnChange() BZ, and I haven’t sent its patch yet. The CopyFileOnChange() will ensure only once IO store/restore access for each library. To avoid duplicated library access is critical not only for the performance, but also for the writing data race in multi-threads.<o:p></o:p> <o:p> </o:p> <o:p> </o:p> Basetool need CopyFileOnChange function to avoid cache file writing race in multi-thread build<o:p></o:p> <a href="https://bugzilla.tianocore.org/show_bug.cgi?id=1894">https://bugzilla.tianocore.org/show_bug.cgi?id=1894</a> <o:p></o:p> <o:p> </o:p> <o:p> </o:p> Thanks<o:p></o:p> <o:p> </o:p> Steven Shi<o:p></o:p> Intel\SSG\FID\Firmware Infrastructure<o:p></o:p> <o:p> </o:p> <a name="_____replyseparator"></a>> -----Original Message----- > From: Rodriguez, Christian > Sent: Tuesday, June 11, 2019 11:41 PM > To: Shi, Steven <steven.shi@intel.com>; devel@edk2.groups.io > Cc: Gao, Liming <liming.gao@intel.com>; Feng, Bob C <bob.c.feng@intel.com> > Subject: RE: [PATCH] BaseTools: Cannot store library cache of different arch > together > > The change looks good overall, but I'm concerned about the performance if > there are multiple of the same library used by different modules. In this case > the library will be copied once per arch per module. I'm not sure if it would > make a giant impact, but just something to think about because you would be > doing extra IO accesses for each duplicate library autogen. > > Possible suggestion: > Is it possible to change the hash to include the arch? Or you can store a > unique tuple pair like (lib.arch, lib) in the set to reduce the libraries being > copied to a unique subset. > > Thanks, > Christian > > >-----Original Message----- > >From: Shi, Steven > >Sent: Monday, June 10, 2019 10:53 PM > >To: <a href="mailto:devel@edk2.groups.io">devel@edk2.groups.io</a> > >Cc: Gao, Liming <<a href="mailto:liming.gao@intel.com">liming.gao@intel.com</a>>; Feng, Bob C > ><<a href="mailto:bob.c.feng@intel.com">bob.c.feng@intel.com</a>>; Rodriguez, Christian > ><<a href="mailto:christian.rodriguez@intel.com">christian.rodriguez@intel.com</a>> > >Subject: [PATCH] BaseTools: Cannot store library cache of different arch > >together > > > ><a href="https://bugzilla.tianocore.org/show_bug.cgi?id=1895">https://bugzilla.tianocore.org/show_bug.cgi?id=1895</a> > > > >Build cache cannot store cache for the same library modules in different arch > >together. E.g. Both the below IA32 and X64 arch BaseLib caches should exist > >after build Ovmf3264, but now only the one in X64 arch exist. > >The reason is the current Basetool use a set() to same all library AutoGen > >objects, but the different arch lib AutoGen objects have same __hash_ value > >which comes from the lib MetaFile(The path of module file): > > def __hash__(self): > > return hash(self.MetaFile) > > > >So the different arch lib AutoGen objects are duplicated one to the set() and > >only one can exist. This is why the Basetool can only store one arch cache for > >library. > >This patch avoid to use the set() as the container to save the library and > >module objects because the objects might have same __hash__ value. > > > >Cc: Liming Gao <<a href="mailto:liming.gao@intel.com">liming.gao@intel.com</a>> > >Cc: Bob Feng <<a href="mailto:bob.c.feng@intel.com">bob.c.feng@intel.com</a>> > >Cc: Christian Rodriguez <<a href="mailto:christian.rodriguez@intel.com">christian.rodriguez@intel.com</a>> > >Signed-off-by: Steven Shi <<a href="mailto:steven.shi@intel.com">steven.shi@intel.com</a>> > >--- > > BaseTools/Source/Python/build/build.py | 13 +++---------- > > 1 file changed, 3 insertions(+), 10 deletions(-) > > > >diff --git a/BaseTools/Source/Python/build/build.py > >b/BaseTools/Source/Python/build/build.py > >index 0855d4561c..f7a79cbbab 100644 > >--- a/BaseTools/Source/Python/build/build.py > >+++ b/BaseTools/Source/Python/build/build.py > >@@ -2203,21 +2203,14 @@ class Build(): > > RemoveDirectory(os.path.dirname(GlobalData.gDatabasePath), True) > > > > def CreateAsBuiltInf(self): > >- all_lib_set = set() > >- all_mod_set = set() > > for Module in self.BuildModules: > > Module.CreateAsBuiltInf() > >- all_mod_set.add(Module) > >+ for lib in Module.LibraryAutoGenList: > >+ lib.CreateAsBuiltInf(True) > > for Module in self.HashSkipModules: > > Module.CreateAsBuiltInf(True) > >- all_mod_set.add(Module) > >- for Module in all_mod_set: > > for lib in Module.LibraryAutoGenList: > >- all_lib_set.add(lib) > >- for lib in all_lib_set: > >- lib.CreateAsBuiltInf(True) > >- all_lib_set.clear() > >- all_mod_set.clear() > >+ lib.CreateAsBuiltInf(True) > > self.BuildModules = [] > > self.HashSkipModules = [] > > ## Do some clean-up works when error occurred > >-- > >2.17.1.windows.2 <o:p> </o:p> </div> </body> </html> <div width="1" style="color:white;clear:both">_._,_._,_</div> <hr> Groups.io Links: You receive all messages sent to this group. <a target="_blank" href="https://edk2.groups.io/g/devel/message/42244">View/Reply Online (#42244)</a> | | <a target="_blank" href="https://groups.io/mt/32013233/1813853">Mute This Topic</a> | <a href="https://edk2.groups.io/g/devel/post">New Topic</a> <a href="https://edk2.groups.io/g/devel/editsub/1813853">Your Subscription</a> | <a href="mailto:devel+owner@edk2.groups.io">Contact Group Owner</a> | <a href="https://edk2.groups.io/g/devel/unsub">Unsubscribe</a> [edk2-devel-archive@redhat.com] <div width="1" style="color:white;clear:both">_._,_._,_</div>