Patchwork Modify buildstats to be merged inside buildhistory

login
register
mail settings
Submitter Corneliu Stoicescu
Date Nov. 1, 2013, 6:03 p.m.
Message ID <1383329028-24348-1-git-send-email-corneliux.stoicescu@intel.com>
Download mbox | patch
Permalink /patch/61001/
State New
Headers show

Comments

Corneliu Stoicescu - Nov. 1, 2013, 6:03 p.m.
- added buildstats inheritance inside buildhistory
- reworked the buildstats directory structure not to contain the build name(read: the build start date).
Now it only contains the target name and machine. This is usefull in order to reduce git noise.
- because builds are no longer separated by build name(read: date) it is necessary to remake(remove and create from scratch)
the buildstats folder for each build in order keep buildstats compatible with tools like pybootchartgui.py

Some changes to make the new functionality compatible with Yocto:
- remove buildstats from default usage because it now needs buildhistory (remove it from USER_CLASSES in local.conf)
- add 'buildhistory' to USER_CLASSES or add INHERIT += "buildhistory" in local.conf
OPTIONAL: - I tested this patch with buildhistory under git enabled (BUILDHISTORY_COMMIT = "1" in local.conf). I believe this
should be made default.

I made some tests with the buildhistory-diff tool and it is compatible with the changes. We can add further functionality
to it in order to make it interpret buildstats data.

Signed-off-by: Corneliu Stoicescu <corneliux.stoicescu@intel.com>
---
 meta/classes/buildhistory.bbclass |    1 +
 meta/classes/buildstats.bbclass   |    8 ++++++--
 2 files changed, 7 insertions(+), 2 deletions(-)
Richard Purdie - Nov. 1, 2013, 6:07 p.m.
On Fri, 2013-11-01 at 19:03 +0100, Corneliu Stoicescu wrote:
> - added buildstats inheritance inside buildhistory
> - reworked the buildstats directory structure not to contain the build name(read: the build start date).
> Now it only contains the target name and machine. This is usefull in order to reduce git noise.
> - because builds are no longer separated by build name(read: date) it is necessary to remake(remove and create from scratch)
> the buildstats folder for each build in order keep buildstats compatible with tools like pybootchartgui.py
> 
> Some changes to make the new functionality compatible with Yocto:
> - remove buildstats from default usage because it now needs buildhistory (remove it from USER_CLASSES in local.conf)
> - add 'buildhistory' to USER_CLASSES or add INHERIT += "buildhistory" in local.conf
> OPTIONAL: - I tested this patch with buildhistory under git enabled (BUILDHISTORY_COMMIT = "1" in local.conf). I believe this
> should be made default.
> 
> I made some tests with the buildhistory-diff tool and it is compatible with the changes. We can add further functionality
> to it in order to make it interpret buildstats data.
> 
> Signed-off-by: Corneliu Stoicescu <corneliux.stoicescu@intel.com>
> ---
>  meta/classes/buildhistory.bbclass |    1 +
>  meta/classes/buildstats.bbclass   |    8 ++++++--
>  2 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/meta/classes/buildhistory.bbclass b/meta/classes/buildhistory.bbclass
> index 3da03c8..a78bd4b 100644
> --- a/meta/classes/buildhistory.bbclass
> +++ b/meta/classes/buildhistory.bbclass
> @@ -19,6 +19,7 @@ BUILDHISTORY_PUSH_REPO ?= ""
>  
>  # Must inherit package first before changing PACKAGEFUNCS
>  inherit package
> +inherit buildstats
>  PACKAGEFUNCS += "buildhistory_emit_pkghistory"

We could do a 

BUILDSTATS_BASE = "${BUILDHISTORY_DIR}/buildstats"

in buildhistory.bbclass

>  # We don't want to force a rerun of do_package for everything
> diff --git a/meta/classes/buildstats.bbclass b/meta/classes/buildstats.bbclass
> index 72fff11..476ae94 100644
> --- a/meta/classes/buildstats.bbclass
> +++ b/meta/classes/buildstats.bbclass
> @@ -1,4 +1,4 @@
> -BUILDSTATS_BASE = "${TMPDIR}/buildstats/"

and here do BUILDSTATS_BASE ??= "${TMPDIR}/buildstats/"

> +BUILDSTATS_BASE = "${BUILDHISTORY_DIR}/buildstats"
>  BNFILE = "${BUILDSTATS_BASE}/.buildname"
>  DEVFILE = "${BUILDSTATS_BASE}/.device"

so when buildhistory is used, buildstats automatically becomes part of
it?

> @@ -33,7 +33,7 @@ def set_bn(e):
>  
>  def get_bn(e):
>      with open(e.data.getVar('BNFILE', True)) as f:
> -        bn = f.readline()
> +        bn = str(f.readline()).split("/")[0]
>      return bn
>  
>  def set_device(e):
> @@ -175,6 +175,10 @@ python run_buildstats () {
>          # set the buildname
>          ########################################################################
>          try:
> +            bb.utils.remove(e.data.getVar('BUILDSTATS_BASE', True), recurse=True)


Do we need to remove this?

I'll let Paul comment on other elements of this.

Cheers,

Richard

> +        except:
> +            pass
> +        try:
>              bb.utils.mkdirhier(e.data.getVar('BUILDSTATS_BASE', True))
>          except:
>              pass
Corneliu Stoicescu - Nov. 3, 2013, 11:37 a.m.
> -----Original Message-----
> From: Richard Purdie [mailto:richard.purdie@linuxfoundation.org]
> Sent: Friday, November 01, 2013 8:07 PM
> To: Stoicescu, CorneliuX
> Cc: openembedded-core@lists.openembedded.org
> Subject: Re: [OE-core] [PATCH] Modify buildstats to be merged inside
> buildhistory
> 
> On Fri, 2013-11-01 at 19:03 +0100, Corneliu Stoicescu wrote:
> > - added buildstats inheritance inside buildhistory
> > - reworked the buildstats directory structure not to contain the build
> name(read: the build start date).
> > Now it only contains the target name and machine. This is usefull in order
> to reduce git noise.
> > - because builds are no longer separated by build name(read: date) it
> > is necessary to remake(remove and create from scratch) the buildstats
> > folder for each build in order keep buildstats compatible with tools
> > like pybootchartgui.py
> >
> > Some changes to make the new functionality compatible with Yocto:
> > - remove buildstats from default usage because it now needs
> > buildhistory (remove it from USER_CLASSES in local.conf)
> > - add 'buildhistory' to USER_CLASSES or add INHERIT += "buildhistory"
> > in local.conf
> > OPTIONAL: - I tested this patch with buildhistory under git enabled
> > (BUILDHISTORY_COMMIT = "1" in local.conf). I believe this should be made
> default.
> >
> > I made some tests with the buildhistory-diff tool and it is compatible
> > with the changes. We can add further functionality to it in order to make it
> interpret buildstats data.
> >
> > Signed-off-by: Corneliu Stoicescu <corneliux.stoicescu@intel.com>
> > ---
> >  meta/classes/buildhistory.bbclass |    1 +
> >  meta/classes/buildstats.bbclass   |    8 ++++++--
> >  2 files changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/meta/classes/buildhistory.bbclass
> > b/meta/classes/buildhistory.bbclass
> > index 3da03c8..a78bd4b 100644
> > --- a/meta/classes/buildhistory.bbclass
> > +++ b/meta/classes/buildhistory.bbclass
> > @@ -19,6 +19,7 @@ BUILDHISTORY_PUSH_REPO ?= ""
> >
> >  # Must inherit package first before changing PACKAGEFUNCS  inherit
> > package
> > +inherit buildstats
> >  PACKAGEFUNCS += "buildhistory_emit_pkghistory"
> 
> We could do a
> 
> BUILDSTATS_BASE = "${BUILDHISTORY_DIR}/buildstats"
> 
> in buildhistory.bbclass
> 
> >  # We don't want to force a rerun of do_package for everything diff
> > --git a/meta/classes/buildstats.bbclass
> > b/meta/classes/buildstats.bbclass index 72fff11..476ae94 100644
> > --- a/meta/classes/buildstats.bbclass
> > +++ b/meta/classes/buildstats.bbclass
> > @@ -1,4 +1,4 @@
> > -BUILDSTATS_BASE = "${TMPDIR}/buildstats/"
> 
> and here do BUILDSTATS_BASE ??= "${TMPDIR}/buildstats/"
> 

Yes, this would help buildstats work without buildhistory. Thanks :)

> > +BUILDSTATS_BASE = "${BUILDHISTORY_DIR}/buildstats"
> >  BNFILE = "${BUILDSTATS_BASE}/.buildname"
> >  DEVFILE = "${BUILDSTATS_BASE}/.device"
> 
> so when buildhistory is used, buildstats automatically becomes part of it?
> 
> > @@ -33,7 +33,7 @@ def set_bn(e):
> >
> >  def get_bn(e):
> >      with open(e.data.getVar('BNFILE', True)) as f:
> > -        bn = f.readline()
> > +        bn = str(f.readline()).split("/")[0]
> >      return bn
> >
> >  def set_device(e):
> > @@ -175,6 +175,10 @@ python run_buildstats () {
> >          # set the buildname
> >
> ##############################################################
> ##########
> >          try:
> > +            bb.utils.remove(e.data.getVar('BUILDSTATS_BASE', True),
> > + recurse=True)
> 
> 
> Do we need to remove this?
> 

By reworking the buildstats directory structure to not use the build date to separate builds, if we don't remove buildstats each time we make a new build the information from multiple builds will merge and tools like bybootchartguy.py will not work anymore with buildstats. 

Do you have any idea on how else we could keep this compatibility?  

> I'll let Paul comment on other elements of this.
> 
> Cheers,
> 
> Richard
> 
> > +        except:
> > +            pass
> > +        try:
> >              bb.utils.mkdirhier(e.data.getVar('BUILDSTATS_BASE', True))
> >          except:
> >              pass
> 

Regards,
Corneliu
Richard Purdie - Nov. 4, 2013, 9:16 a.m.
On Sun, 2013-11-03 at 11:37 +0000, Stoicescu, CorneliuX wrote:
> > > +BUILDSTATS_BASE = "${BUILDHISTORY_DIR}/buildstats"
> > >  BNFILE = "${BUILDSTATS_BASE}/.buildname"
> > >  DEVFILE = "${BUILDSTATS_BASE}/.device"
> > 
> > so when buildhistory is used, buildstats automatically becomes part of it?
> > 
> > > @@ -33,7 +33,7 @@ def set_bn(e):
> > >
> > >  def get_bn(e):
> > >      with open(e.data.getVar('BNFILE', True)) as f:
> > > -        bn = f.readline()
> > > +        bn = str(f.readline()).split("/")[0]
> > >      return bn
> > >
> > >  def set_device(e):
> > > @@ -175,6 +175,10 @@ python run_buildstats () {
> > >          # set the buildname
> > >
> > ##############################################################
> > ##########
> > >          try:
> > > +            bb.utils.remove(e.data.getVar('BUILDSTATS_BASE', True),
> > > + recurse=True)
> > 
> > 
> > Do we need to remove this?
> > 
> 
> By reworking the buildstats directory structure to not use the build
> date to separate builds, if we don't remove buildstats each time we
> make a new build the information from multiple builds will merge and
> tools like bybootchartguy.py will not work anymore with buildstats. 

I'd have expected that separate directories for each build are still
maintained. Testing locally here, I can use pybootchartgui in the form:

scripts/pybootchartgui/pybootchartgui.py build/tmp/buildstats/core-image-sato-qemux86/201309302155/

and I'd have thought a similar command would still work in buildhistory
since we'd still have the timestamped directory there?

Cheers,

Richard
Corneliu Stoicescu - Nov. 4, 2013, 9:24 a.m.
> -----Original Message-----
> From: Richard Purdie [mailto:richard.purdie@linuxfoundation.org]
> Sent: Monday, November 04, 2013 11:16 AM
> To: Stoicescu, CorneliuX
> Cc: openembedded-core@lists.openembedded.org
> Subject: Re: [OE-core] [PATCH] Modify buildstats to be merged inside
> buildhistory
> 
> On Sun, 2013-11-03 at 11:37 +0000, Stoicescu, CorneliuX wrote:
> > > > +BUILDSTATS_BASE = "${BUILDHISTORY_DIR}/buildstats"
> > > >  BNFILE = "${BUILDSTATS_BASE}/.buildname"
> > > >  DEVFILE = "${BUILDSTATS_BASE}/.device"
> > >
> > > so when buildhistory is used, buildstats automatically becomes part of it?
> > >
> > > > @@ -33,7 +33,7 @@ def set_bn(e):
> > > >
> > > >  def get_bn(e):
> > > >      with open(e.data.getVar('BNFILE', True)) as f:
> > > > -        bn = f.readline()
> > > > +        bn = str(f.readline()).split("/")[0]
> > > >      return bn
> > > >
> > > >  def set_device(e):
> > > > @@ -175,6 +175,10 @@ python run_buildstats () {
> > > >          # set the buildname
> > > >
> > >
> ##############################################################
> > > ##########
> > > >          try:
> > > > +            bb.utils.remove(e.data.getVar('BUILDSTATS_BASE',
> > > > + True),
> > > > + recurse=True)
> > >
> > >
> > > Do we need to remove this?
> > >
> >
> > By reworking the buildstats directory structure to not use the build
> > date to separate builds, if we don't remove buildstats each time we
> > make a new build the information from multiple builds will merge and
> > tools like bybootchartguy.py will not work anymore with buildstats.
> 
> I'd have expected that separate directories for each build are still
> maintained. Testing locally here, I can use pybootchartgui in the form:
> 
> scripts/pybootchartgui/pybootchartgui.py build/tmp/buildstats/core-image-
> sato-qemux86/201309302155/
> 
> and I'd have thought a similar command would still work in buildhistory since
> we'd still have the timestamped directory there?
> 

The timestamped directory was removed in order to reduce buildhistory git noise. 
Creating a new directory for each build will also increase the git repo size dramatically
In time.

If we choose to not use the timestamped directory, we need to delete the buildstats 
data each build because, in contrast to buildhistory, buildstats data is relevant when
compiled as a whole. This is also how pybootchart interprets it.

> Cheers,
> 
> Richard

Regards,
Corneliu
Richard Purdie - Nov. 4, 2013, 9:31 a.m.
On Mon, 2013-11-04 at 09:24 +0000, Stoicescu, CorneliuX wrote:
> > -----Original Message-----
> > From: Richard Purdie [mailto:richard.purdie@linuxfoundation.org]
> > Sent: Monday, November 04, 2013 11:16 AM
> > To: Stoicescu, CorneliuX
> > Cc: openembedded-core@lists.openembedded.org
> > Subject: Re: [OE-core] [PATCH] Modify buildstats to be merged inside
> > buildhistory
> > 
> > On Sun, 2013-11-03 at 11:37 +0000, Stoicescu, CorneliuX wrote:
> > > > > +BUILDSTATS_BASE = "${BUILDHISTORY_DIR}/buildstats"
> > > > >  BNFILE = "${BUILDSTATS_BASE}/.buildname"
> > > > >  DEVFILE = "${BUILDSTATS_BASE}/.device"
> > > >
> > > > so when buildhistory is used, buildstats automatically becomes part of it?
> > > >
> > > > > @@ -33,7 +33,7 @@ def set_bn(e):
> > > > >
> > > > >  def get_bn(e):
> > > > >      with open(e.data.getVar('BNFILE', True)) as f:
> > > > > -        bn = f.readline()
> > > > > +        bn = str(f.readline()).split("/")[0]
> > > > >      return bn
> > > > >
> > > > >  def set_device(e):
> > > > > @@ -175,6 +175,10 @@ python run_buildstats () {
> > > > >          # set the buildname
> > > > >
> > > >
> > ##############################################################
> > > > ##########
> > > > >          try:
> > > > > +            bb.utils.remove(e.data.getVar('BUILDSTATS_BASE',
> > > > > + True),
> > > > > + recurse=True)
> > > >
> > > >
> > > > Do we need to remove this?
> > > >
> > >
> > > By reworking the buildstats directory structure to not use the build
> > > date to separate builds, if we don't remove buildstats each time we
> > > make a new build the information from multiple builds will merge and
> > > tools like bybootchartguy.py will not work anymore with buildstats.
> > 
> > I'd have expected that separate directories for each build are still
> > maintained. Testing locally here, I can use pybootchartgui in the form:
> > 
> > scripts/pybootchartgui/pybootchartgui.py build/tmp/buildstats/core-image-
> > sato-qemux86/201309302155/
> > 
> > and I'd have thought a similar command would still work in buildhistory since
> > we'd still have the timestamped directory there?
> > 
> 
> The timestamped directory was removed in order to reduce buildhistory git noise. 
> Creating a new directory for each build will also increase the git repo size dramatically
> In time.
> 
> If we choose to not use the timestamped directory, we need to delete the buildstats 
> data each build because, in contrast to buildhistory, buildstats data is relevant when
> compiled as a whole. This is also how pybootchart interprets it.

I hadn't realised you'd removed the timestamps. If you do that there is
a problem, yes. Paul, how do you think we should handle this? I'd have
thought it would be ok to save the historical data in there side by side
since the size of the git repo will be the same, the size of the
checkout will just be different. We do need to save something into the
timestamped directory saying which revision(s) a given build happened
against though so we have some kind of historical context.

Paul: Any thoughts on this?

Cheers,

Richard
Paul Eggleton - Nov. 4, 2013, 12:14 p.m.
On Monday 04 November 2013 09:31:19 Richard Purdie wrote:
> On Mon, 2013-11-04 at 09:24 +0000, Stoicescu, CorneliuX wrote:
> > > -----Original Message-----
> > > From: Richard Purdie [mailto:richard.purdie@linuxfoundation.org]
> > > Sent: Monday, November 04, 2013 11:16 AM
> > > To: Stoicescu, CorneliuX
> > > Cc: openembedded-core@lists.openembedded.org
> > > Subject: Re: [OE-core] [PATCH] Modify buildstats to be merged inside
> > > buildhistory
> > > 
> > > On Sun, 2013-11-03 at 11:37 +0000, Stoicescu, CorneliuX wrote:
> > > > > > +BUILDSTATS_BASE = "${BUILDHISTORY_DIR}/buildstats"
> > > > > > 
> > > > > >  BNFILE = "${BUILDSTATS_BASE}/.buildname"
> > > > > >  DEVFILE = "${BUILDSTATS_BASE}/.device"
> > > > > 
> > > > > so when buildhistory is used, buildstats automatically becomes part
> > > > > of it?
> > > > > 
> > > > > > @@ -33,7 +33,7 @@ def set_bn(e):
> > > > > >  def get_bn(e):
> > > > > >      with open(e.data.getVar('BNFILE', True)) as f:
> > > > > > -        bn = f.readline()
> > > > > > +        bn = str(f.readline()).split("/")[0]
> > > > > > 
> > > > > >      return bn
> > > > > >  
> > > > > >  def set_device(e):
> > > > > > @@ -175,6 +175,10 @@ python run_buildstats () {
> > > > > > 
> > > > > >          # set the buildname
> > > 
> > > ##############################################################
> > > 
> > > > > ##########
> > > > > 
> > > > > >          try:
> > > > > > +            bb.utils.remove(e.data.getVar('BUILDSTATS_BASE',
> > > > > > + True),
> > > > > > + recurse=True)
> > > > > 
> > > > > Do we need to remove this?
> > > > 
> > > > By reworking the buildstats directory structure to not use the build
> > > > date to separate builds, if we don't remove buildstats each time we
> > > > make a new build the information from multiple builds will merge and
> > > > tools like bybootchartguy.py will not work anymore with buildstats.
> > > 
> > > I'd have expected that separate directories for each build are still
> > > maintained. Testing locally here, I can use pybootchartgui in the form:
> > > 
> > > scripts/pybootchartgui/pybootchartgui.py
> > > build/tmp/buildstats/core-image-
> > > sato-qemux86/201309302155/
> > > 
> > > and I'd have thought a similar command would still work in buildhistory
> > > since we'd still have the timestamped directory there?
> > 
> > The timestamped directory was removed in order to reduce buildhistory git
> > noise. Creating a new directory for each build will also increase the git
> > repo size dramatically In time.
> > 
> > If we choose to not use the timestamped directory, we need to delete the
> > buildstats data each build because, in contrast to buildhistory,
> > buildstats data is relevant when compiled as a whole. This is also how
> > pybootchart interprets it.
> 
> I hadn't realised you'd removed the timestamps. If you do that there is
> a problem, yes. Paul, how do you think we should handle this? I'd have
> thought it would be ok to save the historical data in there side by side
> since the size of the git repo will be the same, the size of the
> checkout will just be different. We do need to save something into the
> timestamped directory saying which revision(s) a given build happened
> against though so we have some kind of historical context.
> 
> Paul: Any thoughts on this?

The basic idea with buildhistory is to only keep the latest version of data 
stored within it, on the assumption that you could always get previous 
versions from the git history; this also makes comparisons to previous 
versions straightforward.

As far as timestamps go, BUILDNAME does go into the commit message; if it 
helps we record the metadata revisions corresponding to the build in the 
buildhistory repo as well (in a "metadata-revs" file).

If we do want to keep the builds split out in separate per-build directories 
then what are we gaining by trying to push buildstats into buildhistory in the 
first place?

Cheers,
Paul
Richard Purdie - Nov. 4, 2013, 12:34 p.m.
On Mon, 2013-11-04 at 12:14 +0000, Paul Eggleton wrote:
> On Monday 04 November 2013 09:31:19 Richard Purdie wrote:
> The basic idea with buildhistory is to only keep the latest version of data 
> stored within it, on the assumption that you could always get previous 
> versions from the git history; this also makes comparisons to previous 
> versions straightforward.

This makes sense for absolute value data. The task build statistics are
not entirely "absolute" data though and having to traverse many
checkouts trying to build up say "average task timing" might be
problematic.

> As far as timestamps go, BUILDNAME does go into the commit message; if it 
> helps we record the metadata revisions corresponding to the build in the 
> buildhistory repo as well (in a "metadata-revs" file).

Currently the main use of the data is to say "visualise this build"
where you're building a target or set of targets. We name the build
after the first item in the list of build targets.

The logged data is useful both as part of a set which are viewed
together and individually on a task basis so see how much CPU/Disk a
given task uses and how that changes over time.

The tricky part is firstly that we should really use a true buildname,
secondly that we need both the set of data and data per target.

If we just logged data per target, we could overwrite any previous data
but that would break usage as a set.

> If we do want to keep the builds split out in separate per-build directories 
> then what are we gaining by trying to push buildstats into buildhistory in the 
> first place?

We have a need to be able to merge and store build statistic data which
build history already does well. The question is whether it makes sense
to use the same mechanism or not since there are commonalities and
differences. Having two data storage systems and repositories doesn't
make a lot of sense.

I'm not sure this makes a solution much clearer but it does hopefully
better explain the challenge.

Cheers,

Richard
Corneliu Stoicescu - Nov. 4, 2013, 12:42 p.m.
> -----Original Message-----
> From: Richard Purdie [mailto:richard.purdie@linuxfoundation.org]
> Sent: Monday, November 04, 2013 2:35 PM
> To: Paul Eggleton
> Cc: Stoicescu, CorneliuX; openembedded-core@lists.openembedded.org
> Subject: Re: [OE-core] [PATCH] Modify buildstats to be merged inside
> buildhistory
> 
> On Mon, 2013-11-04 at 12:14 +0000, Paul Eggleton wrote:
> > On Monday 04 November 2013 09:31:19 Richard Purdie wrote:
> > The basic idea with buildhistory is to only keep the latest version of
> > data stored within it, on the assumption that you could always get
> > previous versions from the git history; this also makes comparisons to
> > previous versions straightforward.
> 
> This makes sense for absolute value data. The task build statistics are not
> entirely "absolute" data though and having to traverse many checkouts
> trying to build up say "average task timing" might be problematic.
> 
> > As far as timestamps go, BUILDNAME does go into the commit message; if
> > it helps we record the metadata revisions corresponding to the build
> > in the buildhistory repo as well (in a "metadata-revs" file).
> 
> Currently the main use of the data is to say "visualise this build"
> where you're building a target or set of targets. We name the build after the
> first item in the list of build targets.
> 
> The logged data is useful both as part of a set which are viewed together and
> individually on a task basis so see how much CPU/Disk a given task uses and
> how that changes over time.
> 
> The tricky part is firstly that we should really use a true buildname, secondly
> that we need both the set of data and data per target.
> 
> If we just logged data per target, we could overwrite any previous data but
> that would break usage as a set.
> 
> > If we do want to keep the builds split out in separate per-build
> > directories then what are we gaining by trying to push buildstats into
> > buildhistory in the first place?
> 
> We have a need to be able to merge and store build statistic data which build
> history already does well. The question is whether it makes sense to use the
> same mechanism or not since there are commonalities and differences.
> Having two data storage systems and repositories doesn't make a lot of
> sense.
> 
> I'm not sure this makes a solution much clearer but it does hopefully better
> explain the challenge.

We could use add code to buildhistory-diff and make it return comparison buildstats data for a set of builds ?
Usually people look at statistics for a certain task at a time and such a tool could actually make things easier.

Regards,
Corneliu
Paul Eggleton - Nov. 7, 2013, 10:38 a.m.
On Monday 04 November 2013 12:42:59 Stoicescu, CorneliuX wrote:
> On Monday 04 November 2013 14:35:00 Richard Purdie wrote:
> > On Mon, 2013-11-04 at 12:14 +0000, Paul Eggleton wrote:
> > > On Monday 04 November 2013 09:31:19 Richard Purdie wrote:
> > > The basic idea with buildhistory is to only keep the latest version of
> > > data stored within it, on the assumption that you could always get
> > > previous versions from the git history; this also makes comparisons to
> > > previous versions straightforward.
> > 
> > This makes sense for absolute value data. The task build statistics are
> > not
> > entirely "absolute" data though and having to traverse many checkouts
> > trying to build up say "average task timing" might be problematic.
> > 
> > > As far as timestamps go, BUILDNAME does go into the commit message; if
> > > it helps we record the metadata revisions corresponding to the build
> > > in the buildhistory repo as well (in a "metadata-revs" file).
> > 
> > Currently the main use of the data is to say "visualise this build"
> > where you're building a target or set of targets. We name the build after
> > the first item in the list of build targets.
> > 
> > The logged data is useful both as part of a set which are viewed together
> > and individually on a task basis so see how much CPU/Disk a given task
> > uses and how that changes over time.
> > 
> > The tricky part is firstly that we should really use a true buildname,
> > secondly that we need both the set of data and data per target.
> > 
> > If we just logged data per target, we could overwrite any previous data
> > but
> > that would break usage as a set.
> > 
> > > If we do want to keep the builds split out in separate per-build
> > > directories then what are we gaining by trying to push buildstats into
> > > buildhistory in the first place?
> > 
> > We have a need to be able to merge and store build statistic data which
> > build history already does well. The question is whether it makes sense
> > to use the same mechanism or not since there are commonalities and
> > differences. Having two data storage systems and repositories doesn't
> > make a lot of sense.
> > 
> > I'm not sure this makes a solution much clearer but it does hopefully
> > better explain the challenge.
> 
> We could use add code to buildhistory-diff and make it return comparison
> buildstats data for a set of builds ? Usually people look at statistics for
> a certain task at a time and such a tool could actually make things easier.

Just to round this off - I still have some reservations about adding this data 
in its current form to buildhistory, but being able to associate it with the 
build and other changes that occurred at the same time is valuable. Let's add 
it under ${BUILDHISTORY_DIR} using ${BUILDNAME} as a subdirectory as Richard 
suggests, and then later we can add some analysis code and maybe optional 
automatic pruning of old data to keep things tidy.

Cheers,
Paul

Patch

diff --git a/meta/classes/buildhistory.bbclass b/meta/classes/buildhistory.bbclass
index 3da03c8..a78bd4b 100644
--- a/meta/classes/buildhistory.bbclass
+++ b/meta/classes/buildhistory.bbclass
@@ -19,6 +19,7 @@  BUILDHISTORY_PUSH_REPO ?= ""
 
 # Must inherit package first before changing PACKAGEFUNCS
 inherit package
+inherit buildstats
 PACKAGEFUNCS += "buildhistory_emit_pkghistory"
 
 # We don't want to force a rerun of do_package for everything
diff --git a/meta/classes/buildstats.bbclass b/meta/classes/buildstats.bbclass
index 72fff11..476ae94 100644
--- a/meta/classes/buildstats.bbclass
+++ b/meta/classes/buildstats.bbclass
@@ -1,4 +1,4 @@ 
-BUILDSTATS_BASE = "${TMPDIR}/buildstats/"
+BUILDSTATS_BASE = "${BUILDHISTORY_DIR}/buildstats"
 BNFILE = "${BUILDSTATS_BASE}/.buildname"
 DEVFILE = "${BUILDSTATS_BASE}/.device"
 
@@ -33,7 +33,7 @@  def set_bn(e):
 
 def get_bn(e):
     with open(e.data.getVar('BNFILE', True)) as f:
-        bn = f.readline()
+        bn = str(f.readline()).split("/")[0]
     return bn
 
 def set_device(e):
@@ -175,6 +175,10 @@  python run_buildstats () {
         # set the buildname
         ########################################################################
         try:
+            bb.utils.remove(e.data.getVar('BUILDSTATS_BASE', True), recurse=True)
+        except:
+            pass
+        try:
             bb.utils.mkdirhier(e.data.getVar('BUILDSTATS_BASE', True))
         except:
             pass