Merge ~mdeslaur/ubuntu-cve-tracker:perf-part3 into ubuntu-cve-tracker:master

[Marc Deslauriers]

commit 0606059918cd6e9c9daecf362066377c37324033
Author: Marc Deslauriers <email address hidden>
Date: Mon Mar 4 17:09:01 2024 -0500

    improve performance in source_map.py by skipping package details

    When source_map.py parses the Sources files, it loads a lot of
    details including doing version comparisons. This is expensive and
    isn't needed for active_edit. Add a details parameter to
    load_sources_collection() that skips the extra details.

commit 42d41242937e972e3061d4782130daefe8b840cb
Author: Marc Deslauriers <email address hidden>
Date: Mon Mar 4 17:06:31 2024 -0500

    source_map.py: apt_pkg.TagFile() natively supports gzip files

Skip some unneeded parts of Source files loading to improve performance.

BEFORE:
$ time ./scripts/active_edit -c CVE-2024-NNN1 -p python-django

real 0m4.912s
user 0m5.278s
sys 0m0.459s

AFTER:
$ time ./scripts/active_edit -c CVE-2024-NNN1 -p python-django

real 0m3.411s
user 0m3.279s
sys 0m0.133s

5a6a72e... by Marc Deslauriers on 2024-03-04

source_map.py: add some missing whitespace (cosmetic only)

5a6d47e... by Marc Deslauriers on 2024-03-04

check-syntax: improve performance by skipping certain source details

The only place where version information is required for source
packages is when --strict is used, so when not using that option,
don't load detailed source map information.

[Marc Deslauriers (mdeslaur)]

The last commit shaved a few seconds more off of check-syntax:

BEFORE:
$ time ./scripts/check-syntax -j8

real 0m50.371s
user 5m58.289s
sys 0m6.823s

AFTER:
$ time ./scripts/check-syntax -j8

real 0m45.963s
user 5m24.200s
sys 0m5.888s

[Alex Murray (alexmurray)]

Nice - yep can confirm the improvement in check-syntax too

review: Approve

[Alex Murray (alexmurray)]

I wonder if _apt_get_tags() should decompress the files and leave them uncompressed on disk to save time on subsequent runs? Or whether packages-mirror should just do this itself?

[Mark Esler (eslerm)]

Nice .gz find.

review: Approve

[Marc Deslauriers (mdeslaur)]

> I wonder if _apt_get_tags() should decompress the files and leave them
> uncompressed on disk to save time on subsequent runs? Or whether packages-
> mirror should just do this itself?

That's a good question, I'll have to time that to see if it would save any time.

Unmerged commits

5a6d47e... by Marc Deslauriers on 2024-03-04

check-syntax: improve performance by skipping certain source details

The only place where version information is required for source
packages is when --strict is used, so when not using that option,
don't load detailed source map information.

	unit-tests:0 (build)
	check-cves:0 (build)

1 → 2 of 2 results

First • Previous • Next • Last

5a6a72e... by Marc Deslauriers on 2024-03-04

source_map.py: add some missing whitespace (cosmetic only)

	unit-tests:0 (build)
	check-cves:0 (build)

1 → 2 of 2 results

First • Previous • Next • Last

0606059... by Marc Deslauriers on 2024-03-04

improve performance in source_map.py by skipping package details

When source_map.py parses the Sources files, it loads a lot of
details including doing version comparisons. This is expensive and
isn't needed for active_edit. Add a details parameter to
load_sources_collection() that skips the extra details.

	unit-tests:0 (build)
	check-cves:0 (build)

1 → 2 of 2 results

First • Previous • Next • Last

42d4124... by Marc Deslauriers on 2024-03-04

source_map.py: apt_pkg.TagFile() natively supports gzip files

diff --git a/scripts/active_edit b/scripts/active_edit
index 9a1c18a..321f39a 100755
--- a/scripts/active_edit
+++ b/scripts/active_edit
@@ -42,7 +42,7 @@ source_releases = []
 for release in cve_lib.all_releases:
     if cve_lib.is_active_esm_release(release) or cve_lib.is_active_release(release):
         source_releases.append(release)
-source = source_map.load(releases=source_releases, skip_eol_releases=False)
+source = source_map.load(releases=source_releases, skip_eol_releases=False, detailed=False)
 
 added_rel_pkg = []
 
diff --git a/scripts/check-syntax b/scripts/check-syntax
index 1dd3781..78640b3 100755
--- a/scripts/check-syntax
+++ b/scripts/check-syntax
@@ -30,10 +30,6 @@ import usn_lib
 
 import source_map
 
-source = source_map.load()
-if cve_lib.devel_release:
-    dev_proposed = source_map.load(pockets=["-proposed"], releases=[cve_lib.devel_release])
-
 # add to this list when adding tracking for kernels that have not been
 # published to updates/security yet. Once published, can be removed
 # (script should warn about that). If a kernel is published in one
@@ -285,6 +281,10 @@ parser.add_option(
 # parser.add_option("-c", "--cna", help="Ensure every CVE assigned by Canonical's CNA has an entry", action='store_true')
 (opt, args) = parser.parse_args()
 
+source = source_map.load(detailed=opt.strict)
+if cve_lib.devel_release:
+    dev_proposed = source_map.load(pockets=["-proposed"], releases=[cve_lib.devel_release], detailed=opt.strict)
+
 if opt.debug:
     pp = pprint.PrettyPrinter(indent=4)
 
diff --git a/scripts/source_map.py b/scripts/source_map.py
index 48c22f4..226b92e 100755
--- a/scripts/source_map.py
+++ b/scripts/source_map.py
@@ -212,21 +212,21 @@ def _find_sources_from_apt(pockets=None, releases=None):
 
 
 # release -> pkg -> dict( 'section', 'pocket', 'version' )
-def load(data_type='sources', pockets=None, releases=None, skip_eol_releases=True, arch="amd64"):
+def load(data_type='sources', pockets=None, releases=None, skip_eol_releases=True, arch="amd64", detailed=True):
     if data_type not in ['sources', 'packages']:
         raise ValueError("'data_type' should be either 'sources' or 'packages'")
 
     map = dict()
     if data_type == 'sources':
         for item in _find_sources(pockets=pockets, releases=releases, skip_eol_releases=skip_eol_releases, arch=arch):
-            load_sources_collection(item, map)
+            load_sources_collection(item, map, detailed)
     else:
         for item in _find_packages(pockets=pockets, releases=releases, skip_eol_releases=skip_eol_releases, arch=arch):
             load_packages_collection(item, map)
 
     # subprojects only do sources, not binaries
     if data_type == 'sources':
-        subproject_lists = load_subprojects_lists(releases=releases)
+        subproject_lists = load_subprojects_lists(releases=releases, detailed=detailed)
         for item in subproject_lists:
             map.setdefault(item, dict())
             for pkg in subproject_lists[item]:
@@ -240,9 +240,7 @@ def load(data_type='sources', pockets=None, releases=None, skip_eol_releases=Tru
 
 def _get_apt_tags(tagfile):
     tags = None
-    if tagfile.endswith('.gz'):
-        tags = subprocess.Popen(['/bin/gunzip', '-c', tagfile], stdout=subprocess.PIPE).stdout
-    elif tagfile.endswith('.bz2'):
+    if tagfile.endswith('.bz2'):
         tags = subprocess.Popen(['/bin/bunzip2', '-c', tagfile], stdout=subprocess.PIPE).stdout
     elif tagfile.endswith('.xz'):
         tags = subprocess.Popen(['/usr//bin/xzcat', tagfile], stdout=subprocess.PIPE).stdout
@@ -252,7 +250,7 @@ def _get_apt_tags(tagfile):
     return tags
 
 
-def load_sources_collection(item, map):
+def load_sources_collection(item, map, detailed=True):
     tagfile, release, pocket, section = item
 
     parser = apt_pkg.TagFile(_get_apt_tags(tagfile))
@@ -260,15 +258,15 @@ def load_sources_collection(item, map):
         pkg = parser.section['Package']
         map.setdefault(release, dict()).setdefault(pkg, {'section': 'unset', 'version': '~', 'pocket': 'unset'})
         map[release][pkg]['section'] = section
-        if 'Description' in parser.section:
-            map[release][pkg]['description'] = parser.section['Description']
-        if not pocket:
-            map[release][pkg]['release_version'] = parser.section['Version']
-        if apt_pkg.version_compare(parser.section['Version'], map[release][pkg]['version']) > 0:
-            map[release][pkg]['pocket'] = pocket
-            map[release][pkg]['version'] = parser.section['Version']
-            map[release][pkg]['binaries'] = parser.section['Binary'].split(', ')
-
+        if detailed:
+            if 'Description' in parser.section:
+                map[release][pkg]['description'] = parser.section['Description']
+            if not pocket:
+                map[release][pkg]['release_version'] = parser.section['Version']
+            if apt_pkg.version_compare(parser.section['Version'], map[release][pkg]['version']) > 0:
+                map[release][pkg]['pocket'] = pocket
+                map[release][pkg]['version'] = parser.section['Version']
+                map[release][pkg]['binaries'] = parser.section['Binary'].split(', ')
     return map
 
 
@@ -416,7 +414,7 @@ def get_aliases_of_ubuntu_package(sources, pkg_name, rel):
                 aliases.append(pkg)
     return aliases
 
-def load_subprojects_lists(releases=None):
+def load_subprojects_lists(releases=None, detailed=True):
     map = dict()
 
     if releases is None:
@@ -424,7 +422,7 @@ def load_subprojects_lists(releases=None):
 
     all_sources_esm = {}
     for item in _find_sources(releases=cve_lib.get_active_releases_with_esm(), skip_eol_releases=False):
-        load_sources_collection(item, all_sources_esm)
+        load_sources_collection(item, all_sources_esm, detailed=detailed)
 
     for rel in releases:
         _, _, _, details = cve_lib.get_subproject_details(rel)