Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimize wildcard matching by converting SafeFNMatch_ to an iterative approach #1151

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

RadoslavPetkow
Copy link

Refactored SafeFNMatch_
Replaced the recursive implementation with an iterative approach to handle ‘*’ and ‘?’ wildcards. This change eliminates recursive call overhead and reduces potential stack usage, leading to improved performance in logging scenarios.
Maintained Behavior
The change preserves all existing semantics, ensuring that only the internal implementation of wildcard matching is affected.
Minor Code Cleanups
Made slight adjustments to pointer arithmetic and code formatting in functions such as VLOG2Initializer for improved clarity and robustness.

@codecov-commenter
Copy link

codecov-commenter commented Feb 16, 2025

Codecov Report

Attention: Patch coverage is 64.51613% with 11 lines in your changes missing coverage. Please review.

Project coverage is 62.12%. Comparing base (6c5c692) to head (c868344).
Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
src/vlog_is_on.cc 64.51% 6 Missing and 5 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1151      +/-   ##
==========================================
- Coverage   62.22%   62.12%   -0.10%     
==========================================
  Files          19       19              
  Lines        2324     2326       +2     
  Branches      845      843       -2     
==========================================
- Hits         1446     1445       -1     
- Misses        596      600       +4     
+ Partials      282      281       -1     
Files with missing lines Coverage Δ
src/vlog_is_on.cc 60.74% <64.51%> (-2.11%) ⬇️

Copy link
Collaborator

@sergiud sergiud left a comment

Choose a reason for hiding this comment

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

Thanks for the PR.

It looks like the changes cause the demangle unit test to no longer pass on Linux. Could you please also split the SafeFNMatch_ unrelated refactorings and submit these in a separate PR?

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.

3 participants