Generate package size statistics

Currently, all the installation work of the toolchain-external package is done during the install-staging step. However, in order to be able to properly collect the size added by each package to the target filesystem, we need to make sure that toolchain-external installs its files to $(TARGET_DIR) during the install-target step.

Signed-off-by Thomas Petazzoni <[email protected]>

---
 toolchain/toolchain-external/toolchain-external.mk | 36 +++++++++++++++++-----
 1 file changed, 29 insertions(+), 7 deletions(-)

Currently, all the installation work of the toolchain-external package is done during the install-staging step. However, in order to be able to properly collect the size added by each package to the target filesystem, we need to make sure that toolchain-external installs its files to $(TARGET_DIR) during the install-target step.

Signed-off-by Thomas Petazzoni <[email protected]>

---
 toolchain/toolchain-external/toolchain-external.mk | 36 +++++++++++++++++-----
 1 file changed, 29 insertions(+), 7 deletions(-)

This patch adds a global instrumentation hook that collects the list of files installed in $(TARGET_DIR) by each package, and stores this list into a file called $(BUILD_DIR)/packages-file-list.txt. It can later be used to determine the size contribution of each package to the target root filesystem.

Note that in order to detect if a file installed by one package is later overriden by another package, we calculate the md5 of installed files and compare them at each installation of a new package.

This commit also adds a Config.in option to enable the collection of this data, as calculating the md5 of all installed files at the beginning and end of the installation of each package can be considered a time-consuming process which maybe some users will not be willing to suffer from.

Signed-off-by Thomas Petazzoni <[email protected]>

---
 Config.in              |  9 +++++++++
 package/pkg-generic.mk | 36 ++++++++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+)

Config.in +9 -0

index 1aa1080..328654c 100644
@@ -569,6 +569,15 @@ config BR2_GLOBAL_PATCH_DIR
 	  Otherwise, if the directory <global-patch-dir>/<packagename> exists,
 	  then all *.patch files in the directory will be applied.
 
+config BR2_COLLECT_FILE_SIZE_STATS
+	bool "collect statistics about installed file size"
+	help
+	  Enable this option to let Buildroot collect data about the
+	  installed files. When this option is enabled, you will be
+	  able to use the 'size-stats' make target, which will
+	  generate a graph and CSV files giving statistics about the
+	  installed size of each file and each package.
+
 endmenu
 
 source "toolchain/Config.in"

package/pkg-generic.mk +1 -0

index 9643a30..82f8ff8 100644
@@ -55,6 +55,42 @@ define step_time
 endef
 GLOBAL_INSTRUMENTATION_HOOKS += step_time
 
+# Hooks to collect statistics about installed files

package/pkg-generic.mk +12 -0

+	$(RM) -f $(BUILD_DIR)/$(1).filelist_before \
+		$(BUILD_DIR)/$(1).filelist_after
+endef
+
+define step_pkg_size
+	$(if $(filter install-target,$(2)),\
+		$(if $(filter start,$(1)),$(call step_pkg_size_start,$(3))) \
+		$(if $(filter end,$(1)),$(call step_pkg_size_end,$(3))))
+endef
+GLOBAL_INSTRUMENTATION_HOOKS += step_pkg_size
+endif
+
 # User-supplied script
 ifneq ($(BR2_INSTRUMENTATION_SCRIPTS),)
 define step_user

package/pkg-generic.mk define step_time · line 68

Quoted 8 lines

package/pkg-generic.mk +8 -0

+# This hook will be called before the target installation of a
+# package. We store in a file named $(1).filelist_before the list of
+# files currently installed in the target. Note that the MD5 is also
+# stored, in order to identify if the files are overwritten.
+define step_pkg_size_start
+	(cd $(TARGET_DIR) ; find . -type f | xargs md5sum) | sort > \
+		$(BUILD_DIR)/$(1).filelist_before
+endef

I think this does not work if filename contains spaces.

Quoted 6 lines

package/pkg-generic.mk +6 -0

+# This hook will be called after the target installation of a
+# package. We store in a file named $(1).filelist_after the list
+# of files (and their MD5) currently installed in the target. We then
+# do a diff with the $(1).filelist_before to compute the list of
+# files installed by this package.
+define step_pkg_size_end

package/pkg-generic.mk define step_time · line 81

Quoted 6 lines

package/pkg-generic.mk +6 -0

+	(cd $(TARGET_DIR); find . -type f | xargs md5sum) | sort > \
+		$(BUILD_DIR)/$(1).filelist_after
+	comm -13 $(BUILD_DIR)/$(1).filelist_before $(BUILD_DIR)/$(1).filelist_after | \
+		while read hash file ; do \
+			echo "$(1),$${file}" >> $(BUILD_DIR)/packages-file-list.txt ; \
+		done

Does it would make sense if we also record removed lines? We may wrote another script that detect if a file was in conflict between two packages.

Dear J?r?me Pouiller,

Quoted 9 lines · J?r?me Pouiller

package/pkg-generic.mk define step_time · line 68

Quoted 8 lines

package/pkg-generic.mk +8 -0

+# This hook will be called before the target installation of a
+# package. We store in a file named $(1).filelist_before the list of
+# files currently installed in the target. Note that the MD5 is also
+# stored, in order to identify if the files are overwritten.
+define step_pkg_size_start
+	(cd $(TARGET_DIR) ; find . -type f | xargs md5sum) | sort > \
+		$(BUILD_DIR)/$(1).filelist_before
+endef

I think this does not work if filename contains spaces.

Hum, yes, very possible. But is Buildroot really working fine as a whole if some file in the target filesystem has some spaces?

Quoted 6 lines

Quoted 6 lines

package/pkg-generic.mk +6 -0

+# This hook will be called after the target installation of a
+# package. We store in a file named $(1).filelist_after the list
+# of files (and their MD5) currently installed in the target. We then
+# do a diff with the $(1).filelist_before to compute the list of
+# files installed by this package.
+define step_pkg_size_end

Quoted 8 lines

package/pkg-generic.mk define step_time · line 81

Quoted 6 lines

package/pkg-generic.mk +6 -0

+	(cd $(TARGET_DIR); find . -type f | xargs md5sum) | sort > \
+		$(BUILD_DIR)/$(1).filelist_after
+	comm -13 $(BUILD_DIR)/$(1).filelist_before $(BUILD_DIR)/$(1).filelist_after | \
+		while read hash file ; do \
+			echo "$(1),$${file}" >> $(BUILD_DIR)/packages-file-list.txt ; \
+		done

Does it would make sense if we also record removed lines? We may wrote another script that detect if a file was in conflict between two packages.

I'm not sure to follow you here. We already take care of packages installing the same file, that's the whole point of storing the MD5 of each file. By using comm -13, we keep only the lines that are unique in the second file (compared to the first file). So we keep lines for either new files added by this package, or files already installed but overwritten by the package (detected using the MD5).

On Tuesday 02 December 2014 13:23:49 Thomas Petazzoni wrote:

I just noticed another thing. To make this feature compatible to BR2_JLEVEL, we just need to manage a mutex in step_pkg_size hook. Do

you planned to add one?

This new script uses the data collected by the step_pkg_size instrumentation hook to generate a pie chart of the size contribution of each package to the target root filesystem, and two CSV files with statistics about the package size and file size. To achieve this, it looks at each file in $(TARGET_DIR), and using the packages-file-list.txt information collected by the step_pkg_size hook, it determines to which package the file belongs. It is therefore able to give the size installed by each package.

Signed-off-by Thomas Petazzoni <[email protected]>

---
 support/scripts/size-stats | 225 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 225 insertions(+)
 create mode 100755 support/scripts/size-stats

support/scripts/size-stats new file +1 -0

new file mode 100755
index 0000000..7dc28a0
@@ -0,0 +1,225 @@
+#!/usr/bin/env python

support/scripts/size-stats new file +12 -0

+#
+# This function returns a dict containing as keys the files present in
+# the filesystem skeleton, and as value, the string "skeleton". It is
+# used to simulate a fake "skeleton" package, to assign the files from
+# the skeleton to some package.
+#
+# builddir: path to the Buildroot output directory
+#
+def build_skeleton_dict(builddir):
+    skeleton_files = {}
+    for root, _, files in os.walk("system/skeleton"):
+        for f in files:

support/scripts/size-stats new file +7 -0

+#
+# This function returns a dict where each key is the path of a file in
+# the root filesystem, and the value is a dict containing two
+# elements: the name of the package to which this file belongs (key:
+# pkg) and the size of the file (key: size).
+#
+# builddir: path to the Buildroot output directory

support/scripts/size-stats new file +3 -0

+    pkgdict.update(build_skeleton_dict(builddir))
+    return pkgdict
+

Is it possible to rely on Kconfiglib in order to support customized skeleton and overlays? Or you think skeleton and overlays (and post-build scripts) should be managed by package infra (like https://patchwork.ozlabs.org/patch/399413/)?

Quoted 11 lines

support/scripts/size-stats new file +11 -0

+#
+# This function returns a dict where each key is the path of a file in
+# the root filesystem, and the value is a dict containing two
+# elements: the name of the package to which this file belongs (key:
+# pkg) and the size of the file (key: size).
+#
+# builddir: path to the Buildroot output directory
+#
+def build_package_dict(builddir):
+    pkgdict = {}
+    with open(os.path.join(builddir, "build", "packages-file-list.txt")) as filelistf:

support/scripts/size-stats line 74

Quoted 6 lines

support/scripts/size-stats new file +6 -0

+        for l in filelistf.readlines():
+            f = l.split(",")
+            fpath = f[1].strip().replace("./", "")
+            fullpath = os.path.join(builddir, "target", fpath)
+            if not os.path.exists(fullpath):
+                continue

It means the file was remove by another package. You may emit a warning there?

support/scripts/size-stats line 77

Quoted 3 lines

support/scripts/size-stats new file +3 -0

+            pkg = f[0]
+            sz = os.stat(fullpath).st_size
+            pkgdict[fpath] = { 'pkg': pkg, 'size': sz }

If pkgdict[fpath] is already defined, it means:
a. pkg == pkgdict[fpath].pkg -> Package was reinstalled
b. pkg != pkgdict[fpath].pkg -> File was overwritten by another package

You may emit a warning is second case?

Dear J?r?me Pouiller,

On Tue, 02 Dec 2014 12:01:11 +0100, J?r?me Pouiller wrote:

Quoted 8 lines

support/scripts/size-stats line 77

Quoted 3 lines

support/scripts/size-stats new file +3 -0

+            pkg = f[0]
+            sz = os.stat(fullpath).st_size
+            pkgdict[fpath] = { 'pkg': pkg, 'size': sz }

If pkgdict[fpath] is already defined, it means:
a. pkg == pkgdict[fpath].pkg -> Package was reinstalled
b. pkg != pkgdict[fpath].pkg -> File was overwritten by another package

You may emit a warning is second case?

Well, it depends on whether we consider overwritten files as normal or not. For now, the main case where we overwrite things in Buildroot is when a package such as coreutils, installs some commands that are "better" than the Busybox ones, in which case coreutils wins over Busybox. But since Busybox installs symlinks, and this tool doesn't track symlinks, we don't consider this as a file being overwritten.

So maybe I could have a warning here.

Regarding "package was reinstalled", this entire script/logic is meant to be used after a "make clean all" cycle. I don't think it is worth bothering with supporting package re-installation and other crazy things that can happen outside of a normal "make clean all" cycle.

This commit implements a size-stats target that calls the script of the same name to generate the graph and CSV files related to package and file sizes.

Signed-off-by Thomas Petazzoni <[email protected]>

---
 Makefile | 11 +++++++++++
 1 file changed, 11 insertions(+)

Makefile +0 -0

index ad018c8..56b2b93 100644

Makefile +4 -0

+		--graph $(O)/graphs/graph-size.$(BR_GRAPH_OUT) \
+		--file-size-csv $(O)/build/file-size-stats.csv \
+		--package-size-csv $(O)/build/package-size-stats.csv
+
 else # ifeq ($(BR2_HAVE_DOT_CONFIG),y)
 
 all: menuconfig

Makefile graph-depends: graph-depends-requirements · line 685

Quoted 6 lines

Makefile +6 -0

+size-stats:
+	@[ -f $(O)/build/packages-file-list.txt ] || \
+		{ echo "ERROR: No package size information available, please rebuild with BR2_COLLECT_FILE_SIZE_STATS" ; exit 1; }
+	@$(INSTALL) -d $(O)/graphs
+	@cd "$(CONFIG_DIR)"; \
+	$(TOPDIR)/support/scripts/graph-size --builddir $(O) \

graph-size -> size-stats ?

But with that fixed I get:

$ make O=test/xfsprogs/ size-stats
Traceback (most recent call last):
  File "/home/naourr/git/buildroot/support/scripts/size-stats", line 217, in <module>
    pkgdict = build_package_dict(args.builddir)
  File "/home/naourr/git/buildroot/support/scripts/size-stats", line 68, in build_package_dict
    with open(os.path.join(builddir, "build", "packages-file-list.txt")) as filelistf:
IOError: [Errno 2] No such file or directory: 'test/xfsprogs/build/packages-file-list.txt'

But this file exist:

$ ls test/xfsprogs/build/packages-file-list.txt
test/xfsprogs/build/packages-file-list.txt

Also I noticed that the man pages (and headers)
are taken into account in packages-file-list.txt:
[...]
tmux,./usr/bin/tmux
tmux,./usr/share/man/man1/tmux.1

Since they are removed at the end, I think we want to remove them from packages-file-list.txt during TARGET_DIR cleanup ?

Dear Romain Naour,

Quoted 8 lines · Romain Naour

Makefile graph-depends: graph-depends-requirements · line 685

Quoted 6 lines

Makefile +6 -0

+size-stats:
+	@[ -f $(O)/build/packages-file-list.txt ] || \
+		{ echo "ERROR: No package size information available, please rebuild with BR2_COLLECT_FILE_SIZE_STATS" ; exit 1; }
+	@$(INSTALL) -d $(O)/graphs
+	@cd "$(CONFIG_DIR)"; \
+	$(TOPDIR)/support/scripts/graph-size --builddir $(O) \

graph-size -> size-stats ?

Will fix, thanks for noticing (I renamed the script late in my development).

Quoted 9 lines

But with that fixed I get:

$ make O=test/xfsprogs/ size-stats
Traceback (most recent call last):
  File "/home/naourr/git/buildroot/support/scripts/size-stats", line 217, in <module>
    pkgdict = build_package_dict(args.builddir)
  File "/home/naourr/git/buildroot/support/scripts/size-stats", line 68, in build_package_dict
    with open(os.path.join(builddir, "build", "packages-file-list.txt")) as filelistf:
IOError: [Errno 2] No such file or directory: 'test/xfsprogs/build/packages-file-list.txt'

Quoted 3 lines

But this file exist:

$ ls test/xfsprogs/build/packages-file-list.txt
test/xfsprogs/build/packages-file-list.txt

Might be that I didn't properly take into account out-of-tree build. My bad, I'll have a look.

Quoted 10 lines

Also I noticed that the man pages (and headers)
are taken into account in packages-file-list.txt:
[...]
tmux,./usr/bin/tmux
tmux,./usr/share/man/man1/tmux.1

Since they are removed at the end, I think we want to remove them from packages-file-list.txt during TARGET_DIR cleanup ?

It does not really matter if there are more files in packages-file-list.txt that there really are in TARGET_DIR, because the logic of the size-stats script is to travel through TARGET_DIR, and for each file in TARGET_DIR, lookup in packages-file-list.txt which package was the last one to install this file. So all files that are in packages-file-list.txt but not in TARGET_DIR are purely and simply ignored.

Thanks for your review and testing!

Thomas