Skip to content

Performance: Hoist regexp.MustCompile out of functions - #29511

Merged
Luap99 merged 1 commit into
podman-container-tools:mainfrom
vishnukothakapu:perf-regexp-mustcompile
Aug 17, 2026
Merged

Performance: Hoist regexp.MustCompile out of functions#29511
Luap99 merged 1 commit into
podman-container-tools:mainfrom
vishnukothakapu:perf-regexp-mustcompile

Conversation

@vishnukothakapu

@vishnukothakapu vishnukothakapu commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Replaces regexp.MustCompile with regexp.Delayed from go.podman.io/storage/pkg/regexp for global regular expressions. This avoids compiling regular expressions during the global init scope, improving startup performance for all commands.
Additionally, where possible (such as in quadlet, mac-helper, and ps), the regex engine was stripped entirely and replaced with faster strings and loop checks to avoid any regex overhead altogether.
Tests are kept using regexp.MustCompile where applicable.

Fixes #29510

Checklist

Ensure you have completed the following checklist for your pull request to be reviewed:

  • I have read and understood our contributing guidelines and will not have more than two open PRs as a new contributor.
  • PR description, commit message, and GitHub comments are human-written, per LLM Policy
  • Certify you wrote the patch or otherwise have the right to pass it on as an open-source patch by signing all
    commits. (git commit -s). (If needed, use git commit -s --amend). The author email must match
    the sign-off email address. See CONTRIBUTING.md
    for more information.
  • Referenced issues using Fixes: #29510 in commit message (if applicable)
  • Tests have been added/updated (or no tests are needed)
  • Documentation has been updated (or no documentation changes are needed)
  • All commits pass make validatepr (format/lint checks)
  • Release note entered in the section below (or None if no user-facing changes)

Does this PR introduce a user-facing change?

None

Comment thread cmd/podman/inspect/inspect.go Outdated
Comment thread cmd/podman/images/trust_set.go
@vishnukothakapu
vishnukothakapu force-pushed the perf-regexp-mustcompile branch from c6db1ad to d7f5f7e Compare August 13, 2026 19:30
@vishnukothakapu
vishnukothakapu requested a review from mheon August 13, 2026 19:30
@mheon mheon added the No New Tests Allow PR to proceed without adding regression tests label Aug 13, 2026

@mheon mheon left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@Luap99 Luap99 left a comment

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.

Performance wise that actually has the opposite effect for most commands.
Compiling tje regex in the global init scope means every signle command must compile them over and over again, given podman is not a daemon and we run commands over and over again. Most code paths will never need the regex that was compiled so this is wasted time.

This was so bad we had to add our own go.podman.io/storage/pkg/regexp package to delay regex compilation until it is actually needed.
All user must use this, see c2b36be

@vishnukothakapu
vishnukothakapu force-pushed the perf-regexp-mustcompile branch 3 times, most recently from 5bb5eae to f02ab1a Compare August 14, 2026 10:11
@packit-as-a-service

Copy link
Copy Markdown

[NON-BLOCKING] Packit jobs failed. @podman-container-tools/packit-jobs please check. Everyone else, feel free to ignore.

@vishnukothakapu
vishnukothakapu requested a review from Luap99 August 14, 2026 10:12

@Honny1 Honny1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I have three things:

  • is posbile to replace with a simple regex with different checks?
  • Please put in the commit msg Fixes: #numOfIssueThatFixesThisCommit
  • Also update the description, commit msg, etc...

@vishnukothakapu
vishnukothakapu force-pushed the perf-regexp-mustcompile branch 3 times, most recently from a853cb4 to a63f581 Compare August 14, 2026 14:24
@vishnukothakapu

Copy link
Copy Markdown
Contributor Author
  • is posbile to replace with a simple regex with different checks?

Yes @Honny1 , that's a great point. I went ahead and replaced the simple regexes in pkg/systemd/quadlet/unitdirs.go, cmd/podman-mac-helper/main.go, and pkg/ps/ps.go entirely with standard strings and loop checks. This avoids regex compilation overhead altogether in those paths. The more complex regex patterns have been kept as regexp.Delayed.

@Honny1

Honny1 commented Aug 14, 2026

Copy link
Copy Markdown
Contributor
Error: pkg/systemd/quadlet/unitdirs.go:226:10: undefined: numericRegex
make: *** [Makefile:525: bin/podman.cross.linux.amd64] Error 1

@Honny1 Honny1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I spotted only one issue. Otherwise, we are on good way.

Comment thread pkg/systemd/quadlet/unitdirs.go Outdated
Comment thread pkg/systemd/quadlet/unitdirs.go
Replaces regexp.MustCompile with regexp.Delayed from go.podman.io/storage/pkg/regexp
for global regular expressions. This avoids compiling regular expressions
during the global init scope, improving startup performance for all commands.

Tests are kept using regexp.MustCompile where applicable.

Fixes: podman-container-tools#29510
Signed-off-by: Vishnu Kothakapu <vishnukothakapu27@gmail.com>

@Luap99 Luap99 left a comment

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.

LGTM

@Honny1 Honny1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@Luap99
Luap99 enabled auto-merge August 17, 2026 10:48
@Luap99
Luap99 merged commit 5b366f4 into podman-container-tools:main Aug 17, 2026
127 of 129 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

machine No New Tests Allow PR to proceed without adding regression tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Performance: Move regexp.MustCompile out of function bodies to package level

4 participants