public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH] BaseTools:Extend the binary cache to support library cache
@ 2019-05-28  8:51 Steven Shi
  2019-05-28 15:15 ` [edk2-devel] " Christian Rodriguez
  0 siblings, 1 reply; 3+ messages in thread
From: Steven Shi @ 2019-05-28  8:51 UTC (permalink / raw)
  To: devel; +Cc: liming.gao, bob.c.feng, christian.rodriguez, zhijux.fan

https://bugzilla.tianocore.org/show_bug.cgi?id=1797

Current binary cache doesn't support to save and restore
the library module. If a driver module cache miss happen,
all its dependency library modules need rebuild which
is very time-consuming. This patch is to entend the binary
cache to support library.

Cc: Liming Gao <liming.gao@intel.com>
Cc: Bob Feng <bob.c.feng@intel.com>
Cc: Christian Rodriguez <christian.rodriguez@intel.com>
Signed-off-by: Steven Shi <steven.shi@intel.com>
---
 BaseTools/Source/Python/AutoGen/AutoGen.py | 17 ++++++++++++++---
 BaseTools/Source/Python/build/build.py     |  3 ++-
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py b/BaseTools/Source/Python/AutoGen/AutoGen.py
index a5bef4f7c6..aeb63f52c5 100644
--- a/BaseTools/Source/Python/AutoGen/AutoGen.py
+++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
@@ -2578,7 +2578,7 @@ class ModuleAutoGen(AutoGen):
         self.AutoGenDepSet = set()
         self.ReferenceModules = []
         self.ConstPcd                  = {}
-
+        self.CacheRestored             = False
 
     def __repr__(self):
         return "%s [%s]" % (self.MetaFile, self.Arch)
@@ -3906,6 +3906,12 @@ class ModuleAutoGen(AutoGen):
             ModuleFile = path.join(self.OutputDir, self.Name + '.inf')
             if os.path.exists(ModuleFile):
                 shutil.copy2(ModuleFile, FileDir)
+        else:
+            OutputDir = self.OutputDir.replace('\\', '/').strip('/')
+            DebugDir = self.DebugDir.replace('\\', '/').strip('/')
+            for Item in self.CodaTargetList:
+                File = Item.Target.Path.replace('\\', '/').strip('/').replace(DebugDir, '').replace(OutputDir, '').strip('/')
+                self.OutputFile.add(File)
         if not self.OutputFile:
             Ma = self.BuildDatabase[self.MetaFile, self.Arch, self.BuildTarget, self.ToolChain]
             self.OutputFile = Ma.Binaries
@@ -3949,6 +3955,7 @@ class ModuleAutoGen(AutoGen):
                                 destination_dir = os.path.dirname(destination_file)
                                 CreateDirectory(destination_dir)
                                 shutil.copy2(File, destination_dir)
+                    self.CacheRestored = True
                     if self.Name == "PcdPeim" or self.Name == "PcdDxe":
                         CreatePcdDatabaseCode(self, TemplateString(), TemplateString())
                     return True
@@ -3987,7 +3994,9 @@ class ModuleAutoGen(AutoGen):
         self.GenFfsList = GenFfsList
         if not self.IsLibrary and CreateLibraryMakeFile:
             for LibraryAutoGen in self.LibraryAutoGenList:
-                LibraryAutoGen.CreateMakeFile()
+                # Only create makefile for libraries which have not been restored
+                if not LibraryAutoGen.CacheRestored:
+                    LibraryAutoGen.CreateMakeFile()
 
         if self.CanSkip():
             return
@@ -4030,7 +4039,9 @@ class ModuleAutoGen(AutoGen):
 
         if not self.IsLibrary and CreateLibraryCodeFile:
             for LibraryAutoGen in self.LibraryAutoGenList:
-                LibraryAutoGen.CreateCodeFile()
+                # Only create autogen code for libraries which have not been restored
+                if not LibraryAutoGen.CacheRestored:
+                    LibraryAutoGen.CreateCodeFile()
 
         if self.CanSkip():
             return
diff --git a/BaseTools/Source/Python/build/build.py b/BaseTools/Source/Python/build/build.py
index 80ceb98310..f1f4c07980 100644
--- a/BaseTools/Source/Python/build/build.py
+++ b/BaseTools/Source/Python/build/build.py
@@ -341,7 +341,8 @@ class ModuleMakeUnit(BuildUnit):
     #   @param  Target      The build target name, one of gSupportedTarget
     #
     def __init__(self, Obj, Target):
-        Dependency = [ModuleMakeUnit(La, Target) for La in Obj.LibraryAutoGenList]
+        # Skip the dependency modules which are already restored from cache
+        Dependency = [ModuleMakeUnit(La, Target) for La in Obj.LibraryAutoGenList if not La.CacheRestored]
         BuildUnit.__init__(self, Obj, Obj.BuildCommand, Target, Dependency, Obj.MakeFileDir)
         if Target in [None, "", "all"]:
             self.Target = "tbuild"
-- 
2.17.1.windows.2


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [edk2-devel] [PATCH] BaseTools:Extend the binary cache to support library cache
  2019-05-28  8:51 [PATCH] BaseTools:Extend the binary cache to support library cache Steven Shi
@ 2019-05-28 15:15 ` Christian Rodriguez
  2019-05-29  0:59   ` Steven Shi
  0 siblings, 1 reply; 3+ messages in thread
From: Christian Rodriguez @ 2019-05-28 15:15 UTC (permalink / raw)
  To: devel@edk2.groups.io, Shi, Steven; +Cc: Gao, Liming, Feng, Bob C, Fan, ZhijuX

[-- Attachment #1: Type: text/plain, Size: 5532 bytes --]

Hi Steven,

After testing the bug reported, it seems that the dependency problem isn't the root cause. We don't need to change the way it skips having to rebuild libraries. The cache is missing the library build artifacts as you mentioned before and this is the root cause of the rebuild failure. This patch should fix that, but you can simplify it by not adding a new tracking system and keeping the dependency reduction as before. That will save the performance because it doesn't have to do extra checking.

Please see attached patch for minimal change needed.

Thanks,
Christian

>-----Original Message-----
>From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
>Steven Shi
>Sent: Tuesday, May 28, 2019 1:52 AM
>To: devel@edk2.groups.io
>Cc: Gao, Liming <liming.gao@intel.com>; Feng, Bob C
><bob.c.feng@intel.com>; Rodriguez, Christian
><christian.rodriguez@intel.com>; Fan, ZhijuX <zhijux.fan@intel.com>
>Subject: [edk2-devel] [PATCH] BaseTools:Extend the binary cache to support
>library cache
>
>https://bugzilla.tianocore.org/show_bug.cgi?id=1797
>
>Current binary cache doesn't support to save and restore
>the library module. If a driver module cache miss happen,
>all its dependency library modules need rebuild which
>is very time-consuming. This patch is to entend the binary
>cache to support library.
>
>Cc: Liming Gao <liming.gao@intel.com>
>Cc: Bob Feng <bob.c.feng@intel.com>
>Cc: Christian Rodriguez <christian.rodriguez@intel.com>
>Signed-off-by: Steven Shi <steven.shi@intel.com>
>---
> BaseTools/Source/Python/AutoGen/AutoGen.py | 17 ++++++++++++++---
> BaseTools/Source/Python/build/build.py     |  3 ++-
> 2 files changed, 16 insertions(+), 4 deletions(-)
>
>diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py
>b/BaseTools/Source/Python/AutoGen/AutoGen.py
>index a5bef4f7c6..aeb63f52c5 100644
>--- a/BaseTools/Source/Python/AutoGen/AutoGen.py
>+++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
>@@ -2578,7 +2578,7 @@ class ModuleAutoGen(AutoGen):
>         self.AutoGenDepSet = set()
>         self.ReferenceModules = []
>         self.ConstPcd                  = {}
>-
>+        self.CacheRestored             = False
>
>     def __repr__(self):
>         return "%s [%s]" % (self.MetaFile, self.Arch)
>@@ -3906,6 +3906,12 @@ class ModuleAutoGen(AutoGen):
>             ModuleFile = path.join(self.OutputDir, self.Name + '.inf')
>             if os.path.exists(ModuleFile):
>                 shutil.copy2(ModuleFile, FileDir)
>+        else:
>+            OutputDir = self.OutputDir.replace('\\', '/').strip('/')
>+            DebugDir = self.DebugDir.replace('\\', '/').strip('/')
>+            for Item in self.CodaTargetList:
>+                File = Item.Target.Path.replace('\\', '/').strip('/').replace(DebugDir,
>'').replace(OutputDir, '').strip('/')
>+                self.OutputFile.add(File)
>         if not self.OutputFile:
>             Ma = self.BuildDatabase[self.MetaFile, self.Arch, self.BuildTarget,
>self.ToolChain]
>             self.OutputFile = Ma.Binaries
>@@ -3949,6 +3955,7 @@ class ModuleAutoGen(AutoGen):
>                                 destination_dir = os.path.dirname(destination_file)
>                                 CreateDirectory(destination_dir)
>                                 shutil.copy2(File, destination_dir)
>+                    self.CacheRestored = True
>                     if self.Name == "PcdPeim" or self.Name == "PcdDxe":
>                         CreatePcdDatabaseCode(self, TemplateString(),
>TemplateString())
>                     return True
>@@ -3987,7 +3994,9 @@ class ModuleAutoGen(AutoGen):
>         self.GenFfsList = GenFfsList
>         if not self.IsLibrary and CreateLibraryMakeFile:
>             for LibraryAutoGen in self.LibraryAutoGenList:
>-                LibraryAutoGen.CreateMakeFile()
>+                # Only create makefile for libraries which have not been restored
>+                if not LibraryAutoGen.CacheRestored:
>+                    LibraryAutoGen.CreateMakeFile()
>
>         if self.CanSkip():
>             return
>@@ -4030,7 +4039,9 @@ class ModuleAutoGen(AutoGen):
>
>         if not self.IsLibrary and CreateLibraryCodeFile:
>             for LibraryAutoGen in self.LibraryAutoGenList:
>-                LibraryAutoGen.CreateCodeFile()
>+                # Only create autogen code for libraries which have not been
>restored
>+                if not LibraryAutoGen.CacheRestored:
>+                    LibraryAutoGen.CreateCodeFile()
>
>         if self.CanSkip():
>             return
>diff --git a/BaseTools/Source/Python/build/build.py
>b/BaseTools/Source/Python/build/build.py
>index 80ceb98310..f1f4c07980 100644
>--- a/BaseTools/Source/Python/build/build.py
>+++ b/BaseTools/Source/Python/build/build.py
>@@ -341,7 +341,8 @@ class ModuleMakeUnit(BuildUnit):
>     #   @param  Target      The build target name, one of gSupportedTarget
>     #
>     def __init__(self, Obj, Target):
>-        Dependency = [ModuleMakeUnit(La, Target) for La in
>Obj.LibraryAutoGenList]
>+        # Skip the dependency modules which are already restored from cache
>+        Dependency = [ModuleMakeUnit(La, Target) for La in
>Obj.LibraryAutoGenList if not La.CacheRestored]
>         BuildUnit.__init__(self, Obj, Obj.BuildCommand, Target, Dependency,
>Obj.MakeFileDir)
>         if Target in [None, "", "all"]:
>             self.Target = "tbuild"
>--
>2.17.1.windows.2
>
>
>


[-- Attachment #2: 0001-Add-library-copy-to-cache.patch --]
[-- Type: application/octet-stream, Size: 1335 bytes --]

From 97127828385c095013a90a20fd8c47e7e3ea0c2d Mon Sep 17 00:00:00 2001
From: Christian Rodriguez <christian.rodriguez@intel.com>
Date: Tue, 28 May 2019 07:58:05 -0700
Subject: [PATCH] Add library copy to cache

---
 BaseTools/Source/Python/AutoGen/AutoGen.py | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py b/BaseTools/Source/Python/AutoGen/AutoGen.py
index a376bc24d6..4e4794fd93 100644
--- a/BaseTools/Source/Python/AutoGen/AutoGen.py
+++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
@@ -3906,6 +3906,12 @@ class ModuleAutoGen(AutoGen):
             ModuleFile = path.join(self.OutputDir, self.Name + '.inf')
             if os.path.exists(ModuleFile):
                 shutil.copy2(ModuleFile, FileDir)
+        else:
+            OutputDir = self.OutputDir.replace('\\', '/').strip('/')
+            DebugDir = self.DebugDir.replace('\\', '/').strip('/')
+            for Item in self.CodaTargetList:
+                File = Item.Target.Path.replace('\\', '/').strip('/').replace(DebugDir, '').replace(OutputDir, '').strip('/')
+                self.OutputFile.add(File)
         if not self.OutputFile:
             Ma = self.BuildDatabase[self.MetaFile, self.Arch, self.BuildTarget, self.ToolChain]
             self.OutputFile = Ma.Binaries
-- 
2.21.0.windows.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [edk2-devel] [PATCH] BaseTools:Extend the binary cache to support library cache
  2019-05-28 15:15 ` [edk2-devel] " Christian Rodriguez
@ 2019-05-29  0:59   ` Steven Shi
  0 siblings, 0 replies; 3+ messages in thread
From: Steven Shi @ 2019-05-29  0:59 UTC (permalink / raw)
  To: Rodriguez, Christian, devel@edk2.groups.io
  Cc: Gao, Liming, Feng, Bob C, Fan, ZhijuX

OK, will send the new version patch to simplify it.


Thanks

Steven Shi
Intel\SSG\FID\Firmware Infrastructure


> -----Original Message-----
> From: Rodriguez, Christian
> Sent: Tuesday, May 28, 2019 11:16 PM
> To: devel@edk2.groups.io; Shi, Steven <steven.shi@intel.com>
> Cc: Gao, Liming <liming.gao@intel.com>; Feng, Bob C <bob.c.feng@intel.com>;
> Fan, ZhijuX <zhijux.fan@intel.com>
> Subject: RE: [edk2-devel] [PATCH] BaseTools:Extend the binary cache to
> support library cache
> 
> Hi Steven,
> 
> After testing the bug reported, it seems that the dependency problem isn't
> the root cause. We don't need to change the way it skips having to rebuild
> libraries. The cache is missing the library build artifacts as you mentioned
> before and this is the root cause of the rebuild failure. This patch should fix
> that, but you can simplify it by not adding a new tracking system and keeping
> the dependency reduction as before. That will save the performance because
> it doesn't have to do extra checking.
> 
> Please see attached patch for minimal change needed.
> 
> Thanks,
> Christian
> 
> >-----Original Message-----
> >From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> >Steven Shi
> >Sent: Tuesday, May 28, 2019 1:52 AM
> >To: devel@edk2.groups.io
> >Cc: Gao, Liming <liming.gao@intel.com>; Feng, Bob C
> ><bob.c.feng@intel.com>; Rodriguez, Christian
> ><christian.rodriguez@intel.com>; Fan, ZhijuX <zhijux.fan@intel.com>
> >Subject: [edk2-devel] [PATCH] BaseTools:Extend the binary cache to support
> >library cache
> >
> >https://bugzilla.tianocore.org/show_bug.cgi?id=1797
> >
> >Current binary cache doesn't support to save and restore
> >the library module. If a driver module cache miss happen,
> >all its dependency library modules need rebuild which
> >is very time-consuming. This patch is to entend the binary
> >cache to support library.
> >
> >Cc: Liming Gao <liming.gao@intel.com>
> >Cc: Bob Feng <bob.c.feng@intel.com>
> >Cc: Christian Rodriguez <christian.rodriguez@intel.com>
> >Signed-off-by: Steven Shi <steven.shi@intel.com>
> >---
> > BaseTools/Source/Python/AutoGen/AutoGen.py | 17 ++++++++++++++---
> > BaseTools/Source/Python/build/build.py     |  3 ++-
> > 2 files changed, 16 insertions(+), 4 deletions(-)
> >
> >diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py
> >b/BaseTools/Source/Python/AutoGen/AutoGen.py
> >index a5bef4f7c6..aeb63f52c5 100644
> >--- a/BaseTools/Source/Python/AutoGen/AutoGen.py
> >+++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
> >@@ -2578,7 +2578,7 @@ class ModuleAutoGen(AutoGen):
> >         self.AutoGenDepSet = set()
> >         self.ReferenceModules = []
> >         self.ConstPcd                  = {}
> >-
> >+        self.CacheRestored             = False
> >
> >     def __repr__(self):
> >         return "%s [%s]" % (self.MetaFile, self.Arch)
> >@@ -3906,6 +3906,12 @@ class ModuleAutoGen(AutoGen):
> >             ModuleFile = path.join(self.OutputDir, self.Name + '.inf')
> >             if os.path.exists(ModuleFile):
> >                 shutil.copy2(ModuleFile, FileDir)
> >+        else:
> >+            OutputDir = self.OutputDir.replace('\\', '/').strip('/')
> >+            DebugDir = self.DebugDir.replace('\\', '/').strip('/')
> >+            for Item in self.CodaTargetList:
> >+                File = Item.Target.Path.replace('\\', '/').strip('/').replace(DebugDir,
> >'').replace(OutputDir, '').strip('/')
> >+                self.OutputFile.add(File)
> >         if not self.OutputFile:
> >             Ma = self.BuildDatabase[self.MetaFile, self.Arch, self.BuildTarget,
> >self.ToolChain]
> >             self.OutputFile = Ma.Binaries
> >@@ -3949,6 +3955,7 @@ class ModuleAutoGen(AutoGen):
> >                                 destination_dir = os.path.dirname(destination_file)
> >                                 CreateDirectory(destination_dir)
> >                                 shutil.copy2(File, destination_dir)
> >+                    self.CacheRestored = True
> >                     if self.Name == "PcdPeim" or self.Name == "PcdDxe":
> >                         CreatePcdDatabaseCode(self, TemplateString(),
> >TemplateString())
> >                     return True
> >@@ -3987,7 +3994,9 @@ class ModuleAutoGen(AutoGen):
> >         self.GenFfsList = GenFfsList
> >         if not self.IsLibrary and CreateLibraryMakeFile:
> >             for LibraryAutoGen in self.LibraryAutoGenList:
> >-                LibraryAutoGen.CreateMakeFile()
> >+                # Only create makefile for libraries which have not been restored
> >+                if not LibraryAutoGen.CacheRestored:
> >+                    LibraryAutoGen.CreateMakeFile()
> >
> >         if self.CanSkip():
> >             return
> >@@ -4030,7 +4039,9 @@ class ModuleAutoGen(AutoGen):
> >
> >         if not self.IsLibrary and CreateLibraryCodeFile:
> >             for LibraryAutoGen in self.LibraryAutoGenList:
> >-                LibraryAutoGen.CreateCodeFile()
> >+                # Only create autogen code for libraries which have not been
> >restored
> >+                if not LibraryAutoGen.CacheRestored:
> >+                    LibraryAutoGen.CreateCodeFile()
> >
> >         if self.CanSkip():
> >             return
> >diff --git a/BaseTools/Source/Python/build/build.py
> >b/BaseTools/Source/Python/build/build.py
> >index 80ceb98310..f1f4c07980 100644
> >--- a/BaseTools/Source/Python/build/build.py
> >+++ b/BaseTools/Source/Python/build/build.py
> >@@ -341,7 +341,8 @@ class ModuleMakeUnit(BuildUnit):
> >     #   @param  Target      The build target name, one of gSupportedTarget
> >     #
> >     def __init__(self, Obj, Target):
> >-        Dependency = [ModuleMakeUnit(La, Target) for La in
> >Obj.LibraryAutoGenList]
> >+        # Skip the dependency modules which are already restored from cache
> >+        Dependency = [ModuleMakeUnit(La, Target) for La in
> >Obj.LibraryAutoGenList if not La.CacheRestored]
> >         BuildUnit.__init__(self, Obj, Obj.BuildCommand, Target, Dependency,
> >Obj.MakeFileDir)
> >         if Target in [None, "", "all"]:
> >             self.Target = "tbuild"
> >--
> >2.17.1.windows.2
> >
> >
> >


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2019-05-29  0:59 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-05-28  8:51 [PATCH] BaseTools:Extend the binary cache to support library cache Steven Shi
2019-05-28 15:15 ` [edk2-devel] " Christian Rodriguez
2019-05-29  0:59   ` Steven Shi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox