From 1bc8dab8df60c5da7cc817bec5f53e60f7eb9817 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Thu, 30 Jan 2025 18:03:26 +0000 Subject: Drop unused version stanza size info from binary cache Same reason as with the description size before, but this one had actually one extremely dated user in src:apt left with dumpavail, which is changed to an implementation that is (in comparison) a bit slower, but should be good enough for a command that is ideally not used anymore (even if codesearch suggests otherwise). After all, what makes this a bit slower is not that we don't know the size, but that this one does additional work preparing for accessing the fields which we don't do. In exchange pkgTagFile has a better implementation for "Jitter" than the old code that we would "just" need to copy here if speed were really an issue. --- cmdline/apt-cache.cc | 50 +++++++++++++++++++------------------------------- 1 file changed, 19 insertions(+), 31 deletions(-) (limited to 'cmdline/apt-cache.cc') diff --git a/cmdline/apt-cache.cc b/cmdline/apt-cache.cc index 7fdd4dff2..4ad0b0099 100644 --- a/cmdline/apt-cache.cc +++ b/cmdline/apt-cache.cc @@ -464,7 +464,6 @@ static bool DumpAvail(CommandLine &) stdoutfd.OpenDescriptor(STDOUT_FILENO, FileFd::WriteOnly, false); // Iterate over all the package files and write them out. - char *Buffer = new char[Cache->HeaderP->MaxVerFileSize+10]; for (pkgCache::VerFile **J = VFList; *J != 0;) { pkgCache::PkgFileIterator File(*Cache, Cache->PkgFileP + (*J)->File); @@ -472,40 +471,23 @@ static bool DumpAvail(CommandLine &) FileFd PkgF(File.FileName(),FileFd::ReadOnly, FileFd::Extension); if (_error->PendingError() == true) break; - - /* Write all of the records from this package file, since we - already did locality sorting we can now just seek through the - file in read order. We apply 1 more optimization here, since often - there will be < 1 byte gaps between records (for the \n) we read that - into the next buffer and offset a bit.. */ - unsigned long Pos = 0; + + pkgTagFile tagsfile(&PkgF, Cache->HeaderP->MaxVerFileSize); + pkgTagSection Tags; + for (; *J != 0; J++) { if (Cache->PkgFileP + (*J)->File != File) break; - - const pkgCache::VerFile &VF = **J; - // Read the record and then write it out again. - unsigned long Jitter = VF.Offset - Pos; - if (Jitter > 8) - { - if (PkgF.Seek(VF.Offset) == false) - break; - Jitter = 0; - } - - if (PkgF.Read(Buffer,VF.Size + Jitter) == false) + const pkgCache::VerFile &VF = **J; + if (not tagsfile.Jump(Tags, VF.Offset)) break; - Buffer[VF.Size + Jitter] = '\n'; - // See above.. if ((File->Flags & pkgCache::Flag::NotSource) == pkgCache::Flag::NotSource) { - pkgTagSection Tags; - if (Tags.Scan(Buffer+Jitter,VF.Size+1) == false || - Tags.Write(stdoutfd, NULL, RW) == false || - stdoutfd.Write("\n", 1) == false) + if (not Tags.Write(stdoutfd, NULL, RW) || + not stdoutfd.Write("\n", 1)) { _error->Error("Internal Error, Unable to parse a package record"); break; @@ -513,18 +495,24 @@ static bool DumpAvail(CommandLine &) } else { - if (stdoutfd.Write(Buffer + Jitter, VF.Size + 1) == false) + char const *Start, *Stop; + Tags.GetSection(Start, Stop); + for (; Start < Stop; --Stop) + if (std::string_view{"\n\r"}.find(*(Stop - 1)) == std::string_view::npos) + break; + if (not stdoutfd.Write(Start, Stop - Start) || + not stdoutfd.Write("\n\n", 2)) + { + _error->Error("Internal Error, Unable to parse a package record"); break; + } } - - Pos = VF.Offset + VF.Size; } - if (_error->PendingError() == true) + if (_error->PendingError()) break; } - delete [] Buffer; delete [] VFList; return !_error->PendingError(); } -- cgit v1.2.3-70-g09d2 From 593cf30cee476bdae21f7e99f67d2572b7381e90 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Thu, 30 Jan 2025 23:10:02 +0000 Subject: Record all available sources for descriptions Avoiding duplications is one thing, but it could also be handy to know all sources for a description so that you can e.g. avoid opening another file or similar such if its also present in one you already have open. Given how the structures were set up this was probably intended all along and either worked at some point or was abandoned for the duplicates that we started culling heavily with the introduction of MultiArch as at that point they blew up the cache and needed to be dealt with. --- apt-pkg/pkgcachegen.cc | 108 +++++++++++++++++++++++++------------------------ cmdline/apt-cache.cc | 7 ++-- 2 files changed, 59 insertions(+), 56 deletions(-) (limited to 'cmdline/apt-cache.cc') diff --git a/apt-pkg/pkgcachegen.cc b/apt-pkg/pkgcachegen.cc index de04d1313..dd79967c0 100644 --- a/apt-pkg/pkgcachegen.cc +++ b/apt-pkg/pkgcachegen.cc @@ -45,9 +45,6 @@ template using Dynamic = pkgCacheGenerator::Dynamic; typedef std::vector::iterator FileIterator; template std::vector pkgCacheGenerator::Dynamic::toReMap; -static bool IsDuplicateDescription(pkgCache &Cache, pkgCache::DescIterator Desc, - std::string_view CurMd5, std::string const &CurLang); - using std::string; using std::string_view; @@ -351,14 +348,10 @@ bool pkgCacheGenerator::MergeListPackage(ListParser &List, pkgCache::PkgIterator continue; map_stringitem_t md5idx = VerDesc->md5sum; - for (std::vector::const_iterator CurLang = availDesc.begin(); CurLang != availDesc.end(); ++CurLang) + for (auto const &CurLang : availDesc) { - // don't add a new description if we have one for the given - // md5 && language - if (IsDuplicateDescription(Cache, VerDesc, CurMd5, *CurLang) == true) - continue; - - AddNewDescription(List, Ver, *CurLang, CurMd5, md5idx); + if (not AddNewDescription(List, Ver, CurLang, CurMd5, md5idx)) + return false; } // we can stop here as all "same" versions will share the description @@ -524,39 +517,55 @@ bool pkgCacheGenerator::MergeListVersion(ListParser &List, pkgCache::PkgIterator } } - // We haven't found reusable descriptions, so add the first description(s) map_stringitem_t md5idx = Ver->DescriptionList == 0 ? 0 : Ver.DescriptionList()->md5sum; for (auto const &CurLang : List.AvailableDescriptionLanguages()) - { - if (Ver->DescriptionList != 0 && IsDuplicateDescription(Cache, Ver.DescriptionList(), CurMd5, CurLang)) - continue; if (not AddNewDescription(List, Ver, CurLang, CurMd5, md5idx)) return false; - } return true; } /*}}}*/ -bool pkgCacheGenerator::AddNewDescription(ListParser &List, pkgCache::VerIterator &Ver, std::string const &lang, std::string_view CurMd5, map_stringitem_t &md5idx) /*{{{*/ +// findDescription /*{{{*/ +static bool findDescription(pkgCache &Cache, pkgCache::DescIterator &Desc, + std::string_view CurMd5, std::string_view const CurLang) +{ + // Descriptions in the same link-list have all the same md5 + if (Desc.end() || Cache.ViewString(Desc->md5sum) != CurMd5) + return false; + for (; not Desc.end(); ++Desc) + if (CurLang == Cache.ViewString(Desc->language_code)) + return true; + return false; +} + /*}}}*/ +bool pkgCacheGenerator::AddNewDescription(ListParser &List, pkgCache::VerIterator &Ver, std::string const &CurLang, std::string_view CurMd5, map_stringitem_t &md5idx) /*{{{*/ { - pkgCache::DescIterator Desc; + pkgCache::DescIterator Desc = Ver.DescriptionList(); Dynamic DynDesc(Desc); - map_pointer const descindex = NewDescription(Desc, lang, CurMd5, md5idx); - if (unlikely(descindex == 0)) - return _error->Error(_("Error occurred while processing %s (%s%d)"), - Ver.ParentPkg().Name(), "NewDescription", 1); + // don't add a new description if we have one for the given md5 && language + if (not findDescription(Cache, Desc, CurMd5, CurLang)) + { + map_pointer const descindex = NewDescription(Desc, CurLang, CurMd5, md5idx); + if (unlikely(descindex == 0)) + return _error->Error(_("Error occurred while processing %s (%s%d)"), + Ver.ParentPkg().Name(), "NewDescription", 1); - md5idx = Desc->md5sum; - Desc->ParentPkg = Ver.ParentPkg().MapPointer(); + md5idx = Desc->md5sum; + Desc->ParentPkg = Ver.ParentPkg().MapPointer(); - // we add at the end, so that the start is constant as we need - // that to be able to efficiently share these lists - pkgCache::DescIterator VerDesc = Ver.DescriptionList(); // old value might be invalid after ReMap - for (;VerDesc.end() == false && VerDesc->NextDesc != 0; ++VerDesc); - map_pointer * const LastNextDesc = (VerDesc.end() == true) ? &Ver->DescriptionList : &VerDesc->NextDesc; - *LastNextDesc = descindex; + // we add at the end, so that the start is constant as we need + // that to be able to efficiently share these lists + if (Ver->DescriptionList == 0) + Ver->DescriptionList = descindex; + else + { + auto VerDesc = Ver.DescriptionList(); + for (; VerDesc->NextDesc != 0; ++VerDesc); + VerDesc->NextDesc = descindex; + } + } - if (NewFileDesc(Desc,List) == false) + if (not NewFileDesc(Desc, List)) return _error->Error(_("Error occurred while processing %s (%s%d)"), Ver.ParentPkg().Name(), "NewFileDesc", 1); @@ -932,31 +941,37 @@ map_pointer pkgCacheGenerator::NewVersion(pkgCache::VerIterat } /*}}}*/ // CacheGenerator::NewFileDesc - Create a new File<->Desc association /*{{{*/ -// --------------------------------------------------------------------- -/* */ bool pkgCacheGenerator::NewFileDesc(pkgCache::DescIterator &Desc, ListParser &List) { if (CurrentFile == nullptr) return true; + auto const DFFile = map_pointer{NarrowOffset(CurrentFile - Cache.PkgFileP)}; + auto const DFOffset = List.Offset(); + for (auto DF = Desc.FileList(); not DF.end(); ++DF) + if (DF->File == DFFile && DF->Offset == DFOffset) + return true; + // Get a structure auto const DescFile = AllocateInMap(); if (DescFile == 0) return false; pkgCache::DescFileIterator DF(Cache,Cache.DescFileP + DescFile); - DF->File = map_pointer{NarrowOffset(CurrentFile - Cache.PkgFileP)}; + DF->File = DFFile; + DF->Offset = DFOffset; // Link it to the end of the list - map_pointer *Last = &Desc->FileList; - for (pkgCache::DescFileIterator D = Desc.FileList(); D.end() == false; ++D) - Last = &D->NextFile; - - DF->NextFile = *Last; - *Last = DF.MapPointer(); + if (Desc->FileList == 0) + Desc->FileList = DescFile; + else + { + auto Last = Desc.FileList(); + for (; Last->NextFile != 0; ++Last); + Last->NextFile = DescFile; + } - DF->Offset = List.Offset(); auto const Size = List.Size(); if (Cache.HeaderP->MaxDescFileSize < Size) Cache.HeaderP->MaxDescFileSize = Size; @@ -1886,19 +1901,6 @@ bool pkgCacheGenerator::MakeOnlyStatusCache(OpProgress *Progress,DynamicMMap **O return true; } /*}}}*/ -// IsDuplicateDescription /*{{{*/ -static bool IsDuplicateDescription(pkgCache &Cache, pkgCache::DescIterator Desc, - std::string_view CurMd5, std::string const &CurLang) -{ - // Descriptions in the same link-list have all the same md5 - if (Desc.end() == true || Cache.ViewString(Desc->md5sum) != CurMd5) - return false; - for (; Desc.end() == false; ++Desc) - if (Desc.LanguageCode() == CurLang) - return true; - return false; -} - /*}}}*/ pkgCacheListParser::pkgCacheListParser() : Owner(NULL), OldDepLast(NULL), d(NULL) {} pkgCacheListParser::~pkgCacheListParser() {} diff --git a/cmdline/apt-cache.cc b/cmdline/apt-cache.cc index 4ad0b0099..67dde67a6 100644 --- a/cmdline/apt-cache.cc +++ b/cmdline/apt-cache.cc @@ -87,9 +87,10 @@ static bool DumpPackage(CommandLine &CmdL) cout << endl; for (pkgCache::DescIterator D = Cur.DescriptionList(); D.end() == false; ++D) { - cout << " Description Language: " << D.LanguageCode() << endl - << " File: " << D.FileList().File().FileName() << endl - << " MD5: " << D.md5() << endl; + cout << " Description Language: " << D.LanguageCode() << '\n'; + for (auto DF = D.FileList(); not DF.end(); ++DF) + cout << " File: " << DF.File().FileName() << '\n'; + cout << " MD5: " << D.md5() << '\n'; } cout << endl; } -- cgit v1.2.3-70-g09d2