Memory usage limited for outgoing eth responses #1057
Conversation
|
@mkalinin ready for review and merge |
|
a3535b0#diff-c66ab184b22490e4ae62cc3f62b9b234R44 is a bit non-effective with memory, but we could enhance it later. Usually I see not a big number of pending txs on Mainnet, so slicing is not really used. |
| @@ -423,4 +425,23 @@ public boolean equals(Object o) { | |||
| public int hashCode() { | |||
| return Arrays.hashCode(getHash()); | |||
| } | |||
|
|
|||
| public static final MemSizeEstimator<BlockHeader> MemEstimator = bh -> | |||
There was a problem hiding this comment.
Seems unused. Let's remove it
|
|
||
| Iterator<byte[]> bodiesIterator = blockchain.getIteratorOfBodiesByHashes(msg.getBlockHashes()); | ||
| List<byte[]> bodies = new ArrayList<>(); | ||
| int sizeSum = 80; // ArrayList skeleton |
There was a problem hiding this comment.
i) it's a magic number ii) does 80 bytes matter much with regard to 32Kb limit?
| msg.getBlockIdentifier(), | ||
| msg.getSkipBlocks(), | ||
| min(msg.getMaxHeaders(), MAX_HASHES_TO_SEND), | ||
| msg.isReverse() | ||
| ); | ||
|
|
||
| BlockHeadersMessage response = new BlockHeadersMessage(headers); | ||
| BlockHeadersMessage response = new BlockHeadersMessage(headersIterator); |
There was a problem hiding this comment.
Why do we need to change BlockHeadersMessage?
|
|
||
| /** | ||
| * Sames as {@link #sendTransactions(List)} but input list is randomly sliced to | ||
| * contain not more than {@link MAX_SAFE_TXS} if needed |
There was a problem hiding this comment.
{@link MAX_SAFE_TXS} -> {@link #MAX_SAFE_TXS}
| * contain not more than {@link MAX_SAFE_TXS} if needed | ||
| * @param txs List of txs to send | ||
| */ | ||
| public void sendTransactionsSafely(List<Transaction> txs) { |
There was a problem hiding this comment.
Safely sounds too abstract for me and confusing on the other hand (is regular sendTransactions unsafe??)
|
|
||
| return rlpEncoded; | ||
| } | ||
|
|
||
| public synchronized void purgeEncoded() { |
There was a problem hiding this comment.
it's not used anywhere except tests. I'd remove it therefore
|
@mkalinin done! |
| for (byte[] blockHash : msg.getBlockHashes()) { | ||
| Block block = blockchain.getBlockByHash(blockHash); | ||
| if (block == null) continue; | ||
|
|
||
| List<TransactionReceipt> blockReceipts = new ArrayList<>(); | ||
| sizeSum += 0; |
| byte[] plainMsg = this.getEncoded(); | ||
| return HashUtil.sha3(plainMsg); | ||
| getEncoded(); | ||
| return hash; |
There was a problem hiding this comment.
This change causes an empty array to be returned when (hash == null && parsed == true && rlpEncoded != null). Happens all the time with the StandaloneBlockchain.
…rlpEncoded != null). See pull request ethereum#1057
…true && rlpEncoded != null). See pull request ethereum#1057" This reverts commit 6bce3d0.
…rlpEncoded != null). See pull request ethereum#1057
…true && rlpEncoded != null). See pull request ethereum#1057" This reverts commit 6bce3d0.
@mkalinin please, take a look