Skip to content

Conversation

@Kikobeats
Copy link
Member

@Kikobeats Kikobeats commented Apr 9, 2024

Sorry for that, but perf matters. Holding this until add a benchmark :cowboy-cool-cry-mild-panic:

before

=== Async RateLimiter Benchmark ===
Iterations: 10000
Concurrency: 100
Rate limit: 100 requests per 60 seconds
IP distribution: 200 IPs (10 hot IPs receiving 20% of traffic)
Redis: localhost:6379
-----------------------------------
Warming up with 1000 requests...
Resetting Redis before benchmark...
Running 10000 iterations...

=== Benchmark Results ===
Total requests: 10000
Successful requests: 10000
Rate limited requests: 0 (0.00%)
Total time: 302.09ms
Requests per second: 33103.24

Latency:
  Average: 1.69ms
  p50: 1.44ms
  p95: 3.93ms
  p99: 6.88ms

**after **

=== Async RateLimiter Benchmark ===
Iterations: 100000
Concurrency: 100
Rate limit: 100 requests per 60 seconds
IP distribution: 200 IPs (10 hot IPs receiving 20% of traffic)
Redis: localhost:6379
-----------------------------------
Warming up with 1000 requests...
Resetting Redis before benchmark...
Running 100000 iterations...

=== Benchmark Results ===
Total requests: 100000
Successful requests: 97534
Rate limited requests: 2466 (2.47%)
Total time: 1231.12ms
Requests per second: 81226.99

Latency:
  Average: 0.63ms
  p50: 0.61ms
  p95: 0.88ms
  p99: 1.13ms

@dy0gu
Copy link

dy0gu commented Apr 8, 2025

Hi there, any plans to merge this? I was thinking about writing my own rate limiter with inlined Lua, much like you just did here. Your solution seems as simple as I was looking to make mine, so I would love to have this in a released version!

@Kikobeats
Copy link
Member Author

Kikobeats commented Apr 8, 2025

Thanks for remembering me to ship this @dy0gu 🙂

I added a benchmark, and it seems the lua implementation is much faster :galactic-brain:

@Kikobeats Kikobeats changed the title perf: embed lua feat: implemented using lua Apr 8, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 5 out of 6 changed files in this pull request and generated 1 comment.

Files not reviewed (1)
  • package.json: Language not supported
Comments suppressed due to low confidence (1)

src/index.js:22

  • If microtime.now() returns a timestamp in microseconds, multiplying duration (in seconds) by 1000 may yield incorrect time differences when later dividing by 1000000. Consider using duration * 1000000 to ensure unit consistency.
local start = now - duration * 1000

Kikobeats and others added 2 commits April 8, 2025 22:59
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@Kikobeats Kikobeats requested a review from Copilot April 8, 2025 21:13
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 6 out of 7 changed files in this pull request and generated 1 comment.

Files not reviewed (1)
  • package.json: Language not supported

@Kikobeats Kikobeats merged commit e28c9fc into master Apr 8, 2025
2 checks passed
@Kikobeats Kikobeats deleted the lua branch April 8, 2025 21:16
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