From 1bc221a9dd22baa6e2e516d45dc858774cafddd0 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Thu, 30 Jan 2025 16:39:40 +0000 Subject: Drop unused description size info from binary cache MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Nothing in src:apt uses this information and that isn't all to surprising as it is usually not that interesting to know how big a specific stanza of a description is as the max size is available and you are usually working with more than one description. So this information just takes up space in our binary cache… 64 bits per structure which can quickly add up to whole MBs depending on how many (and which) sources you have enabled. --- apt-pkg/pkgcachegen.cc | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) (limited to 'apt-pkg/pkgcachegen.cc') diff --git a/apt-pkg/pkgcachegen.cc b/apt-pkg/pkgcachegen.cc index aaa969a8d..76dea6266 100644 --- a/apt-pkg/pkgcachegen.cc +++ b/apt-pkg/pkgcachegen.cc @@ -949,9 +949,14 @@ bool pkgCacheGenerator::NewFileDesc(pkgCache::DescIterator &Desc, *Last = DF.MapPointer(); DF->Offset = List.Offset(); - DF->Size = List.Size(); - if (Cache.HeaderP->MaxDescFileSize < DF->Size) - Cache.HeaderP->MaxDescFileSize = DF->Size; + auto const Size = List.Size(); + if (Cache.HeaderP->MaxDescFileSize < Size) + Cache.HeaderP->MaxDescFileSize = Size; +#if APT_PKG_ABI <= 600 + APT_IGNORE_DEPRECATED_PUSH + DF->Size = Size; + APT_IGNORE_DEPRECATED_POP +#endif Cache.HeaderP->DescFileCount++; return true; -- cgit v1.2.3-70-g09d2 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. --- apt-pkg/pkgcache.h | 5 +++-- apt-pkg/pkgcachegen.cc | 11 ++++++++--- cmdline/apt-cache.cc | 50 +++++++++++++++++++------------------------------- 3 files changed, 30 insertions(+), 36 deletions(-) (limited to 'apt-pkg/pkgcachegen.cc') diff --git a/apt-pkg/pkgcache.h b/apt-pkg/pkgcache.h index d5d8fcad3..0b61b0048 100644 --- a/apt-pkg/pkgcache.h +++ b/apt-pkg/pkgcache.h @@ -604,8 +604,9 @@ struct pkgCache::VerFile map_pointer NextFile; /** \brief position in the package file */ map_filesize_t Offset; // File offset - /** @TODO document pkgCache::VerFile::Size */ - map_filesize_t Size; +#if APT_PKG_ABI <= 600 + [[deprecated("No usage in src:apt, try MaxVerFileSize instead")]] map_filesize_t Size; +#endif }; /*}}}*/ // DescFile structure /*{{{*/ diff --git a/apt-pkg/pkgcachegen.cc b/apt-pkg/pkgcachegen.cc index 76dea6266..055b2e08e 100644 --- a/apt-pkg/pkgcachegen.cc +++ b/apt-pkg/pkgcachegen.cc @@ -854,9 +854,14 @@ bool pkgCacheGenerator::NewFileVer(pkgCache::VerIterator &Ver, *Last = VF.MapPointer(); VF->Offset = List.Offset(); - VF->Size = List.Size(); - if (Cache.HeaderP->MaxVerFileSize < VF->Size) - Cache.HeaderP->MaxVerFileSize = VF->Size; + auto const Size = List.Size(); + if (Cache.HeaderP->MaxVerFileSize < Size) + Cache.HeaderP->MaxVerFileSize = Size; +#if APT_PKG_ABI <= 600 + APT_IGNORE_DEPRECATED_PUSH + VF->Size = Size; + APT_IGNORE_DEPRECATED_POP +#endif Cache.HeaderP->VerFileCount++; return true; 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 d0c2cc03d75fa66986a9aadf8b1361426e9ceb4d Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Thu, 30 Jan 2025 21:45:40 +0000 Subject: Do not create descriptions structs for each architecture The architectures are supposed to share the descriptions as they are (very usually) the same, but the code was happily creating structs for every architecture of a package which is especially pointless as for e.g. Debian those will be the short description likely superseded in output and all uses by the 'en' "translation". --- apt-pkg/pkgcachegen.cc | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) (limited to 'apt-pkg/pkgcachegen.cc') diff --git a/apt-pkg/pkgcachegen.cc b/apt-pkg/pkgcachegen.cc index 055b2e08e..de04d1313 100644 --- a/apt-pkg/pkgcachegen.cc +++ b/apt-pkg/pkgcachegen.cc @@ -513,23 +513,26 @@ bool pkgCacheGenerator::MergeListVersion(ListParser &List, pkgCache::PkgIterator a version with a description of the same MD5 - if so we reuse this description group instead of creating our own for this version */ for (pkgCache::PkgIterator P = Grp.PackageList(); - P.end() == false; P = Grp.NextPkg(P)) + not P.end() && Ver->DescriptionList == 0; P = Grp.NextPkg(P)) { - for (pkgCache::VerIterator V = P.VersionList(); - V.end() == false; ++V) + for (pkgCache::VerIterator V = P.VersionList(); not V.end(); ++V) { if (V->DescriptionList == 0 || Cache.ViewString(V.DescriptionList()->md5sum) != CurMd5) continue; Ver->DescriptionList = V->DescriptionList; + break; } } // We haven't found reusable descriptions, so add the first description(s) map_stringitem_t md5idx = Ver->DescriptionList == 0 ? 0 : Ver.DescriptionList()->md5sum; - std::vector availDesc = List.AvailableDescriptionLanguages(); - for (std::vector::const_iterator CurLang = availDesc.begin(); CurLang != availDesc.end(); ++CurLang) - if (AddNewDescription(List, Ver, *CurLang, CurMd5, md5idx) == false) + 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; } /*}}}*/ -- 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 'apt-pkg/pkgcachegen.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