Skip to content

perf: reduce allocations in rueidislimiter #795

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

Merged
merged 1 commit into from
Mar 10, 2025
Merged

Conversation

nesty92
Copy link
Contributor

@nesty92 nesty92 commented Mar 10, 2025

goos: linux
goarch: amd64
pkg: github.com/redis/rueidis/rueidislimiter
cpu: AMD Ryzen 7 7840HS with Radeon 780M Graphics   
          │   old.txt   │              new.txt               │
          │   sec/op    │   sec/op     vs base               │
AllowN-16   872.9n ± 1%   787.0n ± 0%  -9.84% (p=0.000 n=20)

          │  old.txt   │              new.txt               │
          │    B/op    │    B/op     vs base                │
AllowN-16   440.0 ± 0%   368.0 ± 0%  -16.36% (p=0.000 n=20)

          │   old.txt   │              new.txt               │
          │  allocs/op  │ allocs/op   vs base                │
AllowN-16   12.000 ± 0%   8.000 ± 0%  -33.33% (p=0.000 n=20)
go test -coverprofile=coverage.out && go tool cover -func=coverage.out
PASS
coverage: 100.0% of statements
ok      github.com/redis/rueidis/rueidislimiter 0.004s
github.com/redis/rueidis/rueidislimiter/limit.go:10:    WithCustomRateLimit     100.0%
github.com/redis/rueidis/rueidislimiter/limiter.go:53:  NewRateLimiter          100.0%
github.com/redis/rueidis/rueidislimiter/limiter.go:84:  Limit                   100.0%
github.com/redis/rueidis/rueidislimiter/limiter.go:88:  Check                   100.0%
github.com/redis/rueidis/rueidislimiter/limiter.go:92:  Allow                   100.0%
github.com/redis/rueidis/rueidislimiter/limiter.go:96:  AllowN                  100.0%
github.com/redis/rueidis/rueidislimiter/syncp.go:15:    Capacity                100.0%
github.com/redis/rueidis/rueidislimiter/syncp.go:19:    ResetLen                100.0%
total:                                                  (statements)            100.0%

Closes #775

- allocs/op: -33.33% (12 -> 8)
- B/op: -16.36% (440 -> 368)
- ns/op: -7.91% (873ns -> 804ns)

Closes redis#775
}
if option.Limit <= 0 {
option.Limit = 1
return nil, ErrInvalidLimit
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks @nesty92. BTY, why do you change these defaults?

Copy link
Contributor Author

@nesty92 nesty92 Mar 10, 2025

Choose a reason for hiding this comment

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

Hi @rueian, just trying to prevent unintended behavior when someone forgets to specify a limit potentially some causing confusion.

@rueian rueian merged commit 696b564 into redis:main Mar 10, 2025
30 checks passed
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.

Need to improve the test coverage on rueidislimiter
2 participants