Servicer module#2
Closed
theinterestedr1 wants to merge 13 commits into
Closed
Conversation
gilxgil
reviewed
Mar 17, 2022
|
|
||
| message BlockNum { | ||
|
|
||
| uint64 num = 1; |
Collaborator
There was a problem hiding this comment.
block height is defined as int64 in tendermint
https://github.com/tendermint/tendermint/blob/master/proto/tendermint/consensus/types.proto
int64 height = 1;
…age for simplicity
nimrod-teich
added a commit
that referenced
this pull request
Nov 26, 2025
## Additional Memory Leaks Found After the initial fix (batch[].Result = nil), profiling showed the provider still OOM'd at 115.76MB. Analysis revealed THREE additional places where 30MB Solana batch responses were being held in memory: ### 1. replyMsgs Array After Marshaling (jsonRPC.go) **Problem**: After `json.Marshal(replyMsgs)`, the replyMsgs[] array continued to hold all JsonrpcMessage.Result fields (30MB × 3 = 90MB) until function return. **Fix**: Set `replyMsgs[idx].Result = nil` immediately after marshaling. The marshaled data is in `retData`, so the RawMessage copies are no longer needed. ### 2. Cache Deep Copy (rpcprovider_server.go) **Problem**: `protocopy.DeepCopyProtoObject()` created a FULL 30MB deep copy of reply.Data in a goroutine for caching. This copy stayed in memory until cache.SetEntry() completed (~100ms+). **Fix**: Share the Data field reference instead of deep copying it. Since reply.Data is immutable after creation (it's a []byte that's never modified), sharing the reference is safe. Only copy the metadata slice. **Impact**: Eliminates 30MB × N concurrent cached requests allocation. ### 3. ConvertBatchElement Dereference **Problem**: `result = *resultRef` in ConvertBatchElement creates a copy of the RawMessage by dereferencing the pointer. This happens BEFORE our `batch[idx].Result = nil` fix, so that fix was too late. **Status**: Can't fix without breaking API. However, fixes #1 and #2 compensate. ## Memory Reduction Summary **Per 3-block batch request:** - Old allocations: 1. HTTP buffer: 30MB 2. JsonrpcMessage.Result: 30MB 3. batch[].Result: 30MB ← FIX 1 (previous commit) 4. replyMsgs[].Result: 90MB ← FIX 2 (this commit) 5. retData: 30MB (needed for response) 6. Cache deep copy: 30MB ← FIX 3 (this commit) - Total before all fixes: 240MB per batch - Total after all fixes: ~90MB per batch (HTTP + retData + conversion overhead) - **Reduction: ~62% memory savings** **For 8 concurrent batch requests:** - Before: 8 × 240MB = 1.92GB → OOM - After: 8 × 90MB = 720MB (plus ~30MB baseline) = 750MB → Should not OOM ## Testing - Builds successfully - No behavioral changes (Data reference sharing is safe) - Real-world test pending ## Files Changed - `protocol/chainlib/jsonRPC.go`: - Added `replyMsgs[idx].Result = nil` after marshaling - Expanded comments explaining all 6 memory copies - `protocol/rpcprovider/rpcprovider_server.go`: - Replaced `protocopy.DeepCopyProtoObject()` with manual struct copy - Share Data field reference instead of deep copying - Only deep copy the Metadata slice (small, mutable) - Removed unused protocopy import
12 tasks
avitenzer
added a commit
that referenced
this pull request
Apr 1, 2026
Re-evaluate all 6 shortfalls against current branch state: - #1 (standalone binary): RESOLVED - #2 (test suite): RESOLVED - #3 (lavasession test): pre-existing flaky test, not a regression - #4 (dependency cleanup): cosmos-sdk removed, cosmossdk.io utilities remain - #5 (Dockerfile): still has stale ldflags, needs update - #6 (branch hygiene): team workflow, not code Added action plan with must-do, should-do, and deferred items.
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
servicer work in progress, needs tests and code review