[PATCH 2/2] lz4: use CFLAGS from bitbake

[PATCH 2/2] lz4: use CFLAGS from bitbake

[Mikko Rapeli]

Currently lz4 uses it's own defaults which include O3 optimization.
Switch from O3 to bitbake default O2 reduces binary package size
from 467056 to 331888 bytes. Enables also building with Os if needed.

Signed-off-by: Mikko Rapeli <mikko.rapeli@bmw.de>
---
 meta/recipes-support/lz4/lz4_1.9.3.bb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meta/recipes-support/lz4/lz4_1.9.3.bb b/meta/recipes-support/lz4/lz4_1.9.3.bb
index effc530b94..3905ef7dbc 100644
--- a/meta/recipes-support/lz4/lz4_1.9.3.bb
+++ b/meta/recipes-support/lz4/lz4_1.9.3.bb
@@ -22,7 +22,7 @@ S = "${WORKDIR}/git"
 # Fixed in r118, which is larger than the current version.
 CVE_CHECK_WHITELIST += "CVE-2014-4715"
 
-EXTRA_OEMAKE = "PREFIX=${prefix} CC='${CC}' DESTDIR=${D} LIBDIR=${libdir} INCLUDEDIR=${includedir} BUILD_STATIC=no"
+EXTRA_OEMAKE = "PREFIX=${prefix} CC='${CC}' CFLAGS='${CFLAGS}' DESTDIR=${D} LIBDIR=${libdir} INCLUDEDIR=${includedir} BUILD_STATIC=no"
 
 do_install() {
 	oe_runmake install
-- 
2.20.1

Re: [OE-core] [PATCH 2/2] lz4: use CFLAGS from bitbake

[Khem Raj]

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

On Thu, Apr 22, 2021 at 7:42 AM Mikko Rapeli <mikko.rapeli@bmw.de> wrote:

> Currently lz4 uses it's own defaults which include O3 optimization.
> Switch from O3 to bitbake default O2 reduces binary package size
> from 467056 to 331888 bytes. Enables also building with Os if needed.


These could impact runtime performance have you
Checked what the rough impact is ?
Secondly we will be using non default set which could result in errors less
seen by others over time I have realized it’s also good to open a dialog
upstream and get the reasoning behind not letting distro defaults apply
some packages do have valid reasons


>
> Signed-off-by: Mikko Rapeli <mikko.rapeli@bmw.de>
> ---
>  meta/recipes-support/lz4/lz4_1.9.3.bb | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/meta/recipes-support/lz4/lz4_1.9.3.bb
> b/meta/recipes-support/lz4/lz4_1.9.3.bb
> index effc530b94..3905ef7dbc 100644
> --- a/meta/recipes-support/lz4/lz4_1.9.3.bb
> +++ b/meta/recipes-support/lz4/lz4_1.9.3.bb
> @@ -22,7 +22,7 @@ S = "${WORKDIR}/git"
>  # Fixed in r118, which is larger than the current version.
>  CVE_CHECK_WHITELIST += "CVE-2014-4715"
>
> -EXTRA_OEMAKE = "PREFIX=${prefix} CC='${CC}' DESTDIR=${D} LIBDIR=${libdir}
> INCLUDEDIR=${includedir} BUILD_STATIC=no"
> +EXTRA_OEMAKE = "PREFIX=${prefix} CC='${CC}' CFLAGS='${CFLAGS}'
> DESTDIR=${D} LIBDIR=${libdir} INCLUDEDIR=${includedir} BUILD_STATIC=no"
>
>  do_install() {
>         oe_runmake install
> --
> 2.20.1
>
>
> 
>
>

[-- Attachment #2: Type: text/html, Size: 2563 bytes --]

Re: [OE-core] [PATCH 2/2] lz4: use CFLAGS from bitbake

[Mikko Rapeli]

On Thu, Apr 22, 2021 at 08:05:20AM -0700, Khem Raj wrote:
> On Thu, Apr 22, 2021 at 7:42 AM Mikko Rapeli <mikko.rapeli@bmw.de> wrote:
> 
> > Currently lz4 uses it's own defaults which include O3 optimization.
> > Switch from O3 to bitbake default O2 reduces binary package size
> > from 467056 to 331888 bytes. Enables also building with Os if needed.
> 
> 
> These could impact runtime performance have you
> Checked what the rough impact is ?

Nope, have not checked this. I've build my targets with this patch
and have not noticed any major performance regression. If there is
a performance problem, then I'd rather see O3 explicitly set in the recipe,
for example for native target.

> Secondly we will be using non default set which could result in errors less
> seen by others over time I have realized it’s also good to open a dialog
> upstream and get the reasoning behind not letting distro defaults apply
> some packages do have valid reasons

I've seen bugs with O3 optimziation level too on some target archs.
I would not enable that by default. But I do understand your concern.

In similar ways, I was checking ffmpeg, attr, acl etc which were defaulting
to O3 in older versions but with new poky master versions now behave well
and use bitbake default O2. I guess there too no major performance regressions
have been detected.

Cheers,

-Mikko

Re: [OE-core] [PATCH 2/2] lz4: use CFLAGS from bitbake

[Khem Raj]

On Thu, Apr 22, 2021 at 10:49 PM <Mikko.Rapeli@bmw.de> wrote:
>
> On Thu, Apr 22, 2021 at 08:05:20AM -0700, Khem Raj wrote:
> > On Thu, Apr 22, 2021 at 7:42 AM Mikko Rapeli <mikko.rapeli@bmw.de> wrote:
> >
> > > Currently lz4 uses it's own defaults which include O3 optimization.
> > > Switch from O3 to bitbake default O2 reduces binary package size
> > > from 467056 to 331888 bytes. Enables also building with Os if needed.
> >
> >
> > These could impact runtime performance have you
> > Checked what the rough impact is ?
>
> Nope, have not checked this. I've build my targets with this patch
> and have not noticed any major performance regression. If there is
> a performance problem, then I'd rather see O3 explicitly set in the recipe,
> for example for native target.
>
> > Secondly we will be using non default set which could result in errors less
> > seen by others over time I have realized it’s also good to open a dialog
> > upstream and get the reasoning behind not letting distro defaults apply
> > some packages do have valid reasons
>
> I've seen bugs with O3 optimziation level too on some target archs.
> I would not enable that by default. But I do understand your concern.
>
> In similar ways, I was checking ffmpeg, attr, acl etc which were defaulting
> to O3 in older versions but with new poky master versions now behave well
> and use bitbake default O2. I guess there too no major performance regressions
> have been detected.
>

Thanks for checking this out. Perhaps it will be good to ping upstream
and raise this
that we are changing default opts,  are there any concerns we should know.

> Cheers,
>
> -Mikko

Re: [OE-core] [PATCH 2/2] lz4: use CFLAGS from bitbake

[Mikko Rapeli]

On Thu, Apr 22, 2021 at 11:00:40PM -0700, Khem Raj wrote:
> On Thu, Apr 22, 2021 at 10:49 PM <Mikko.Rapeli@bmw.de> wrote:
> >
> > On Thu, Apr 22, 2021 at 08:05:20AM -0700, Khem Raj wrote:
> > > On Thu, Apr 22, 2021 at 7:42 AM Mikko Rapeli <mikko.rapeli@bmw.de> wrote:
> > >
> > > > Currently lz4 uses it's own defaults which include O3 optimization.
> > > > Switch from O3 to bitbake default O2 reduces binary package size
> > > > from 467056 to 331888 bytes. Enables also building with Os if needed.
> > >
> > >
> > > These could impact runtime performance have you
> > > Checked what the rough impact is ?
> >
> > Nope, have not checked this. I've build my targets with this patch
> > and have not noticed any major performance regression. If there is
> > a performance problem, then I'd rather see O3 explicitly set in the recipe,
> > for example for native target.
> >
> > > Secondly we will be using non default set which could result in errors less
> > > seen by others over time I have realized it’s also good to open a dialog
> > > upstream and get the reasoning behind not letting distro defaults apply
> > > some packages do have valid reasons
> >
> > I've seen bugs with O3 optimziation level too on some target archs.
> > I would not enable that by default. But I do understand your concern.
> >
> > In similar ways, I was checking ffmpeg, attr, acl etc which were defaulting
> > to O3 in older versions but with new poky master versions now behave well
> > and use bitbake default O2. I guess there too no major performance regressions
> > have been detected.
> >
> 
> Thanks for checking this out. Perhaps it will be good to ping upstream
> and raise this
> that we are changing default opts,  are there any concerns we should know.

Well, to me the global optimization flags set by bitbake are a distro
wide policy which we on bitbake side are allowed and should do when ever we
please. Upstreams may have different opinions, but all I've seen is that
those who think O3 is good or better, usually end up adding a lot of
workarounds due to various buggy compiler versions, e.g. fall back to O2.

I checked lz4 chat and found this which even says that O2 is faster
on some arch and benchmark:

https://groups.google.com/g/lz4c/c/L0caJWd-vCY

Now, if anyone wants to try to use Os everywhere to reduce binary sizes, then
respecting bitbake compile flags becomes important. The exceptions to the
defaults need to be evaluated per use case, per target and with measurements.

And sorry, for this change I don't have measurements of the effect except for
binary size.

-Mikko