-
Notifications
You must be signed in to change notification settings - Fork 861
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Estimate the memory size of EIP-7702 transactions #7984
Estimate the memory size of EIP-7702 transactions #7984
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small comments, but nothing really blocking.
@@ -489,6 +509,15 @@ private long sizeOfField(final Object container, final String... excludePaths) { | |||
return size.sum(); | |||
} | |||
|
|||
@SuppressWarnings("unused") | |||
private static void dumpHeap(final String filePath, final boolean live) throws IOException { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see that the method is not used, why do we need to have it ? with no comment, it looks like a dead code for me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
documented how to use this utility
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the detailed documentation, to access JMX mbeans, you need to have a java process running that exposes those Mbeans. I still don't see how can this work in a unit test. Could you give an example how can this work ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just put it wherever you need, no other setup needed
@Test
public void baseFrontierAndAccessListTransactionMemorySize() {
final Transaction txFrontier =
createTransaction(TransactionType.FRONTIER, 1, Wei.of(500), 0, KEYS1);
try {
dumpHeap("/tmp/heap.hprof", true);
} catch (IOException e) {
throw new RuntimeException(e);
}
assertThat(baseTransactionMemorySize(txFrontier, FRONTIER_ACCESS_LIST_CONSTANT_FIELD_PATHS))
.isEqualTo(FRONTIER_AND_ACCESS_LIST_SHALLOW_SIZE);
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
...rg/hyperledger/besu/ethereum/eth/transactions/PendingTransactionEstimatedMemorySizeTest.java
Show resolved
Hide resolved
...eum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/PendingTransaction.java
Outdated
Show resolved
Hide resolved
...rg/hyperledger/besu/ethereum/eth/transactions/PendingTransactionEstimatedMemorySizeTest.java
Outdated
Show resolved
Hide resolved
...rg/hyperledger/besu/ethereum/eth/transactions/PendingTransactionEstimatedMemorySizeTest.java
Show resolved
Hide resolved
d1b2314
to
f164dc9
Compare
Signed-off-by: Fabio Di Fabio <[email protected]>
f164dc9
to
26ac71d
Compare
Signed-off-by: Fabio Di Fabio <[email protected]>
26ac71d
to
7c3955b
Compare
* Estimate the memory size of EIP-7702 transactions Signed-off-by: Fabio Di Fabio <[email protected]> * Apply suggestions from code review Signed-off-by: Fabio Di Fabio <[email protected]> --------- Signed-off-by: Fabio Di Fabio <[email protected]> Signed-off-by: Daniel Lehrner <[email protected]>
PR description
Estimate the memory size of EIP-7702 transactions and implement the detached copy of code delegation objects to avoid keeping references to big bytes arrays while the tx sits in the txpool
Fixed Issue(s)
fixes #7976
Thanks for sending a pull request! Have you done the following?
doc-change-required
label to this PR if updates are required.Locally, you can run these tests to catch failures early:
./gradlew build
./gradlew acceptanceTest
./gradlew integrationTest
./gradlew ethereum:referenceTests:referenceTests