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

backend: improve parse function performance in gtime package #1238

Merged
merged 6 commits into from
Feb 26, 2025

Conversation

itsmylife
Copy link
Contributor

@itsmylife itsmylife commented Feb 23, 2025

What this PR does / why we need it:

parse function is used in ParseDuration and ParseInterval. Those methods is used in many places in grafana/grafana. ParseDuration is especially used in settings.go to parse the config. Improving their performance will result a better start time and less memory usage.

This is also a complimentary PR for #1237

This method is using when we get the request and parse the request data into a model. Its overall impact to memory consumption is small comparing to the other parts. But considering that it's been used in many places total impact is still important.

Also I improved it in all areas that means, less allocations, less gc overheat, faster calculations.

Here is the benchmark comparison:

goos: darwin
goarch: arm64
pkg: github.com/grafana/grafana-plugin-sdk-go/backend/gtime
cpu: Apple M1 Pro
                     │  pmem.0.txt   │             pmem.2.txt             │
                     │    sec/op     │   sec/op     vs base               │
Parse/PureNumber-10    164.50n ± 11%   91.89n ± 1%  -44.14% (p=0.002 n=6)
Parse/SimpleUnit-10     79.85n ±  0%   15.21n ± 0%  -80.94% (p=0.002 n=6)
Parse/ComplexUnit-10   105.80n ±  0%   42.40n ± 1%  -59.93% (p=0.002 n=6)
Parse/DateUnit-10      148.55n ±  1%   13.27n ± 4%  -91.07% (p=0.002 n=6)
Parse/MonthUnit-10     151.40n ±  1%   13.66n ± 1%  -90.97% (p=0.002 n=6)
Parse/YearUnit-10      151.20n ±  1%   13.59n ± 0%  -91.01% (p=0.002 n=6)
geomean                 129.5n         22.95n       -82.28%

                     │  pmem.0.txt  │               pmem.2.txt               │
                     │     B/op     │    B/op     vs base                    │
Parse/PureNumber-10      80.00 ± 0%   72.00 ± 0%   -10.00% (p=0.002 n=6)
Parse/SimpleUnit-10      8.000 ± 0%   0.000 ± 0%  -100.00% (p=0.002 n=6)
Parse/ComplexUnit-10     8.000 ± 0%   0.000 ± 0%  -100.00% (p=0.002 n=6)
Parse/DateUnit-10      152.000 ± 0%   4.000 ± 0%   -97.37% (p=0.002 n=6)
Parse/MonthUnit-10     152.000 ± 0%   4.000 ± 0%   -97.37% (p=0.002 n=6)
Parse/YearUnit-10      152.000 ± 0%   4.000 ± 0%   -97.37% (p=0.002 n=6)
geomean                  51.18                    ?                      ¹ ²
¹ summaries must be >0 to compute geomean
² ratios must be >0 to compute geomean

                     │ pmem.0.txt │               pmem.2.txt               │
                     │ allocs/op  │ allocs/op   vs base                    │
Parse/PureNumber-10    5.000 ± 0%   4.000 ± 0%   -20.00% (p=0.002 n=6)
Parse/SimpleUnit-10    1.000 ± 0%   0.000 ± 0%  -100.00% (p=0.002 n=6)
Parse/ComplexUnit-10   1.000 ± 0%   0.000 ± 0%  -100.00% (p=0.002 n=6)
Parse/DateUnit-10      3.000 ± 0%   1.000 ± 0%   -66.67% (p=0.002 n=6)
Parse/MonthUnit-10     3.000 ± 0%   1.000 ± 0%   -66.67% (p=0.002 n=6)
Parse/YearUnit-10      3.000 ± 0%   1.000 ± 0%   -66.67% (p=0.002 n=6)
geomean                2.265                    ?                      ¹ ²
¹ summaries must be >0 to compute geomean
² ratios must be >0 to compute geomean

@itsmylife itsmylife requested review from a team as code owners February 23, 2025 16:55
@itsmylife itsmylife requested review from aangelisc and a team and removed request for a team February 23, 2025 16:55
Copy link

@beejeebus beejeebus left a comment

Choose a reason for hiding this comment

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

LGTM, one nit in the parse function.

@@ -75,18 +75,45 @@ func ParseDuration(inp string) (time.Duration, error) {
}

func parse(inp string) (time.Duration, string, error) {
result := dateUnitPattern.FindSubmatch([]byte(inp))
// Fast path for simple duration formats (no date units)
if len(inp) > 0 {

Choose a reason for hiding this comment

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

I think this should be:

  if inp == "" {
    return 0, "", errors.New("empty input")
  }

then the following code can come out of the if block.

@itsmylife itsmylife merged commit e79256b into main Feb 26, 2025
3 checks passed
@itsmylife itsmylife deleted the ismail/gtime-parse-perf-improvement branch February 26, 2025 16:15
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.

2 participants