Hi, On Fri, 05 Dec 2008, Modestas Vainius wrote: > All 3 issues resolved. Patch attached (against lenny branch, applies to > master > without problems). I have been using this patched dpkg quite heavily lately, > no problems noticed.
Cool, thanks. > This patch optimizes dpkg-shlibdeps by caching parsed symbols files and > objdump objects. This way neither of the libraries or symbols files are > parsed more than once. This patch significantly improves performance of > dpkg-shlibdeps bringing it near to performance levels of << 1.14.8 > dpkg-shlibdeps without loosing any of new functionally at all. Memory > requirements are reduced too. Next time, please try to follow the recommendations described on the wiki for writing log messages: http://wiki.debian.org/Teams/Dpkg/GitUsage > +sub merge_from_symfile { > + my ($self, $src) = @_; > + while (($soname, $srcobj) = each(%{$src->{objects}})) { > + if (exists $self->{objects}{$soname}) { > + # Update/override infos only > + $self->{objects}{$soname}{deps} = $srcobj->{deps}; I don't think it makes sense to update the dependency if we already have the object in the symfile. If the library is described in 2 symbols files, either it's wrong or one of the two should take precedence, using the symbol list of one and the dependencies of the second seems wrong. I don't think that we will encounter such cases in practice, hence I would suggest to just display a warning stating that we have two conflicting objects for the same library. Or it means that we're trying to merge several times the same symfile. > + } else { > + # Shallow copy the soname object (because deps can be replaced > later) > + my %obj; > + while (($key, $val) = each(%$srcobj)) { > + $obj{$key} = $val; > + } > + $self->{objects}{$soname} = \%obj; > + } > + } Taking into account the previous comment, this "shallow copy" is probably not needed. Otherwise, if it's a matter of API-cleanliness, then we should rather do a deep copy to ensure we don't share anything with the merged object. Using dclone() of Storable would be appopriate in that case. I'm not sure if it represents a big run-time cost or not. > - if (defined $dpkg_symfile) { > - # Load symbol information > - print "Using symbols file $dpkg_symfile for $soname\n" if > $debug; > - $symfile->load($dpkg_symfile); > + if (defined($dpkg_symfile)) { > + if (exists $dpkg_symfile_cache{$dpkg_symfile}) { > + print "Using symbols file $dpkg_symfile (cached) for > $soname\n" if $debug; > + } else { > + # Load symbol information > + print "Using symbols file $dpkg_symfile for $soname\n" > if $debug; > + $dpkg_symfile_cache{$dpkg_symfile} = new > Dpkg::Shlibs::SymbolFile(); > + $dpkg_symfile_cache{$dpkg_symfile}->load($dpkg_symfile); > + } > + > $symfile->merge_from_symfile($dpkg_symfile_cache{$dpkg_symfile}); There's a possibility we're going to merge the same file multiple times but it's not big deal (except if we display an ugly warning). To avoid doing too much we could however restrict the merge to only what is of interest to us: the infos describing the library of soname $soname. > } > } > if (defined($dpkg_symfile) && $symfile->has_object($soname)) { > @@ -214,13 +225,25 @@ foreach my $file (keys %exec) { > } > } else { > # No symbol file found, fall back to standard shlibs > - my $id = $dumplibs_wo_symfile->parse($lib); > + my $id; > + my $libobj; > + if (exists $dpkg_objdump_cache{$lib}) { > + $libobj = $dpkg_objdump_cache{$lib}; > + # We don't want to process the same lib more than once > (redundant) > + next if > ($dumplibs_wo_symfile->get_object($libobj->get_id())); Looks like an has_object() would be welcome for consistency with the SymbolFile object. > + $id = > $dumplibs_wo_symfile->add_object($dpkg_objdump_cache{$lib}); $dpkg_objdump_cache{$lib} => $libobj > sub symfile_has_soname { > my ($file, $soname) = @_; > + > + return 1 if (exists $symfile_has_soname_cache{$file}{$soname}); > + > open(SYM_FILE, "<", $file) || > syserr(_g("cannot open file %s"), $file); > my $result = 0; > while (<SYM_FILE>) { > if (/^\Q$soname\E /) { > $result = 1; > + $symfile_has_soname_cache{$file}{$soname} = 1; > last; > } > } Why are you not caching a negative result as well? it's not more complicated… Otherwise it's fine and I will happily apply it to the master branch. Tell me if you plan to submit an updated patch or if I should finish the work myself. Cheers, -- Raphaël Hertzog Le best-seller français mis à jour pour Debian Etch : http://www.ouaza.com/livre/admin-debian/ -- To UNSUBSCRIBE, email to [EMAIL PROTECTED] with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]
