Async Cache Operations and File-Based Storage Implementation#1534
Async Cache Operations and File-Based Storage Implementation#1534chsami merged 2 commits intochsami:developmentfrom
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Walkthrough
Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/cache/Rs2Cache.java (1)
576-585: Return a nullable Long from getCacheTimestamp and update callersJavadoc/implementation mismatch (Javadoc says -1, impl returns 0L) and callers are inconsistent: several callers expect a nullable Long (they do null checks) while one caller (Rs2CacheManager) treats the result as a primitive long. Change the API to return Long (nullable) and return cacheTimestamps.get(key); update the primitive-using call site to handle null/sentinel.
- Change: runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/cache/Rs2Cache.java — make signature "public Long getCacheTimestamp(K key)" and return cacheTimestamps.get(key).
- Update: runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/cache/Rs2CacheManager.java:892 — stop unboxing; use Long and check (cacheTimestamp == null || cacheTimestamp <= 0) as appropriate.
- No change needed for callers already checking null:
- runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/cache/Rs2QuestCache.java:614
- runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/cache/Rs2NpcCache.java:450
- runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/cache/Rs2VarPlayerCache.java:390
- runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/cache/Rs2GroundItemCache.java:669
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/cache/Rs2CacheManager.java (4)
595-610: Profile key check uses the wrong variable; prevents VarPlayer cache from loadingYou validate rsProfileKey (the AtomicReference) before setting it, instead of validating the freshly read profileKey. This early return fires almost always.
- String profileKey = Microbot.getConfigManager().getRSProfileKey(); - if(rsProfileKey == null || rsProfileKey.get().isEmpty()){ + String profileKey = Microbot.getConfigManager().getRSProfileKey(); + if (profileKey == null || profileKey.isEmpty()) { log.warn("Cannot load persistent varplayer cache: profile key is null"); return; } - rsProfileKey.set( profileKey); - if (rsProfileKey == null || rsProfileKey.get().isEmpty()) { + rsProfileKey.set(profileKey); + if (rsProfileKey.get().isEmpty()) { log.warn("Cannot load persistent varplayer cache: profile key is null"); return; }
784-791: String reference comparison and wrong value loggedUsing == on Strings is incorrect; also the log argument passes the AtomicReference, not the actual key.
- if ( isCacheDataValid() - && rsProfileKey != null - && Microbot.getConfigManager() != null - && rsProfileKey.get() == Microbot.getConfigManager().getRSProfileKey()) { - log.info("In Setting initial cache state as unknown for profile '{}', saving current cache state", rsProfileKey); + if (isCacheDataValid() + && Microbot.getConfigManager() != null + && rsProfileKey.get().equals(Microbot.getConfigManager().getRSProfileKey())) { + log.info("Setting initial cache state as unknown for profile '{}', saving current cache state", rsProfileKey.get());
845-848: Previous-profile save is gated by current-profile validityDuring a profile switch this guard prevents saving the previous profile because isCacheDataValid() will be false once ConfigManager has the new key. Let the save method handle validity using the explicit profileKey.
- if (rsProfileKey != null&& !rsProfileKey.get().isEmpty() && isCacheDataValid()) { + if (rsProfileKey != null && !rsProfileKey.get().isEmpty()) { log.info("Saving current cache state before loading new profile: {}, we have valid cache", rsProfileKey.get()); savePersistentCaches(rsProfileKey.get()); }
1018-1041: Same blocking issue in sync save pathMirror the async fix so synchronous saves can also persist the previous profile during a switch.
public static void savePersistentCaches(String profileKey) { try { - if (!isCacheDataValid() ) { - log.warn("Cache data is not valid, cannot save persistent caches"); - return; - } - if (profileKey == null) { + if (profileKey == null) { log.warn("Cannot save persistent caches: profile key is null"); return; } + if (profileKey.equals(rsProfileKey.get())) { + if (!isCacheDataValid()) { + log.warn("Cache data is not valid for profile '{}', cannot save persistent caches", profileKey); + return; + } + } else { + log.debug("Saving caches for previous profile '{}' (active='{}')", profileKey, rsProfileKey.get()); + }
🧹 Nitpick comments (7)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/cache/compression/CompressionUtils.java (4)
66-69: SLF4J formatting: {:.2f} won’t renderUse {} and pre‑format numerics; current pattern will log literals.
- log.debug("Compressed cache data: {} bytes → {} bytes (ratio: {:.2f}, time: {:.2f}ms)", - originalSize, compressedSize, compressionRatio, compressionTimeMs); + log.debug("Compressed cache data: {} bytes → {} bytes (ratio: {}, time: {} ms)", + originalSize, compressedSize, + String.format("%.2f", compressionRatio), + String.format("%.2f", compressionTimeMs));
104-106: SLF4J formatting: {:.2f} won’t render (decompression log)Same issue as above.
- log.debug("Decompressed cache data: {} bytes → {} bytes (time: {:.2f}ms)", - compressedBytes.length, originalBytes.length, decompressionTimeMs); + log.debug("Decompressed cache data: {} bytes → {} bytes (time: {} ms)", + compressedBytes.length, originalBytes.length, + String.format("%.2f", decompressionTimeMs));
145-147: Suitability check ignores Base64 expansionHeuristic compares raw compressed estimate to cap; factor Base64 growth to reduce false positives.
- int estimatedSize = estimateCompressedSize(jsonData); - return estimatedSize <= MAX_COMPRESSED_SIZE; + int estimatedRawCompressedSize = estimateCompressedSize(jsonData); + int estimatedEncodedSize = base64EncodedSize(estimatedRawCompressedSize); + return estimatedEncodedSize <= MAX_COMPRESSED_SIZE;Add this helper (outside this hunk):
private static int base64EncodedSize(int rawSize) { return ((rawSize + 2) / 3) * 4; }
217-219: Guard GZIP header mutationAdd a length check to avoid potential AIOOBE on unexpected outputs.
- byte[] out = bout.toByteArray(); - out[9] = 0; // JDK-8244706: set OS to 0 + byte[] out = bout.toByteArray(); + if (out.length > 9) { // safety guard + out[9] = 0; // JDK-8244706: set OS to 0 + } return out;runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/cache/Rs2CacheManager.java (1)
169-174: Incorrect warning message when already registeredThe condition conflates “null EventBus” with “already registered” and always logs the null-EventBus warning even when the bus is non-null. Split the branches.
- if (eventBus == null || isEventRegistered.get()) { - log.warn("EventBus is null, cannot register cache event handlers"); - return; - } + if (eventBus == null) { + log.warn("EventBus is null, cannot register cache event handlers"); + return; + } + if (isEventRegistered.get()) { + log.debug("Cache event handlers already registered; skipping"); + return; + }runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/cache/serialization/CacheSerializationManager.java (1)
435-444: Method/API-doc mismatch: clears “for a specific profile and player” but no player parameter.
clearCache(String configKey, String rsProfileKey)silently uses the current player. You can’t clear for an arbitrary player when logged out, contradicting the docstring.Options:
- Update the docs to reflect current behavior; or
- Add a player‑scoped overload and prefer that from call sites.
Suggested addition (new overload outside this block):
public static void clearCache(String configKey, String rsProfileKey, String playerName) { clearCacheFiles(configKey, rsProfileKey, playerName); }runelite-client/src/main/java/net/runelite/client/plugins/microbot/MicrobotPlugin.java (1)
540-552: Avoid double-closing Rs2CacheManager in shutdown pathsonClientShutdown() closes the cache manager, and shutDown() -> shutdownCacheSystem() also closes it. This split responsibility risks “already closed” errors depending on event ordering. Consolidate closure to a single place.
Apply within this block:
- Rs2CacheManager.getInstance().close(); + // Defer close to shutdownCacheSystem() to avoid double-close during plugin shutdown
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/MicrobotConfig.java(2 hunks)runelite-client/src/main/java/net/runelite/client/plugins/microbot/MicrobotPlugin.java(2 hunks)runelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/pathfinder/PathfinderConfig.java(3 hunks)runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/cache/Rs2Cache.java(1 hunks)runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/cache/Rs2CacheManager.java(17 hunks)runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/cache/compression/CompressionUtils.java(1 hunks)runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/cache/serialization/CacheSerializationManager.java(10 hunks)
👮 Files not reviewed due to content moderation or server errors (1)
- runelite-client/src/main/java/net/runelite/client/plugins/microbot/MicrobotConfig.java
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-23T16:17:37.615Z
Learnt from: g-mason0
PR: chsami/Microbot#1423
File: runelite-client/src/main/java/net/runelite/client/plugins/microbot/externalplugins/MicrobotPluginManager.java:366-389
Timestamp: 2025-08-23T16:17:37.615Z
Learning: The ScheduledExecutorService injected into MicrobotPluginManager (and other RuneLite classes) is configured as a single-threaded executor via Executors.newSingleThreadScheduledExecutor() at RuneLiteModule.java line 130, wrapped in ExecutorServiceExceptionLogger. This ensures all async tasks execute sequentially, preventing race conditions.
Applied to files:
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/cache/Rs2CacheManager.java
📚 Learning: 2025-09-03T03:59:10.180Z
Learnt from: g-mason0
PR: chsami/Microbot#1462
File: runelite-client/src/main/java/net/runelite/client/plugins/microbot/ui/MicrobotPluginHubPanel.java:343-344
Timestamp: 2025-09-03T03:59:10.180Z
Learning: In MicrobotPluginManager, the public methods installPlugin(), removePlugin(), and update() already use executor.submit() internally to perform their operations asynchronously, making them non-blocking on the EDT. These are wrapper methods that delegate to the actual implementation methods (install(), remove(), refresh()) via the executor.
Applied to files:
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/cache/Rs2CacheManager.java
🧬 Code graph analysis (1)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/cache/serialization/CacheSerializationManager.java (2)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/cache/Rs2CacheManager.java (1)
Slf4j(26-1197)runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/cache/compression/CompressionUtils.java (1)
Slf4j(22-240)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (1)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/pathfinder/PathfinderConfig.java (1)
546-553: Remove fallback suggestion—cached boosted levels never return ≤0, so the sentinel scenario described cannot occur.Likely an incorrect or invalid review comment.
.../main/java/net/runelite/client/plugins/microbot/util/cache/compression/CompressionUtils.java
Outdated
Show resolved
Hide resolved
.../main/java/net/runelite/client/plugins/microbot/util/cache/compression/CompressionUtils.java
Outdated
Show resolved
Hide resolved
...te-client/src/main/java/net/runelite/client/plugins/microbot/util/cache/Rs2CacheManager.java
Show resolved
Hide resolved
...te-client/src/main/java/net/runelite/client/plugins/microbot/util/cache/Rs2CacheManager.java
Show resolved
Hide resolved
...te-client/src/main/java/net/runelite/client/plugins/microbot/util/cache/Rs2CacheManager.java
Show resolved
Hide resolved
...net/runelite/client/plugins/microbot/util/cache/serialization/CacheSerializationManager.java
Outdated
Show resolved
Hide resolved
...net/runelite/client/plugins/microbot/util/cache/serialization/CacheSerializationManager.java
Outdated
Show resolved
Hide resolved
...net/runelite/client/plugins/microbot/util/cache/serialization/CacheSerializationManager.java
Outdated
Show resolved
Hide resolved
Voxsylvae
force-pushed
the
feat/improved-Rs2CacheManager
branch
2 times, most recently
from
e178b82 to
cbe953d
Compare
|
fixed the suggestion form coderabbitai |
Voxsylvae
force-pushed
the
feat/improved-Rs2CacheManager
branch
2 times, most recently
from
9e90991 to
df2d3c9
Compare
|
To clarify how caching works: Nothing is stored in the .properties profile file. All cache data is stored separately under Serialization is responsible for reading and writing the cache files located in that caches folder. Strategy classes define how each cache is update, for example, how quest states or inventory data get refreshed. Utils are helper classes used for performing cache-related queries or formatting logs. Each Rs2Cache instance manages its own in-memory values and persistence for a specific data type (e.g., quests, items). Rs2CacheManager acts as the central controller that tracks and coordinates all Rs2Cache instances. |
| position = 0, | ||
| section = cloudSyncSection | ||
| ) | ||
| default boolean enableCacheCloudSync() { |
There was a problem hiding this comment.
Do we really need this? I think we can remove it and also the compressionUtils to keep things less bloated.
…2CacheManager - Add async save/load operations (savePersistentCachesAsync, loadCachesAsync) using CompletableFuture - Implement file-based cache storage under .runelite/microbot-profiles to prevent profile bloat - Add cloud sync configuration option for optional RuneLite profile synchronization - Enhance CacheMetadata with data integrity checks and debugging information - Improve profile change handling with async operations and proper operation chaining - Add dedicated thread pool (cacheManagerExecutor) for cache persistence operations - Implement operation tracking to prevent concurrent save/load conflicts - Update skill level checks in PathfinderConfig to use cached values when available - Add graceful shutdown handling with timeout for ongoing cache operations - Improve error handling and logging throughout cache management system
Voxsylvae
force-pushed
the
feat/improved-Rs2CacheManager
branch
from
df2d3c9 to
dd4bf89
Compare
- Implement dual storage system: file-based (primary) + compressed profile sync (secondary) - Add CompressionUtils with GZip compression and Base64 encoding for profile storage - Update CacheSerializationManager with cloud sync methods (saveCacheToProfile, loadCacheFromProfile) - Integrate dual storage in Rs2CacheManager for all persistent caches - Add configuration option enableCacheCloudSync for optional cross-device sync - Files provide fast local access while compressed profiles enable device synchronization - Size limit of 64KB for compressed profile data to prevent profile bloat - File-first loading strategy with profile fallback for optimal performance
Voxsylvae
force-pushed
the
feat/improved-Rs2CacheManager
branch
from
dd4bf89 to
11dc84e
Compare
2 participants
If it has any value and is appreciated for improving the already in place caching system -> This PR would implements significant improvements to the Rs2CacheManager system, focusing on performance, reliability, and preventing RuneLite profile bloat.
Key Changes
Async Operations
File-Based Storage
.runelite/microbot-profilesdirectoryConfiguration Improvements
Technical Enhancements
Performance Impact