[v2] arm-trusted-firmware-mvebu: stop cluttering Image Builder

From: Tomasz Maciej Nowak <tmn505@gmail.com>

All contents of staging_dir/image are included in Image Builder (IB) in
case some binary needs to be included in final image. But in case of
this package, all sources are stored there and those clutter the final
tarball of IB for no reason. Those sources are not used during image
creation and are just dead weight. To put it in perspective, the IB for
21.02.0 is 158 MiB, 22.03.0-rc6 is 366 MiB and snapshot is over 620 MiB!
To fix it, put them in package build directory, so they won't end up
included in IB tarball. With that, the custom clean definition can be
removed, as the default one will take over.

Signed-off-by: Tomasz Maciej Nowak <tmn505@gmail.com>
Reviewed-by: Andre Heider <a.heider@gmail.com>
---
v1 -> v2
- as pointed by Andre, we can use default clean definition
- add his review tag

 .../boot/arm-trusted-firmware-mvebu/Makefile  | 40 +++++++------------
 1 file changed, 15 insertions(+), 25 deletions(-)

On 8/31/22 17:03, Tomasz Maciej Nowak wrote:
> From: Tomasz Maciej Nowak <tmn505@gmail.com>
> 
> All contents of staging_dir/image are included in Image Builder (IB) in
> case some binary needs to be included in final image. But in case of
> this package, all sources are stored there and those clutter the final
> tarball of IB for no reason. Those sources are not used during image
> creation and are just dead weight. To put it in perspective, the IB for
> 21.02.0 is 158 MiB, 22.03.0-rc6 is 366 MiB and snapshot is over 620 MiB!
> To fix it, put them in package build directory, so they won't end up
> included in IB tarball. With that, the custom clean definition can be
> removed, as the default one will take over.
> 
> Signed-off-by: Tomasz Maciej Nowak <tmn505@gmail.com>
> Reviewed-by: Andre Heider <a.heider@gmail.com>
> ---
> v1 -> v2
> - as pointed by Andre, we can use default clean definition
> - add his review tag

I think the build times are much higher with this change compared to 
before. I haven't found the time to do real measurements. I used this 
configuration to test:
CONFIG_TARGET_mvebu=y
CONFIG_TARGET_mvebu_cortexa53=y
CONFIG_TARGET_mvebu_cortexa53_DEVICE_globalscale_espressobin=y

Hauke

W dniu 7.09.2022 o 00:15, Hauke Mehrtens pisze:
> On 8/31/22 17:03, Tomasz Maciej Nowak wrote:
>> From: Tomasz Maciej Nowak <tmn505@gmail.com>
>>
>> All contents of staging_dir/image are included in Image Builder (IB) in
>> case some binary needs to be included in final image. But in case of
>> this package, all sources are stored there and those clutter the final
>> tarball of IB for no reason. Those sources are not used during image
>> creation and are just dead weight. To put it in perspective, the IB for
>> 21.02.0 is 158 MiB, 22.03.0-rc6 is 366 MiB and snapshot is over 620 MiB!
>> To fix it, put them in package build directory, so they won't end up
>> included in IB tarball. With that, the custom clean definition can be
>> removed, as the default one will take over.
>>
>> Signed-off-by: Tomasz Maciej Nowak <tmn505@gmail.com>
>> Reviewed-by: Andre Heider <a.heider@gmail.com>
>> ---
>> v1 -> v2
>> - as pointed by Andre, we can use default clean definition
>> - add his review tag
> 
> I think the build times are much higher with this change compared to before.

Yes, that's correct, since all source is extracted/rebuilt for each variant.
The toolchain tarball is huge and it takes lot of time to extract. The second
culprit is cryptopp. It's rebuilt for each variant with one job.

> I haven't found the time to do real measurements. I used this configuration to test:
> CONFIG_TARGET_mvebu=y
> CONFIG_TARGET_mvebu_cortexa53=y
> CONFIG_TARGET_mvebu_cortexa53_DEVICE_globalscale_espressobin=y

I did measurements, before change:
time make -j4 package/arm-trusted-firmware-mvebu/compile
[ ... ]
real    13m17,046s
user    25m24,272s
sys     1m49,963s

with the change:
time make -j4 package/arm-trusted-firmware-mvebu/compile
[ ... ]
real    28m26,566s
user    79m45,906s
sys     4m6,547s

As You can see it's more than double. I also did test where $(BUILD_DIR)
was used:
time make -j4 package/arm-trusted-firmware-mvebu/compile
[ ... ]
real    13m19,462s
user    25m21,511s
sys     1m53,047s

which looks same as in current state (ignore spinning rust hiccups). Should
I use $(BUILD_DIR) for the sources?

> 
> Hauke

Regards, Tomek

On 9/7/22 19:23, Tomasz Maciej Nowak wrote:
> W dniu 7.09.2022 o 00:15, Hauke Mehrtens pisze:
>> On 8/31/22 17:03, Tomasz Maciej Nowak wrote:
>>> From: Tomasz Maciej Nowak <tmn505@gmail.com>
>>>
>>> All contents of staging_dir/image are included in Image Builder (IB) in
>>> case some binary needs to be included in final image. But in case of
>>> this package, all sources are stored there and those clutter the final
>>> tarball of IB for no reason. Those sources are not used during image
>>> creation and are just dead weight. To put it in perspective, the IB for
>>> 21.02.0 is 158 MiB, 22.03.0-rc6 is 366 MiB and snapshot is over 620 MiB!
>>> To fix it, put them in package build directory, so they won't end up
>>> included in IB tarball. With that, the custom clean definition can be
>>> removed, as the default one will take over.
>>>
>>> Signed-off-by: Tomasz Maciej Nowak <tmn505@gmail.com>
>>> Reviewed-by: Andre Heider <a.heider@gmail.com>
>>> ---
>>> v1 -> v2
>>> - as pointed by Andre, we can use default clean definition
>>> - add his review tag
>>
>> I think the build times are much higher with this change compared to before.
> 
> Yes, that's correct, since all source is extracted/rebuilt for each variant.
> The toolchain tarball is huge and it takes lot of time to extract. The second
> culprit is cryptopp. It's rebuilt for each variant with one job.
> 
>> I haven't found the time to do real measurements. I used this configuration to test:
>> CONFIG_TARGET_mvebu=y
>> CONFIG_TARGET_mvebu_cortexa53=y
>> CONFIG_TARGET_mvebu_cortexa53_DEVICE_globalscale_espressobin=y
> 
> I did measurements, before change:
> time make -j4 package/arm-trusted-firmware-mvebu/compile
> [ ... ]
> real    13m17,046s
> user    25m24,272s
> sys     1m49,963s
> 
> with the change:
> time make -j4 package/arm-trusted-firmware-mvebu/compile
> [ ... ]
> real    28m26,566s
> user    79m45,906s
> sys     4m6,547s
> 
> As You can see it's more than double. I also did test where $(BUILD_DIR)
> was used:
> time make -j4 package/arm-trusted-firmware-mvebu/compile
> [ ... ]
> real    13m19,462s
> user    25m21,511s
> sys     1m53,047s
> 
> which looks same as in current state (ignore spinning rust hiccups). Should
> I use $(BUILD_DIR) for the sources?

did you mean a common folder in $(BUILD_DIR) with your $(BUILD_DIR) 
approach?

I think it would be better to place the bootloader toolchain in some 
common place and not duplicated for each variant.
Is cryptopp also common for all variant and only used on the host?

Maybe we can make this all a host package?

Hauke

W dniu 7.09.2022 o 22:57, Hauke Mehrtens pisze:
> On 9/7/22 19:23, Tomasz Maciej Nowak wrote:
>> W dniu 7.09.2022 o 00:15, Hauke Mehrtens pisze:
>>> On 8/31/22 17:03, Tomasz Maciej Nowak wrote:
>>>> From: Tomasz Maciej Nowak <tmn505@gmail.com>
>>>>
>>>> All contents of staging_dir/image are included in Image Builder (IB) in
>>>> case some binary needs to be included in final image. But in case of
>>>> this package, all sources are stored there and those clutter the final
>>>> tarball of IB for no reason. Those sources are not used during image
>>>> creation and are just dead weight. To put it in perspective, the IB for
>>>> 21.02.0 is 158 MiB, 22.03.0-rc6 is 366 MiB and snapshot is over 620 MiB!
>>>> To fix it, put them in package build directory, so they won't end up
>>>> included in IB tarball. With that, the custom clean definition can be
>>>> removed, as the default one will take over.
>>>>
>>>> Signed-off-by: Tomasz Maciej Nowak <tmn505@gmail.com>
>>>> Reviewed-by: Andre Heider <a.heider@gmail.com>
>>>> ---
>>>> v1 -> v2
>>>> - as pointed by Andre, we can use default clean definition
>>>> - add his review tag
>>>
>>> I think the build times are much higher with this change compared to before.
>>
>> Yes, that's correct, since all source is extracted/rebuilt for each variant.
>> The toolchain tarball is huge and it takes lot of time to extract. The second
>> culprit is cryptopp. It's rebuilt for each variant with one job.
>>
>>> I haven't found the time to do real measurements. I used this configuration to test:
>>> CONFIG_TARGET_mvebu=y
>>> CONFIG_TARGET_mvebu_cortexa53=y
>>> CONFIG_TARGET_mvebu_cortexa53_DEVICE_globalscale_espressobin=y
>>
>> I did measurements, before change:
>> time make -j4 package/arm-trusted-firmware-mvebu/compile
>> [ ... ]
>> real    13m17,046s
>> user    25m24,272s
>> sys     1m49,963s
>>
>> with the change:
>> time make -j4 package/arm-trusted-firmware-mvebu/compile
>> [ ... ]
>> real    28m26,566s
>> user    79m45,906s
>> sys     4m6,547s
>>
>> As You can see it's more than double. I also did test where $(BUILD_DIR)
>> was used:
>> time make -j4 package/arm-trusted-firmware-mvebu/compile
>> [ ... ]
>> real    13m19,462s
>> user    25m21,511s
>> sys     1m53,047s
>>
>> which looks same as in current state (ignore spinning rust hiccups). Should
>> I use $(BUILD_DIR) for the sources?
> 
> did you mean a common folder in $(BUILD_DIR) with your $(BUILD_DIR) approach?

Simply $(BUILD_DIR), which in our example translates to
"build_dir/target-aarch64_cortex-a53_musl" without any subdir. Naturally each
source is untarred to own subdir.

> I think it would be better to place the bootloader toolchain in some common place and not duplicated for each variant.

That's with current state using STAGING_DIR_IMAGE or with using BUILD_DIR,
so I'll use BUILD_DIR instead of PKG_BUILD_DIR.

> Is cryptopp also common for all variant and only used on the host?

Yes to both.

> Maybe we can make this all a host package?

That would make sense if we would untangle the whole TF-A build for mvebu,
which at current form, needs work, as we already discussed that [1]. Having
a second look at the build flow of it, I saw it's even more entangled than 
what I described in that topic. Anyway, lets keep it in current state, until
me/someone finds time to fix it.

1. http://lists.openwrt.org/pipermail/openwrt-devel/2020-October/031677.html

> Hauke

Regards, Tomek