size-stats: don't count hard links
PATCH 1/1 size-stats: don't count hard links
This change adds inode tracking to the size-stats script so that hard links don't cause files to be double counted. This has a significant effect on the size computation for some packages. For example, git has around a dozen hard links to a large file. Before this change, git would weigh in at about 170 MB with the total filesystem size reported as 175 MB. The actual rootfs.ext2 size was around 16 MB. With the change, the git package registers at 10.5 MB with a total filesystem size of 15.8 MB.
--- support/scripts/size-stats | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-)
support/scripts/size-stats +22 -5
@@ -41,17 +41,32 @@ colors = ['#e60004', '#009836', '#2e1d86', '#ffed00', # key, and as the value a tuple containing the name of the package to # which the file belongs and the size of the file. # +# Hard links are pruned by tracking inodes so that files are not +# double counted. +# # filesdict: the dict to which the file is added +# seeninodes: the set of inodes that have been seen before # relpath: relative path of the file # fullpath: absolute path to the file # pkg: package to which the file belongs # -def add_file(filesdict, relpath, abspath, pkg): +def add_file(filesdict, seeninodes, relpath, abspath, pkg): if not os.path.exists(abspath): return if os.path.islink(abspath): return - sz = os.stat(abspath).st_size + if relpath in filesdict: + return + + st = os.stat(abspath) + sz = st.st_size + ino = st.st_ino + if ino in seeninodes: + # hard link + sz = 0 + else: + seeninodes.add(ino) + filesdict[relpath] = (pkg, sz) #
@@ -64,13 +79,14 @@ def add_file(filesdict, relpath, abspath, pkg): # def build_package_dict(builddir): filesdict = {} + seeninodes = set() with open(os.path.join(builddir, "build", "packages-file-list.txt")) as filelistf: for l in filelistf.readlines(): pkg, fpath = l.split(",", 1) # remove the initial './' in each file path fpath = fpath.strip()[2:] fullpath = os.path.join(builddir, "target", fpath) - add_file(filesdict, fpath, fullpath, pkg) + add_file(filesdict, seeninodes, fpath, fullpath, pkg) return filesdict #
@@ -97,10 +113,11 @@ def build_package_size(filesdict, builddir): if not frelpath in filesdict: print("WARNING: %s is not part of any package" % frelpath) pkg = "unknown" + sz = os.path.getsize(fpath) else: - pkg = filesdict[frelpath][0] + pkg, sz = filesdict[frelpath] - pkgsize[pkg] += os.path.getsize(fpath) + pkgsize[pkg] += sz return pkgsize
Signature · 1 line
2.7.4
PATCH 1/1 size-stats: don't count hard links
Hello,
Quoted 10 lines · Frank Hunleth
This change adds inode tracking to the size-stats script so that hard links don't cause files to be double counted. This has a significant effect on the size computation for some packages. For example, git has around a dozen hard links to a large file. Before this change, git would weigh in at about 170 MB with the total filesystem size reported as 175 MB. The actual rootfs.ext2 size was around 16 MB. With the change, the git package registers at 10.5 MB with a total filesystem size of 15.8 MB.
Signed-off-by Frank Hunleth <[email protected]>
Thanks a lot for this change! Definitely this is something that needs to be handled.
support/scripts/size-stats colors = ['#e60004', '#009836', '#2e1d86', '#ffed00', · line 59
Quoted 9 lines
support/scripts/size-stats +3
-2-def add_file(filesdict, relpath, abspath, pkg): +def add_file(filesdict, seeninodes, relpath, abspath, pkg): if not os.path.exists(abspath): return if os.path.islink(abspath): return - sz = os.stat(abspath).st_size + if relpath in filesdict: + return
I'm not sure why this test is being added, or at least why it's related to the inode tracking.
support/scripts/size-stats def build_package_size(filesdict, builddir): · line 116
Quoted 5 lines
support/scripts/size-stats +1
-0@@ -97,10 +113,11 @@ def build_package_size(filesdict, builddir): if not frelpath in filesdict: print("WARNING: %s is not part of any package" % frelpath) pkg = "unknown" + sz = os.path.getsize(fpath)
So for files not belonging to packages, we do not track inodes?
Maybe we should instead have our own filesize() helper function that takes care of returning the right size if we have never seen this inode, or 0 if we have already seen it. It could then be used in both places.
Another concern is that some files will now be reported as having a 0 size, while it's not entirely correct. This does not matter at all for the per-package graph or CSV file, but is a bit more annoying for the per-file CSV file. Indeed, a user inspecting this CSV file will wonder what those zero-size files are. So, another option is to divide the size of the file by the number of hard-links, and spread the size over the different hard-links. But it's also not very nice, as the size reported in the CSV will not match the visible size of the file.
So, maybe we should just leave it like you propose, unless others have a better idea about this.
Thanks!
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
PATCH 1/1 size-stats: don't count hard links
Quoted 17 lines · Thomas Petazzoni
Hello,
Quoted 10 lines · Frank Hunleth
This change adds inode tracking to the size-stats script so that hard links don't cause files to be double counted. This has a significant effect on the size computation for some packages. For example, git has around a dozen hard links to a large file. Before this change, git would weigh in at about 170 MB with the total filesystem size reported as 175 MB. The actual rootfs.ext2 size was around 16 MB. With the change, the git package registers at 10.5 MB with a total filesystem size of 15.8 MB.
Signed-off-by Frank Hunleth <[email protected]>Thanks a lot for this change! Definitely this is something that needs to be handled.
Quoted 12 lines
support/scripts/size-stats colors = ['#e60004', '#009836', '#2e1d86', '#ffed00', · line 59
Quoted 9 lines
support/scripts/size-stats +3
-2-def add_file(filesdict, relpath, abspath, pkg): +def add_file(filesdict, seeninodes, relpath, abspath, pkg): if not os.path.exists(abspath): return if os.path.islink(abspath): return - sz = os.stat(abspath).st_size + if relpath in filesdict: + returnI'm not sure why this test is being added, or at least why it's related to the inode tracking.
I find that I often have duplicate files listed in package-file-list.txt. I think that they're all due to rebuilding packages. Without the hard link check, the duplicate files were harmless in that they just resulted in a file being added to filesdict more than once. With the hard link checking, an if statement was needed to make sure that a file with a previously seen inode wouldn't mistakenly be added as a hard link. Checking up front here seemed best since it not only address the hard link case, but also saved a little time handling duplicate entries.
Quoted 8 lines
support/scripts/size-stats def build_package_size(filesdict, builddir): · line 116
Quoted 6 lines
support/scripts/size-stats +1
-0@@ -97,10 +113,11 @@ def build_package_size(filesdict, builddir): if not frelpath in filesdict: print("WARNING: %s is not part of any package" % frelpath) pkg = "unknown" + sz = os.path.getsize(fpath)So for files not belonging to packages, we do not track inodes?
Good point. I hadn't considered that anyone would use hard links outside of packages, but I suppose there's nothing preventing it.
Quoted 15 lines
Maybe we should instead have our own
filesize()helper function that takes care of returning the right size if we have never seen this inode, or 0 if we have already seen it. It could then be used in both places.Another concern is that some files will now be reported as having a 0 size, while it's not entirely correct. This does not matter at all for the per-package graph or CSV file, but is a bit more annoying for the per-file CSV file. Indeed, a user inspecting this CSV file will wonder what those zero-size files are. So, another option is to divide the size of the file by the number of hard-links, and spread the size over the different hard-links. But it's also not very nice, as the size reported in the CSV will not match the visible size of the file.
Quoted 2 lines
So, maybe we should just leave it like you propose, unless others have a better idea about this.
I had similar concerns about reporting 0 sized files, since it's not that obvious and which file gets picked as the non-zero length file is arbitrary. I hadn't thought of spreading the file size across all links, but I'm not sure that would make things more clear. The only idea I have is to add a comments field to the csv, add the file as 0 length, and have a comment "hard link to xyz." I pretty much only use the package size stats, so I'll wait around for other suggestions too.
Frank
Quoted 8 lines
Thanks!
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com