Skip to content

Conversation

@317787106
Copy link
Contributor

@317787106 317787106 commented Dec 19, 2025

What does this PR do?

  • Optimize memory allocation for the filter interface of JSON-RPC, eth_newFilter, eth_newBlockFilter
  • add config item node.jsonrpc.maxBlockFilterNum

Why are these changes required?

This PR has been tested by:

  • Unit Tests
  • Manual Testing

Follow up

Extra details

Copy link
Contributor

@waynercheung waynercheung left a comment

Choose a reason for hiding this comment

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

LGTM

@317787106 317787106 changed the title fix(api): Optimize memory allocation for the eth_newFilter interface fix(api): Optimize memory allocation for the filter interface of JSON-RPC Dec 22, 2025
public int jsonRpcMaxSubTopics = 1000;
@Getter
@Setter
public int jsonRpcMaxFilterNum = 50000;

Choose a reason for hiding this comment

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

Has this default value been tested, or is there any other reference for it?

Copy link
Contributor Author

@317787106 317787106 Dec 23, 2025

Choose a reason for hiding this comment

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

Yes. It has been tested for mainnet. But nile should set it as 30000 or smaller.

@317787106 317787106 force-pushed the hotfix/fix_newFilter branch 2 times, most recently from 84fa97c to 52a662e Compare December 23, 2025 12:10
continue;
}
entry.getValue().getResult().add(ByteArray.toJsonHex(blockFilterCapsule.getBlockHash()));
String finalBlockHash = blockHash;
Copy link
Contributor

Choose a reason for hiding this comment

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

what about moving cache lookup outside the loop to avoid redundant queries.

Since originalBlockHash is constant, querying the cache N times in the
loop is unnecessary. Moving it outside:

  • Reduces O(N) cache accesses to O(1)
  • Improves performance with identical functionality
  • Better error handling with fallback strategy

// Approach A: Optimal - Query the cache once outside the loop (Recommended)
final String originalBlockHash = ByteArray.toJsonHex(blockFilterCapsule.getBlockHash());

// Query the cache only once
String cachedBlockHash;
try {
    cachedBlockHash = blockHashCache.get(originalBlockHash, () -> originalBlockHash);
} catch (ExecutionException e) {
    logger.error("Getting/loading blockHash from cache failed", e);
    cachedBlockHash = originalBlockHash;  // Fallback: use the original value
}

// Use the cached result directly inside the loop
while (it.hasNext()) {
    Entry<String, BlockFilterAndResult> entry = it.next();
    if (entry.getValue().isExpire()) {
        it.remove();
        continue;
    }
    entry.getValue().getResult().add(cachedBlockHash);
}

Copy link
Contributor Author

@317787106 317787106 Dec 23, 2025

Choose a reason for hiding this comment

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

It might put unnecessary blockHash into blockHashCache if filter map is empty. But it's a good suggestion

@317787106 317787106 force-pushed the hotfix/fix_newFilter branch from 52a662e to 1bc6bdd Compare December 23, 2025 14:18
Copy link
Contributor

@waynercheung waynercheung left a comment

Choose a reason for hiding this comment

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

LGTM

@kuny0707 kuny0707 merged commit 114bbec into tronprotocol:release_v4.8.1 Dec 24, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

8 participants