Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve compilation speed for android build #166

Open
wants to merge 1 commit into
base: celadon/s/mr0/master
Choose a base branch
from

Conversation

sgnanase
Copy link
Contributor

FFTF python script used for flashfiles.zip in IVI. It uses ZipWrite python api to create it. This method is time consuming and adds several unwanted build steps that adds overhead. Like, flash.json generation, unpacking of target files, installer.cmd generation.

This fix, changes FFTF python to a normal FFTF bash script, which uses tar + pigz combo for faster flashfiles generation.

NOTE:
We deprecate flash.json as its used only by PFT & installer.cmd will be maintained in product config repo.

Tracked-On: OAM-112083

@sysopenci
Copy link

Android CI has started Engineering Build for this issue ,Please check the linked Tracked-On issue/Android CI Web for more details.

1 similar comment
@sysopenci
Copy link

Android CI has started Engineering Build for this issue ,Please check the linked Tracked-On issue/Android CI Web for more details.

@sysopenci
Copy link

Android CI has started Engineering Build for this issue ,Please check the linked Tracked-On issue/Android CI Web for more details.

@sysopenci
Copy link

Android CI has completed Engineering Build for this issue, build is FAILURE. Please check the linked Tracked-On issue/Android CI Web for more details

2 similar comments
@sysopenci
Copy link

Android CI has completed Engineering Build for this issue, build is FAILURE. Please check the linked Tracked-On issue/Android CI Web for more details

@sysopenci
Copy link

Android CI has completed Engineering Build for this issue, build is FAILURE. Please check the linked Tracked-On issue/Android CI Web for more details

@sysopenci
Copy link

Android CI has started Engineering Build for this issue ,Please check the linked Tracked-On issue/Android CI Web for more details.

@@ -349,20 +366,20 @@ flashfiles: $(INTEL_FACTORY_FLASHFILES_TARGET) publish_gptimage_var publish_mkdi
ifeq (,$(filter apollo_ivi blizzard_ivi celadon_ivi,$(TARGET_PRODUCT)))
@echo "Publishing Release files started"
$(hide) mkdir -p $(TOP)/pub/$(TARGET_PRODUCT)/$(TARGET_BUILD_VARIANT)/Release_Files
$(hide) cp -r $(PRODUCT_OUT)/*-flashfiles-*.zip $(TOP)/pub/$(TARGET_PRODUCT)/$(TARGET_BUILD_VARIANT)/Release_Files
$(hide) cp -r $(PRODUCT_OUT)/*-flashfiles-*.$(fn_compress_format) $(TOP)/pub/$(TARGET_PRODUCT)/$(TARGET_BUILD_VARIANT)/Release_Files
$(hide) cp -r $(PRODUCT_OUT)/scripts $(TOP)/pub/$(TARGET_PRODUCT)/$(TARGET_BUILD_VARIANT)/Release_Files
$(hide) cp -r vendor/intel/utils/host $(TOP)/pub/$(TARGET_PRODUCT)/$(TARGET_BUILD_VARIANT)/Release_Files
$(hide) mv $(TOP)/pub/$(TARGET_PRODUCT)/$(TARGET_BUILD_VARIANT)/Release_Files/host $(TOP)/pub/$(TARGET_PRODUCT)/$(TARGET_BUILD_VARIANT)/Release_Files/patches
$(hide) cp -r $(TOP)/pub/$(TARGET_PRODUCT)/$(TARGET_BUILD_VARIANT)/Release_Files/* $(TOP)
ifneq (,$(wildcard vendor/intel/utils_vertical))
ifneq (,$(wildcard vendor/intel/fw/keybox_provisioning))
@echo "vertical_keybox_provisioning included"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need to check LEGACY_FLASHZIP_TRUE here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fn_compress_format is already decided based on LEGACY_FLASHZIP_TRUE flag.

ifeq ($(LEGACY_FLASHZIP_TRUE),true)
fn_compress_format := zip
else
fn_compress_format := tar.gz
endif

We dont need to add another condition check for this pls.

@@ -286,7 +303,7 @@ ISO_INSTALL_IMG = $(PRODUCT_OUT)/$(TARGET_PRODUCT)-sign-flashfile-$(FILE_NAME_TA
else
ISO_INSTALL_IMG = $(PRODUCT_OUT)/$(TARGET_PRODUCT)-flashfile-$(FILE_NAME_TAG).iso
endif
ISO_INSTALL_IMG_ZIP = $(ISO_INSTALL_IMG).zip
ISO_INSTALL_IMG_ZIP = $(ISO_INSTALL_IMG).tar.gz

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

better to rename ISO_INSTALL_IMG_ZIP to ISO_INSTALL_IMG_TAR_GZ

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will make this change (y)

@sysopenci
Copy link

Android CI has completed Engineering Build for this issue, build is SUCCESS.To merge the changes please click on APPROVE FOR MERGE button in Android CI WEB

Copy link

@yang8621 yang8621 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Can we have a test:
    rename /usr/bin/pigz to /usr/bin/pigz_bak
    make flashfiles LEGACY_FLASHZIP_TRUE -jxx

  2. Not sure pigz is Ubuntu built-in or manually installed? If it's manually installed, it can be added into env setup wiki.

@sgnanase
Copy link
Contributor Author

  1. Can we have a test:
    rename /usr/bin/pigz to /usr/bin/pigz_bak
    make flashfiles LEGACY_FLASHZIP_TRUE -jxx
  2. Not sure pigz is Ubuntu built-in or manually installed? If it's manually installed, it can be added into env setup wiki.

By default, for ISO we are moving to tar+pigz from zip, So we need /usr/bin/pigz by default. We will capture this as pre-requisite.

Copy link

@sysopenci sysopenci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR integrated into 'preintegration' builder

More details, logs, and binaries: /absp/builders/celadon_smr0_master-preintegration/builds/857

Done by "Gnanasekaran, Sundar" [email protected]

@sysopenci
Copy link

Android CI has completed Engineering Build for this issue, build is FAILURE. Please check the linked Tracked-On issue/Android CI Web for more details

@asaddamhu
Copy link

@sgnanase , please don't deprecate flash.json as it is powerful, supports multi configuration flashing and steps.
We have created a simple tool to flash android(replacement for PFT) using 'flash.json' and 'installer.cmd'. Please consider this as a requirement to keep and maintain flash.json.

Thanks!

Currently FFTF python script is used for creating flashfiles.zip.
It uses ZipWrite python api which is time consuming and adds
overhead to build time. Say, flash.json generation, unpacking of
target files, installer.cmd generation, etc.

This fix helps to optimize build time and involves below changes:
- Use tar + pigz combo for faster flashfiles generation.
- Make FFTF a bash script as python doesn't directly support
  tar + pigz
- This new bash script will support old python FFTF. Just pass
  LEGACY_FLASHZIP_TRUE=true flag to "make flashfiles" to enable it.
- The new script still supports all old functionalities, say,
  generation of installer.cmd, flash.json, repacking of
  bootloader.img, etc.

Tracked-On: OAM-112083
Signed-off-by: sgnanase <[email protected]>
echo "Generate installer.cmd"
echo "========================"
device/intel/build/releasetools/flash_cmd_generator.py device/intel/project-celadon/$TARGET/flashfiles.ini $TARGET $VARIANT | sed '$d' | sed '$d' | sed -n '/installer.cmd/,$p' | sed '1d' > $PRODUCT_OUT/$flashfile_dir/installer.cmd
sed -i 's/flash super super.img/flash super super.img.part00 super.img.part01/g' $PRODUCT_OUT/$flashfile_dir/installer.cmd
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if image is smaller than 4G, there is only super.img. Ff image is bigger than 4G, there will be super.img.part00, super.img.part01, if image is bigger then 8G, there will be super.img.part00, super.img.part01, super.img.part02...
You can decide refine the patch or submit another patch to fix.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One more comment is that flash.json will be used by new flash tool, did you keep it?

@sysopenci sysopenci added the Stale Stale label for inactive open prs label Sep 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Stale Stale label for inactive open prs Valid commit message
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants