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.

Sign up for GitHub

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

Jump to bottom

Label: Changed regexp.go file and added a single test case 'ſſs' #14086

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

Label: Changed regexp.go file and added a single test case 'ſſs' #14086

wants to merge 1 commit into from
+10 −2

Conversation

kushalShukla-web
Copy link
Contributor

@kushalShukla-web kushalShukla-web commented May 13, 2024 •
edited

Fixes #14066

Added a test case "ſſs" and changed Regexp.go file , such that all the test case will pass.

@kushalShukla-web
Changed regexp.go file and added a single test case 'ſſs'
2fec1e8 
Signed-off-by: Kushal Shukla <kushalshukla110@gmail.com>
Signed-off-by: kushagra Shukla <kushalshukla110@gmail.com>
@kushalShukla-web
Copy link
Contributor Author

pls review it @colega . and let me know if i can make some more optimization .

@colega
Copy link
Contributor

colega commented May 13, 2024

As I mentioned in my comment this looks like the least performant way to implement this, and I'd guess that it can be even slower than running the original regular expression before optimization.

I would run some benchmarks to see the performance impact of looping here.

@tesla59
Copy link
Contributor

tesla59 commented May 15, 2024 •
edited

Here is the impact of the loop

goos: darwin
goarch: arm64
pkg: github.com/prometheus/prometheus/model/labels
                                                    │  test1.txt   │               test2.txt                │
                                                    │    sec/op    │    sec/op     vs base                  │
FastRegexMatcher/#00-8                                43.80n ±  4%    45.18n ± 1%          ~ (p=0.138 n=10)
FastRegexMatcher/foo-8                                52.66n ±  3%    51.74n ± 1%          ~ (p=0.779 n=10)
FastRegexMatcher/^foo-8                               45.23n ±  5%    45.32n ± 4%          ~ (p=0.836 n=10)
FastRegexMatcher/(foo|bar)-8                          54.62n ±  0%    55.14n ± 1%     +0.95% (p=0.004 n=10)
FastRegexMatcher/foo.*-8                              93.60n ±  0%    93.91n ± 1%          ~ (p=0.060 n=10)
FastRegexMatcher/.*foo-8                              111.5n ±  0%    111.8n ± 0%     +0.27% (p=0.001 n=10)
FastRegexMatcher/^.*foo$-8                            111.6n ±  2%    111.8n ± 2%          ~ (p=0.419 n=10)
FastRegexMatcher/^.+foo$-8                            113.8n ±  4%    111.9n ± 0%     -1.71% (p=0.001 n=10)
FastRegexMatcher/.*-8                                 72.21n ± 13%    79.39n ± 2%     +9.94% (p=0.023 n=10)
FastRegexMatcher/.+-8                                 83.32n ±  3%    81.88n ± 3%     -1.73% (p=0.011 n=10)
FastRegexMatcher/foo.+-8                              96.25n ±  4%    95.68n ± 3%          ~ (p=0.393 n=10)
FastRegexMatcher/.+foo-8                              112.3n ±  3%    112.3n ± 2%          ~ (p=0.927 n=10)
FastRegexMatcher/foo_.+-8                             81.34n ±  4%    80.90n ± 1%          ~ (p=0.075 n=10)
FastRegexMatcher/foo_.*-8                             80.40n ±  1%    80.36n ± 0%          ~ (p=0.470 n=10)
FastRegexMatcher/.*foo.*-8                            188.9n ±  0%    189.3n ± 0%     +0.21% (p=0.043 n=10)
FastRegexMatcher/.+foo.+-8                            199.2n ±  0%    201.1n ± 0%     +0.98% (p=0.000 n=10)
FastRegexMatcher/(?s:.*)-8                            42.57n ±  1%    42.48n ± 1%          ~ (p=0.055 n=10)
FastRegexMatcher/(?s:.+)-8                            50.90n ±  6%    50.46n ± 0%     -0.86% (p=0.003 n=10)
FastRegexMatcher/(?s:^.*foo$)-8                       111.2n ±  1%    109.2n ± 1%     -1.84% (p=0.001 n=10)
FastRegexMatcher/(?i:foo)-8                           69.84n ±  4%    69.83n ± 2%          ~ (p=0.871 n=10)
FastRegexMatcher/(?i:(foo|bar))-8                     157.8n ±  1%    157.5n ± 8%          ~ (p=0.985 n=10)
FastRegexMatcher/(?i:(foo1|foo2|bar))-8               281.8n ±  2%    281.3n ± 0%          ~ (p=0.182 n=10)
FastRegexMatcher/^(?i:foo|oo)|(bar)$-8                718.9n ±  1%    720.6n ± 0%          ~ (p=0.425 n=10)
FastRegexMatcher/(?i:(foo1|foo2|aaa|bbb|ccc|ddd|e-8   1.576µ ±  1%    6.052µ ± 2%   +284.13% (p=0.000 n=10)
FastRegexMatcher/((.*)(bar|b|buzz)(.+)|foo)$-8        478.9n ±  1%    479.7n ± 0%          ~ (p=0.118 n=10)
FastRegexMatcher/^$-8                                 43.71n ±  3%    42.54n ± 2%     -2.69% (p=0.006 n=10)
FastRegexMatcher/(prometheus|api_prom)_api_v1_.+-8    185.0n ±  1%    185.0n ± 0%          ~ (p=0.781 n=10)
FastRegexMatcher/10\.0\.(1|2)\.+-8                    80.36n ±  0%    80.13n ± 0%     -0.29% (p=0.000 n=10)
FastRegexMatcher/10\.0\.(1|2).+-8                     80.28n ±  0%    80.13n ± 2%          ~ (p=0.060 n=10)
FastRegexMatcher/((fo(bar))|.+foo)-8                  200.0n ±  7%    199.2n ± 0%     -0.40% (p=0.001 n=10)
FastRegexMatcher/zQPbMkNO|NNSPdvMi|iWuuSoAl|qbvKM-8   204.4n ±  7%    210.8n ± 9%          ~ (p=0.382 n=10)
FastRegexMatcher/jyyfj00j0061|jyyfj00j0062|jyyfj9-8   203.8n ±  9%    206.1n ± 5%          ~ (p=0.853 n=10)
FastRegexMatcher/(?i:(zQPbMkNO|NNSPdvMi|iWuuSoAl|-8   1.587µ ±  1%   18.608µ ± 1%  +1072.93% (p=0.000 n=10)
FastRegexMatcher/(?i:(zQPbMkNO.*|NNSPdvMi.*|iWuuS-8   6.093µ ±  0%    6.104µ ± 0%     +0.18% (p=0.001 n=10)
FastRegexMatcher/(?i:(.*zQPbMkNO|.*NNSPdvMi|.*iWu-8   7.353µ ±  0%    7.367µ ± 1%          ~ (p=0.105 n=10)
FastRegexMatcher/fo.?-8                               92.92n ±  0%    95.05n ± 2%     +2.30% (p=0.002 n=10)
FastRegexMatcher/foo.?-8                              93.25n ±  1%    94.90n ± 0%     +1.77% (p=0.002 n=10)
FastRegexMatcher/f.?o-8                               79.46n ±  0%    79.93n ± 2%          ~ (p=0.382 n=10)
FastRegexMatcher/.*foo.?-8                            202.4n ±  3%    201.5n ± 0%          ~ (p=0.726 n=10)
FastRegexMatcher/.?foo.+-8                            188.8n ±  2%    190.4n ± 0%     +0.85% (p=0.022 n=10)
FastRegexMatcher/foo.?|bar-8                          139.8n ±  5%    138.9n ± 0%     -0.61% (p=0.002 n=10)
geomean                                               154.5n          170.0n         +10.04%

I think we should look for another way to match the case. the performance impact is substantial

@kushalShukla-web
Copy link
Contributor Author

will look at this code again

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Case insensitive FastRegexMatcher doesn't check all folds
3 participants
@kushalShukla-web @colega @tesla59