Skip to content

⚡ Bolt: Optimize Proxmox inventory regex filtering with cache - #910

Open
adolago wants to merge 1 commit into
mainfrom
bolt-proxmox-regex-cache-6597655516682391507
Open

⚡ Bolt: Optimize Proxmox inventory regex filtering with cache#910
adolago wants to merge 1 commit into
mainfrom
bolt-proxmox-regex-cache-6597655516682391507

Conversation

@adolago

@adolago adolago commented Jul 2, 2026

Copy link
Copy Markdown
Owner

💡 What: Replaced regex::Regex::new with crate::utils::get_regex in the proxmox inventory plugin's FilterOperator::Regex matching arm.
🎯 Why: Instantiating a new Regex object requires parsing and compiling the pattern, which is computationally expensive. Doing this inside a loop or mapping function (like value_matches_operator) creates a significant performance bottleneck.
📊 Impact: Considerably faster execution times when filtering large amounts of Proxmox VMs or containers based on complex Regex patterns, as the compiled expression is retrieved from a thread-safe cache rather than being compiled on every match check.
🔬 Measurement: Run a cargo test --lib -- tests to verify existing functionality is intact. Performance can be profiled by executing a playbook with intensive Proxmox Regex filtering and comparing execution durations before and after the change.


PR created automatically by Jules for task 6597655516682391507 started by @dolagoartur

Replaces the direct instantiation of `regex::Regex::new` in the proxmox
inventory plugin's `value_matches_operator` filter with the thread-safe
`crate::utils::get_regex` cache. This prevents redundant regex compilations
during heavy filtering loop operations, providing a notable performance boost.

Co-authored-by: dolagoartur <146357947+dolagoartur@users.noreply.github.com>
@google-labs-jules

Copy link
Copy Markdown
Contributor

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Optimize Proxmox inventory regex filtering via cached compiled regexes

✨ Enhancement 🕐 10-20 Minutes

Grey Divider

AI Description

• Use shared cached regex compilation for Proxmox inventory Regex filters.
• Avoid per-match regex parse/compile overhead during large inventory filtering loops.
• Preserve prior behavior by treating invalid regex patterns as non-matching.
Diagram

graph TD
  A["Proxmox inventory plugin"] --> B["value_matches_operator"] --> C{"Operator = Regex?"}
  C -->|Yes| D["utils::get_regex"] --> E[("Global regex cache")]
  D --> F["re.is_match(value)"]
  C -->|No| G["String match ops"]
  subgraph Legend
    direction LR
    _fn["Function"] ~~~ _dec{"Decision"} ~~~ _cache[("Cache")]
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Precompile regex in filter config
  • ➕ Eliminates runtime cache/global state; compilation happens once when parsing filters
  • ➕ Avoids cache eviction semantics and reduces contention under heavy parallelism
  • ➖ Requires broader refactor of filter config types to carry compiled Regex (or fallible parsing)
  • ➖ More invasive changes across plugins/serialization and potentially CLI/API boundaries
2. Adopt an LRU/TTL cache (e.g., moka)
  • ➕ More predictable memory behavior than clear-all-at-threshold
  • ➕ Can provide better hit rate under diverse patterns
  • ➖ Adds dependency/complexity and tuning knobs (size, TTL)
  • ➖ May be overkill if patterns are limited and current cache works well

Recommendation: Current approach is a good minimal, low-risk performance improvement because it reuses an existing shared get_regex utility used elsewhere in the codebase and keeps the change localized to the Proxmox plugin. If regex filtering grows more complex or memory behavior becomes a concern, consider moving toward precompiled regexes in filter configs or swapping the cache policy to LRU/TTL.

Files changed (1) +2 / -1

Enhancement (1) +2 / -1
proxmox.rsUse cached regex compilation for 'FilterOperator::Regex' matching +2/-1

Use cached regex compilation for 'FilterOperator::Regex' matching

• Switches regex matching in 'value_matches_operator' from direct 'Regex::new' to the shared 'crate::utils::get_regex' cache. This avoids repeated regex compilation during filtering loops while keeping the existing 'unwrap_or(false)' behavior for invalid patterns.

src/inventory/plugins/proxmox.rs

@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📜 Skill insights (0)

Context used
✅ Compliance rules (platform): 2 rules

Grey Divider


Informational

1. Shared regex cache coupling 🐞 Bug ➹ Performance
Description
Proxmox FilterOperator::Regex now inserts user-supplied patterns into the process-wide
REGEX_CACHE, changing memory/performance characteristics compared to compile-and-drop behavior.
When the shared cache hits MAX_CACHE_SIZE, get_regex clears it entirely, so Proxmox inventory
runs can evict cached regex used by other components (e.g., templating) and trigger avoidable
recompilation.
Code

src/inventory/plugins/proxmox.rs[R827-830]

+        // Optimize: Use cached get_regex to avoid recompiling the regex pattern during filtering loop operations
+        FilterOperator::Regex => crate::utils::get_regex(filter_value)
            .map(|re| re.is_match(value))
            .unwrap_or(false),
Relevance

⭐ Low

Team repeatedly adopted global get_regex caching despite process-wide effects (PRs #63, #198, #842).

PR-#63
PR-#198
PR-#842

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The diff shows Proxmox filtering now calls crate::utils::get_regex(filter_value) for each regex
check. filter_value is user-configured via PluginConfig.filters, and get_regex stores compiled
regexes in a global DashMap and clears the whole cache once it reaches MAX_CACHE_SIZE, which is
shared across other subsystems like templating filters.

src/inventory/plugins/proxmox.rs[820-831]
src/inventory/plugins/config.rs[30-45]
src/utils/regex_cache.rs[10-46]
src/template.rs[641-676]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`value_matches_operator` now uses the global `crate::utils::get_regex` cache for Proxmox inventory filtering. This makes inventory filtering participate in (and potentially churn) a shared, process-wide cache that uses a clear-all eviction policy at `MAX_CACHE_SIZE`, which can reduce cache hit-rate for other unrelated regex users.

### Issue Context
- Proxmox filtering pulls regex patterns from `PluginConfig.filters` (user YAML), and now those patterns are stored in the shared cache.
- `get_regex` clears the entire cache once it reaches `MAX_CACHE_SIZE`, so any subsystem relying on it may see avoidable recompilation if another subsystem fills the cache.

### Fix Focus Areas
- src/inventory/plugins/proxmox.rs[820-831]
- src/utils/regex_cache.rs[10-46]

### Suggested direction
- Prefer precompiling regex patterns once per Proxmox plugin config (or once per filter evaluation) and reusing them without touching the global cache, **or**
- Introduce a dedicated cache namespace for inventory filtering, **or**
- Replace the clear-all policy with a more granular eviction policy (LRU/TTL), to reduce cross-feature cache disruption.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant