Re: [PATCH 7/8] command.py: add new command to get the CPU info

[Richard Purdie <richard.purdie@linuxfoundation.org>]

On Wed, 2012-01-11 at 11:52 +0000, Wang, Shane wrote:
> Richard Purdie wrote on 2012-01-11:
> 
> > On Wed, 2012-01-11 at 11:03 +0800, Dongxiao Xu wrote:
> >> Add new API in command.py to get the CPU core and threads information
> >> in order to set the appropriate BB_NUMBER_THREADS and PARALLEL_MAKE
> >> variables.
> >> 
> >> Signed-off-by: Shane Wang <shane.wang@intel.com>
> >> ---
> >>  bitbake/lib/bb/helper.py |   39
> >>  +++++++++++++++++++++++++++++++++++++++ lib/bb/command.py        |  
> >>  12 ++++++++++++ 2 files changed, 51 insertions(+), 0 deletions(-)
> >>  create mode 100644 bitbake/lib/bb/helper.py
> >> diff --git a/bitbake/lib/bb/helper.py b/bitbake/lib/bb/helper.py new
> >> file mode 100644 index 0000000..291158b --- /dev/null +++
> >> b/bitbake/lib/bb/helper.py @@ -0,0 +1,39 @@ +# +# Helper for BitBake
> >> Graphical GTK User Interface +# +# Copyright (C) 2011        Intel
> >> Corporation +# +# Authored by Shane Wang <shane.wang@intel.com> +# +#
> >> This program is free software; you can redistribute it and/or modify +#
> >> it under the terms of the GNU General Public License version 2 as +#
> >> published by the Free Software Foundation. +# +# This program is
> >> distributed in the hope that it will be useful, +# but WITHOUT ANY
> >> WARRANTY; without even the implied warranty of +# MERCHANTABILITY or
> >> FITNESS FOR A PARTICULAR PURPOSE.  See the +# GNU General Public
> >> License for more details. +# +# You should have received a copy of the
> >> GNU General Public License along +# with this program; if not, write to
> >> the Free Software Foundation, Inc., +# 51 Franklin Street, Fifth Floor,
> >> Boston, MA 02110-1301 USA. + +import os + +class CpuInfo(object): + +  
> >>  coefficient = 4
> > 
> > This is setting off alarm bells...
> > 
> >> +    @classmethod
> >> +    def getNumOfCpus(cls):
> >> +        pfile = os.popen('cat /proc/cpuinfo | grep cpu\ cores')
> >> +        num = len(pfile.readlines())
> >> +        return num
> > 
> > You can get this number with something like:
> > 
> > import multiprocessing
> > multiprocessing.cpu_count()
> Good to see this works, will change that.
> 
> > 
> >> +    @classmethod +    def getNumOfCpuCores(cls): +        pfile =
> >> os.popen('cat /proc/cpuinfo | grep cpu\ cores | cut -d: -f2') +       
> >> contents = pfile.readlines() +        num = int(contents[0]) +       
> >> return num
> > 
> > I'm curious what you're using the number of cores to do? It doesn't seem
> > used by your code?
> > 
> > 
> >> diff --git a/lib/bb/command.py b/lib/bb/command.py
> >> index 05555c5..eaf8236 100644
> >> --- a/lib/bb/command.py
> >> +++ b/lib/bb/command.py
> >> @@ -30,6 +30,7 @@ Commands are queued in a CommandQueue
> >> 
> >>  import bb.event
> >>  import bb.cooker
> >> +import bb.helper
> >> 
> >>  class CommandCompleted(bb.event.Event):
> >>      pass
> >> @@ -173,6 +174,17 @@ class CommandsSync:
> >>          """
> >>          command.cooker.reset()
> >> +    def getDefaultNumOfThreads(self, command, params): +        """ + 
> >>       Get the default number of threads on the server = number of CPUs
> >> +        """ +        return bb.helper.CpuInfo.getNumOfCpus() + +   
> >> def getMaxNumOfThreads(self, command, params): +        """ +       
> >> Get the max number of threads that the server can tolerate +        """
> >> +        return bb.helper.CpuInfo.getNumOfCpus() *
> >> bb.helper.CpuInfo.coefficient
> >> 
> > 
> > I can understand needing to query the number of cpus but this last
> > function seems rather arbitrary. If you want to do bounds checking, I'd
> > suggest just adding the factor of 4 into the UI.
> OK
> 
> > I'm still not convinced
> > we should be setting any value for this though, or doing any bounds
> > checking on the value.
> Which value? We hope when the UI starts up, the UI can take advantage of cpus for build and set it as default.
> But users can change between 0 ~ max num of threads.

You can either:

a) Make it a text input (and then attempt to convert to an int, error if
its not an int). This means you don't need a maximum value.
b) In the UI, just show options up to 4 * cpu count.

I'd probably prefer a) but I can see why b) is easier. I don't see any
value in encoding "4" in the bitbake server though.

Cheers,

Richard