Artemis: feat: Improve RateLimiterService performance and add client map cleanup

[mike-turintech]

Refactors the RateLimiterService to enhance performance and address potential memory issues related to per-client limiters.

• Performance Improvement: Switched from using java.time.Instant for tracking the last refill time (lastRefill) to using a long representing milliseconds since the epoch (lastRefillMs). This uses System.currentTimeMillis() which is generally faster than Instant.now() and its conversions, reducing overhead in the critical allow() method.

• Client Map Cleanup: Added a mechanism to prevent the clientLimiters ConcurrentHashMap, which stores per-client rate limiters, from growing indefinitely. This uncontrolled growth could lead to significant memory consumption over time in applications with many unique client IDs.

  • Introduced a MAX_CLIENTS constant (10000) as a limit.
  • Added lastCleanupMs and CLEANUP_INTERVAL_MS (5 minutes) to track when cleanup was last performed.
  • Implemented a maybeCleanupClients() method, called from allowClient(), which checks periodically if cleanup is needed based on map size and the interval.
  • The cleanup logic, when triggered and the map size is large, currently removes half of the entries in the map to reduce memory usage.
  • Added logging to report when cleanup occurs and how many limiters are removed.

These changes improve the robustness and efficiency of the rate limiting service, particularly for scenarios involving a high volume of distinct clients.

Detailed Score Information

Score Details

This section contains detailed information about the performance scores for top 5 scored suggestions.

Top Performing Changes

1. src/main/java/com/llmproxy/service/ratelimit/RateLimiterService.java:1-58 - Mean Improvement: 0.54, Mean Original Score: 3.69

• 🔴 Compatibility (Change: -0.17): The constructor and public API for RateLimiterService have not changed, so existing usages will work. However, internal state now uses primitive milliseconds instead of Instant, which can cause minor serialization/deserialization differences or problematic dependency injection if the internal state was expected elsewhere. The cleanup of clientLimiters alters the concrete behavior with a potential for removing clients while in use, which could affect edge-case clients if stateful information was relied upon, but this is minor for typical uses as clientLimiters is an internal implementation detail.

• 🔴 Equivalence (Change: -0.35): The rate limiter's logic and public API remain unchanged. The switch from Instant to System.currentTimeMillis() is a straightforward optimization. The major difference is the periodic cleanup of clientLimiters, which could cause a rarely used client's limiter to be evicted and recreated, potentially resetting their rate limit state. For most cases, this does not affect behavior, but in high-throughput or long-lived scenarios, the cleanup could cause a few requests to be allowed 'early' after cleanup. This is a very minor difference.

• 🟢 Performance (Change: +1.60): The introduction of the maybeCleanupClients mechanism is a crucial improvement, preventing potential OutOfMemoryError or severe performance degradation from an unbounded clientLimiters map. This makes the service significantly more resilient and capable of sustained operation with a large number of diverse clients. The change from Instant to System.currentTimeMillis() for timekeeping in the allow() method is a micro-optimization that could yield minor performance gains in a highly contested hot path by reducing object allocations. While the cleanup process itself introduces some overhead, this is a necessary trade-off for long-term stability and performance.

• 🟢 Quality (Score: 4.56; Change: +0.50): The changes significantly improve code quality, primarily by enhancing robustness. The new maybeCleanupClients method addresses a critical flaw in the original code—the potential for unbounded growth of the clientLimiters map, which could lead to system instability. Fixing this makes the service more reliable and maintainable. Constants are used for configuration, and logging is added for the cleanup process, which are good practices. While the eviction strategy employed (removing an arbitrary half of the entries when the map exceeds certain thresholds) is simplistic, and a comment acknowledges that a more sophisticated approach could be used, its presence is far superior to no eviction mechanism. The switch from Instant to a long for lastRefillMs, while done for performance, is a minor trade-off in terms of pure type safety/expressiveness but is a common practice in performance-sensitive sections and is documented with a comment.

• 🔴 Security (Score: 3.47; Change: -0.33): The addition of a client map cleanup routine helps prevent unbounded memory growth (which is positive for security). However, removing half of the clientLimiters map without checking usage could theoretically result in race conditions or denial of service for rare edge cases if a client is removed and immediately requests again. There is a log message that could be abused for log flooding if the map is routinely pruned, but this risk is low and not materially worse than before.

• 🟢 Value (Change: +2.00): The changes significantly improve the rate limiter implementation by addressing important production concerns. The code now uses System.currentTimeMillis() instead of Instant for better performance in the hot path. It adds a crucial memory management mechanism with the new maybeCleanupClients() method to prevent unbounded memory growth from client-specific rate limiters, which would eventually cause OOM errors in production. The implementation includes smart cleanup logic that only runs periodically and when needed (when client count exceeds thresholds), minimizing performance impact. The code also adds logging to monitor cleanup operations, which helps with observability. These changes transform the implementation from a potential production liability to a robust service that can handle real-world conditions with many clients.