Skip to content

fix: Cache config metadata per sdk key per config - #201

Open
JamieSinn wants to merge 3 commits into
mainfrom
ICP-8826-fix-memory-alloc
Open

fix: Cache config metadata per sdk key per config#201
JamieSinn wants to merge 3 commits into
mainfrom
ICP-8826-fix-memory-alloc

Conversation

@JamieSinn

Copy link
Copy Markdown
Member

This was over-read from and caused a memory allocation spike that isn't really required as part of the normal operation of the SDK. Caching this once per config write is a more reasonable memory usage path without requiring a whole new release of WASM.

This should resolve #195

This was over-read from and caused a memory allocation spike that isn't really required as part of the normal operation of the SDK. Caching this once per config write is a more reasonable memory usage path without requiring a whole new release of WASM.
@JamieSinn
JamieSinn requested review from kaushalkapasi and a lite review from Copilot August 17, 2026 20:00
@JamieSinn
JamieSinn requested a review from a team as a code owner August 17, 2026 20:00

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR reduces repeated WASM-side and Java-side allocations by caching the config metadata JSON per SDK key, so DevCycleLocalClient.getMetadata() no longer re-reads metadata from the WASM module on every variable evaluation. This aims to prevent the observed RSS growth/memory spike described in issue #195.

Changes:

  • Cache config metadata on storeConfig writes.
  • Add a read-through cache in getConfigMetadata to avoid repeated WASM calls.
  • Minor Java generics modernization (new HashMap<>()).
Suppressed comments (1)

src/main/java/com/devcycle/sdk/server/local/bucketing/LocalBucketing.java:392

  • getConfigMetadata is the only public method in this class that is not synchronized, but it calls getSDKKeyAddress and interacts with the WASM store/linker and mutable HashMaps. Concurrent variable evaluations can call this method from multiple threads and race with storeConfig, which can corrupt the internal maps/WASM interactions. Make this method synchronized and avoid the containsKey()+get() double lookup.
    public String getConfigMetadata(String sdkKey) {
        if (configMetadataCache.containsKey(sdkKey)) {
            return configMetadataCache.get(sdkKey);
        } else {
            int sdkKeyAddress = getSDKKeyAddress(sdkKey);

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/main/java/com/devcycle/sdk/server/local/bucketing/LocalBucketing.java Outdated
Func setConfigDataPtr = linker.get(store, "", "setConfigDataUTF8").get().func();
WasmFunctions.Consumer2<Integer, Integer> fn = WasmFunctions.consumer(store, setConfigDataPtr, I32, I32);
fn.accept(sdkKeyAddress, configAddress);
configMetadataCache.put(sdkKey, internalGetConfigMetadata(sdkKeyAddress));

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The config unit test only checks there is no thrown exceptions - nothing else.

Make this more clear that this is fully isolated to the eventId pinned parameter as part of the queued aggregate and single events. This has no bearing on the actual hotpath.
@JamieSinn JamieSinn changed the title ICP-8826: fix: Cache config metadata per sdk key per config fix: Cache config metadata per sdk key per config Aug 17, 2026
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@JamieSinn
JamieSinn enabled auto-merge (squash) August 17, 2026 20:30
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.

Local bucketing leaks native memory (it's causing OOMs)

2 participants