Skip to content

[cleaner] Break loop over compiled_regexes in case of no match#4267

Merged
pmoravec merged 1 commit intososreport:mainfrom
pmoravec:sos-pmoravec-cleaner-break-compiled-search-cycle
Apr 10, 2026
Merged

[cleaner] Break loop over compiled_regexes in case of no match#4267
pmoravec merged 1 commit intososreport:mainfrom
pmoravec:sos-pmoravec-cleaner-break-compiled-search-cycle

Conversation

@pmoravec
Copy link
Copy Markdown
Contributor

@pmoravec pmoravec commented Mar 9, 2026

A heuristic improving performance by several additional percents. Since a line usually contains at most a few matches only, it pays off to search again for one compiled_search rather than tens/hundreds of individual compiled_regexes searches.

Closes: #4267
Relevant: #4227


Please place an 'X' inside each '[]' to confirm you adhere to our Contributor Guidelines

  • Is the commit message split over multiple lines and hard-wrapped at 72 characters?
  • Is the subject and message clear and concise?
  • Does the subject start with [plugin_name] if submitting a plugin patch or a [section_name] if part of the core sosreport code?
  • Does the commit contain a Signed-off-by: First Lastname email@example.com?
  • Are any related Issues or existing PRs properly referenced via a Closes (Issue) or Resolved (PR) line?
  • Are all passwords or private data gathered by this PR obfuscated?

A heuristic improving performance by several additional percents. Since
a line usually contains at most a few matches only, it pays off to
search again for one compiled_search rather than tens/hundreds of
individual compiled_regexes searches.

Closes: sosreport#4267
Relevant: sosreport#4227

Signed-off-by: Pavel Moravec <pmoravec@redhat.com>
@pmoravec pmoravec force-pushed the sos-pmoravec-cleaner-break-compiled-search-cycle branch from 65fb0b6 to 4481464 Compare March 9, 2026 21:28
@packit-as-a-service
Copy link
Copy Markdown

Congratulations! One of the builds has completed. 🍾

You can install the built RPMs by following these steps:

  • sudo dnf install -y 'dnf*-command(copr)'
  • dnf copr enable packit/sosreport-sos-4267
  • And now you can install the packages.

Please note that the RPMs should be used only in a testing environment.

@pmoravec
Copy link
Copy Markdown
Contributor Author

pmoravec commented Apr 1, 2026

Kindly asking @TurboTurtle and/or @arif-ali for a review.

count += _count
# break the cycle if no further search can apply
if not self.mapping.compiled_search.search(line):
break
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think it would be worth adding some logging here? Perhaps at debug level, so it's not printed every time. The idea is that we avoid situations where we silently break the cycle and then we need to investigate an issue where that shouldn't have happened

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than that, LGTM :)

@jcastill jcastill added Kind/cleaner cleaner component of sos Status/Needs Review This issue still needs a review from project members labels Apr 9, 2026
Copy link
Copy Markdown
Member

@TurboTurtle TurboTurtle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@TurboTurtle TurboTurtle added Kind/Enhancement Reviewed/Ready for Merge Has been reviewed, ready for merge and removed Status/Needs Review This issue still needs a review from project members labels Apr 9, 2026
@pmoravec pmoravec merged commit cffe8e3 into sosreport:main Apr 10, 2026
44 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Kind/cleaner cleaner component of sos Kind/Enhancement Reviewed/Ready for Merge Has been reviewed, ready for merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants