Re: [edk2-devel] [Patch 10/10 V3] BaseTools: Enable block queue log agent.

["Bob Feng" <bob.c.feng@intel.com>]

Good suggestion. I think ThreadNum * 10 is better than a const. I'll update it in patches V4.

I'd like to collect more comments on other parts and update all the comments in V4.

Thanks,
Bob

-----Original Message-----
From: Laszlo Ersek [mailto:lersek@redhat.com] 
Sent: Tuesday, July 23, 2019 6:12 PM
To: devel@edk2.groups.io; Feng, Bob C <bob.c.feng@intel.com>
Cc: Gao, Liming <liming.gao@intel.com>
Subject: Re: [edk2-devel] [Patch 10/10 V3] BaseTools: Enable block queue log agent.

On 07/23/19 05:58, Bob Feng wrote:
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1875
> 
> To support Ctrl+S and Ctrl+Q, we enable block queue for log.
> 
> Cc: Liming Gao <liming.gao@intel.com>
> Signed-off-by: Bob Feng <bob.c.feng@intel.com>
> ---
>  BaseTools/Source/Python/Common/EdkLogger.py | 10 ++++++----
>  BaseTools/Source/Python/build/build.py      |  8 +++++---
>  2 files changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/BaseTools/Source/Python/Common/EdkLogger.py 
> b/BaseTools/Source/Python/Common/EdkLogger.py
> index 15fd1458a95a..06da4a9d0a1d 100644
> --- a/BaseTools/Source/Python/Common/EdkLogger.py
> +++ b/BaseTools/Source/Python/Common/EdkLogger.py
> @@ -93,11 +93,13 @@ except:
>              """
>              try:
>                  self.enqueue(self.prepare(record))
>              except Exception:
>                  self.handleError(record)
> -
> +class BlockQueueHandler(QueueHandler):
> +    def enqueue(self, record):
> +        self.queue.put(record,True)
>  ## Log level constants
>  DEBUG_0 = 1
>  DEBUG_1 = 2
>  DEBUG_2 = 3
>  DEBUG_3 = 4
> @@ -290,23 +292,23 @@ def LogClientInitialize(log_q):
>      # Since we use different format to log different levels of message into different
>      # place (stdout or stderr), we have to use different "Logger" objects to do this.
>      #
>      # For DEBUG level (All DEBUG_0~9 are applicable)
>      _DebugLogger.setLevel(INFO)
> -    _DebugChannel = QueueHandler(log_q)
> +    _DebugChannel = BlockQueueHandler(log_q)
>      _DebugChannel.setFormatter(_DebugFormatter)
>      _DebugLogger.addHandler(_DebugChannel)
>  
>      # For VERBOSE, INFO, WARN level
>      _InfoLogger.setLevel(INFO)
> -    _InfoChannel = QueueHandler(log_q)
> +    _InfoChannel = BlockQueueHandler(log_q)
>      _InfoChannel.setFormatter(_InfoFormatter)
>      _InfoLogger.addHandler(_InfoChannel)
>  
>      # For ERROR level
>      _ErrorLogger.setLevel(INFO)
> -    _ErrorCh = QueueHandler(log_q)
> +    _ErrorCh = BlockQueueHandler(log_q)
>      _ErrorCh.setFormatter(_ErrorFormatter)
>      _ErrorLogger.addHandler(_ErrorCh)
>  
>  ## Set log level
>  #
> diff --git a/BaseTools/Source/Python/build/build.py 
> b/BaseTools/Source/Python/build/build.py
> index 4125b2832946..603d3aa6dad4 100644
> --- a/BaseTools/Source/Python/build/build.py
> +++ b/BaseTools/Source/Python/build/build.py
> @@ -2046,14 +2046,15 @@ class Build():
>                      for m in Pa.GetAllModuleInfo:
>                          mqueue.put(m)
>                      data_pipe_file = os.path.join(Pa.BuildDir, "GlobalVar_%s_%s.bin" % (str(Pa.Guid),Pa.Arch))
>                      Pa.DataPipe.dump(data_pipe_file)
>                      autogen_rt = self.StartAutoGen(mqueue, Pa.DataPipe, self.SkipAutoGen, PcdMaList,self.share_data)
> -                    self.Progress.Stop("done!")
> -                    self.AutoGenTime += int(round((time.time() - AutoGenStart)))
> +
>                      if not autogen_rt:
>                          return
> +                self.AutoGenTime += int(round((time.time() - AutoGenStart)))
> +                self.Progress.Stop("done!")
>                  for Arch in Wa.ArchList:
>                      MakeStart = time.time()
>                      for Ma in BuildModules:
>                          # Generate build task for the module
>                          if not Ma.IsBinaryModule:
> @@ -2294,17 +2295,18 @@ def LogBuildTime(Time):
>  # if it's executed successfully or not.
>  #
>  #   @retval 0     Tool was successful
>  #   @retval 1     Tool failed
>  #
> +LogQMaxSize = 60

Can we use a value here that is proportional to multiprocessing.cpu_count()?

Or, more precisely, proportional to the thread count set with "-n"?
Like, multiply the thread count by 10.

Because, in case the thread count is really high, a constant log queue size could throttle compilation, even if terminal output is not suspended.

The factor 10 is still quite arbitrary, but I believe it should block compilation "soon enough" after terminal output is suspended, and should not throttle compilation in practice when terminal output is not suspended.

Just an idea.

Thanks
Laszlo

>  def Main():
>      StartTime = time.time()
>  
>      #
>      # Create a log Queue
>      #
> -    LogQ = mp.Queue()
> +    LogQ = mp.Queue(LogQMaxSize)
>      # Initialize log system
>      EdkLogger.LogClientInitialize(LogQ)
>      GlobalData.gCommand = sys.argv[1:]
>      #
>      # Parse the options and args
>