Hi Kamil,

On Thu, Apr 14, 2016 at 8:40 PM, Kamil Paral <kparal@redhat.com> wrote:
Hi Sinny,

I have created a review here:

I used our Phabricator, because it makes it easy to comment on particular lines. You can log in using your FAS email [1] and reply there, or here in the mailing list, it doesn't matter. I'm pleasantly surprised that despite our very lacking documentation, you managed to create a very good Taskotron task :-)

Thanks for the quick review. I have addressed review comments and added new diff at https://phab.qadevel.cloud.fedoraproject.org/D817?id=2081 . I am not sure if I  updated diff in right way but this is what I ended up by following option "Update diff" available in right side :)
One additional thing that I noticed today - when I ran the abipkgdiff comparison on lyx package, it used up to 6 GB RAM. That is really high and I believe our test clients don't have that many available. I don't know how abipkgdiff works and whether it is able to adjust its memory requirements based on the amount of available system memory (so that it would use less memory on a less equipped system and still work properly). Do you know if this is going to be a problem or not? And do you know what is the peak memory usage for very big packages (e.g. what about comparing two kernel packages)? Is there any solution that we could use to limit the memory consumption of abipkgdiff?

I had a talk about this with libabigail's maintainer (Dodji). If a binary is taking more RAM then right now, we can't do anything about it. This is because ELF information for binaries to be compared and corresponding debuginfo gets loaded while performing ABI comparison.  Also, right now libabigail doesn't support comparison of two kernel version.

I noticed that binary files available in lyx package are PIE (Position Independent Executable) files which are basically executable files. As we know, an application mainly consumes shared libraries from its dependent packages. ABI change of shared libraries are what really matter in packages. Therefore, I am now calling  abipkgdiff  with --dso-only  option. This will avoid unnecessary comparison between executable files. This reduces overall comparison time of a koji build update and hopefully reduces memory consumption since most of shared libraries are smaller in size (few MBs).

Right now there is a bug in libabigail that it doesn't distinguish between PIE files and shared libraries since both are of type DSO(Dynamic Shared object). So, right now abipkgdiff with --dso-only option does ABI comparison for PIE files as well. Upstream bug has been reported to distinguish between Shared libraries and PIE files https://sourceware.org/bugzilla/show_bug.cgi?id=19961. This will be fixed soon and will be available in next libabigail release.