Skip to content
This repository has been archived by the owner on Apr 19, 2024. It is now read-only.

Performance improvements in GetRateLimits() #93

Merged
merged 1 commit into from
Aug 20, 2021
Merged

Conversation

thrawn01
Copy link
Contributor

@thrawn01 thrawn01 commented Jun 4, 2021

Purpose

To incorporate the performance improvements recommended in #74 Comments by @valer-cara

Implementation

  • GetRateLimits() now processes requests synchronously until the key is found to belong to a remote instance. A go-routine is then spawned to handle the remote call.

Results

While the change did show an improvement, I didn't see the huge improvement I was expecting in GetRateLimit tests. The biggest improvement is in the ThunderingHerd tests where there is almost a 1x improvement. Users should notice the greatest improvement for rate limits when using Behavior = GLOBAL and making many concurrent requests.

== Master ==
BenchmarkServer_GetPeerRateLimitNoBatching
BenchmarkServer_GetPeerRateLimitNoBatching/GetPeerRateLimitNoBatching
BenchmarkServer_GetPeerRateLimitNoBatching/GetPeerRateLimitNoBatching-12            10000    108844 ns/op
BenchmarkServer_GetRateLimit
BenchmarkServer_GetRateLimit/GetRateLimit
BenchmarkServer_GetRateLimit/GetRateLimit-12                                         1581    778386 ns/op
BenchmarkServer_GetRateLimitGlobal
BenchmarkServer_GetRateLimitGlobal/GetRateLimitGlobal
BenchmarkServer_GetRateLimitGlobal/GetRateLimitGlobal-12                             4720    248670 ns/op
BenchmarkServer_Ping
BenchmarkServer_Ping/HealthCheck
BenchmarkServer_Ping/HealthCheck-12                                                 13084     90224 ns/op
BenchmarkServer_ThunderingHeard
BenchmarkServer_ThunderingHeard/ThunderingHeard
BenchmarkServer_ThunderingHeard/ThunderingHeard-12                                  45579     26908 ns/op

== Performance Improvements ==
BenchmarkServer_GetPeerRateLimitNoBatching
BenchmarkServer_GetPeerRateLimitNoBatching/GetPeerRateLimitNoBatching
BenchmarkServer_GetPeerRateLimitNoBatching/GetPeerRateLimitNoBatching-12            11348    102626 ns/op
BenchmarkServer_GetRateLimit
BenchmarkServer_GetRateLimit/GetRateLimit
BenchmarkServer_GetRateLimit/GetRateLimit-12                                         1612    739589 ns/op
BenchmarkServer_GetRateLimitGlobal
BenchmarkServer_GetRateLimitGlobal/GetRateLimitGlobal
BenchmarkServer_GetRateLimitGlobal/GetRateLimitGlobal-12                             5185    199153 ns/op
BenchmarkServer_Ping
BenchmarkServer_Ping/HealthCheck
BenchmarkServer_Ping/HealthCheck-12                                                 13460     89366 ns/op
BenchmarkServer_ThunderingHerd
BenchmarkServer_ThunderingHerd/ThunderingHerd
BenchmarkServer_ThunderingHerd/ThunderingHerd-12                                    76543     16422 ns/op

Copy link
Contributor

@dimaqq dimaqq left a comment

Choose a reason for hiding this comment

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

[I'm not a pro gopher] the intent of the change makes sense to me.

@thrawn01 thrawn01 merged commit 72b9541 into master Aug 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants