From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 192.55.52.115, mailfrom: christian.rodriguez@intel.com) Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by groups.io with SMTP; Tue, 11 Jun 2019 08:41:22 -0700 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga002.jf.intel.com ([10.7.209.21]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 11 Jun 2019 08:41:22 -0700 X-ExtLoop1: 1 Received: from orsmsx103.amr.corp.intel.com ([10.22.225.130]) by orsmga002.jf.intel.com with ESMTP; 11 Jun 2019 08:41:21 -0700 Received: from orsmsx114.amr.corp.intel.com ([169.254.8.154]) by ORSMSX103.amr.corp.intel.com ([169.254.5.232]) with mapi id 14.03.0415.000; Tue, 11 Jun 2019 08:41:21 -0700 From: "Christian Rodriguez" To: "Shi, Steven" , "devel@edk2.groups.io" CC: "Gao, Liming" , "Feng, Bob C" Subject: Re: [PATCH] BaseTools: Cannot store library cache of different arch together Thread-Topic: [PATCH] BaseTools: Cannot store library cache of different arch together Thread-Index: AQHVIBoH6t1g74NJIEupoDm7H5v6mKaWk3Qw Date: Tue, 11 Jun 2019 15:41:20 +0000 Message-ID: <3A7DCC9A944C6149BF832E1C9B718ABC01F42EC0@ORSMSX114.amr.corp.intel.com> References: <20190611055327.18508-1-steven.shi@intel.com> In-Reply-To: <20190611055327.18508-1-steven.shi@intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ctpclassification: CTP_NT x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMzljN2ZmODItYTBjYy00OTFhLWE5NzctMGNmOTkxNzVkMTkyIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiYlJuYWNsWmNuTHBieHpGUDQ5SkRsNWZSdUJGV01sUStYaVFLZU9QYTlRcnNrRkRNekFoSWN0cENuek1JVEVLRSJ9 dlp-product: dlpe-windows dlp-version: 11.2.0.6 dlp-reaction: no-action x-originating-ip: [10.22.254.138] MIME-Version: 1.0 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable The change looks good overall, but I'm concerned about the performance if t= here are multiple of the same library used by different modules. In this ca= se 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 wo= uld 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 u= nique tuple pair like (lib.arch, lib) in the set to reduce the libraries be= ing copied to a unique subset. Thanks, Christian >-----Original Message----- >From: Shi, Steven >Sent: Monday, June 10, 2019 10:53 PM >To: devel@edk2.groups.io >Cc: Gao, Liming ; Feng, Bob C >; Rodriguez, Christian > >Subject: [PATCH] BaseTools: Cannot store library cache of different arch >together > >https://bugzilla.tianocore.org/show_bug.cgi?id=3D1895 > >Build cache cannot store cache for the same library modules in different a= rch >together. E.g. Both the below IA32 and X64 arch BaseLib caches should exis= t >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_ valu= e >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 >Cc: Bob Feng >Cc: Christian Rodriguez >Signed-off-by: Steven Shi >--- > 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), Tr= ue) > > def CreateAsBuiltInf(self): >- all_lib_set =3D set() >- all_mod_set =3D 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 =3D [] > self.HashSkipModules =3D [] > ## Do some clean-up works when error occurred >-- >2.17.1.windows.2