optimize node startup speed and memory allocation #6952
Conversation
Signed-off-by: lyfsn <dev.wangyu@proton.me>
| } | ||
|
|
||
| private String genesisConfigString = ""; |
There was a problem hiding this comment.
A Suppliers.memoize seems better suited for this field
There was a problem hiding this comment.
I made a commit to use Suppliers.memoize: 8ed9e4c
| } | ||
|
|
||
| private String genesisConfigString = ""; | ||
|
|
||
| private String genesisConfig() { |
There was a problem hiding this comment.
now this method does not only return the genesis config, so the name is no more descriptive of its behavior
There was a problem hiding this comment.
Do you mean the method genesisConfig()?
| // If the genesis state hash is present in the database, we can use the genesis file without | ||
| pluginCommonConfiguration.init( | ||
| dataDir(), | ||
| dataDir().resolve(DATABASE_PATH), | ||
| getDataStorageConfiguration(), | ||
| getMiningParameters()); | ||
| final KeyValueStorageProvider storageProvider = keyValueStorageProvider(keyValueStorageName); | ||
| if (storageProvider != null) { | ||
| boolean isGenesisStateHashPresent; | ||
| try { | ||
| // A null pointer exception may be thrown here if the database is not initialized. | ||
| VariablesStorage variablesStorage = storageProvider.createVariablesStorage(); | ||
| Optional<Hash> genesisStateHash = variablesStorage.getGenesisStateHash(); | ||
| isGenesisStateHashPresent = genesisStateHash.isPresent(); | ||
| } catch (Exception ignored) { | ||
| isGenesisStateHashPresent = false; | ||
| } | ||
| if (isGenesisStateHashPresent) { | ||
| genesisConfigString = JsonUtil.getJsonFromFileWithout(genesisFile, "alloc"); | ||
| } | ||
| } |
There was a problem hiding this comment.
please explain why this code is needed here now, so we can find a better organization for it
There was a problem hiding this comment.
In the original code flow, genesis file (disk) --(1)--> genesis string --(2)--> genesis struct.
Here, the code is positioned at step (1).
If the node has the --genesis-state-hash-cache-enabled parameter enabled, the value of state root hash will be saved in variablesStorage.
The intent of this code is to determine if the value of state root hash already exists in the node's variablesStorage, and if so, during step (1), it will directly ignore the alloc field in the gensis.json file.
Because if the state root hash value already exists, it indicates that the account information in the genesis.json file has already been written into the state database, and there is no need to calculate the state root hash again later.
| public static String getJsonFromFileWithout( | ||
| final File genesisFile, final String excludedFieldName) { | ||
| StringBuilder jsonBuilder = new StringBuilder(); | ||
| JsonFactory jsonFactory = | ||
| JsonFactory.builder() | ||
| .configure(JsonFactory.Feature.INTERN_FIELD_NAMES, false) | ||
| .configure(JsonFactory.Feature.CANONICALIZE_FIELD_NAMES, false) | ||
| .build(); | ||
| try (JsonParser parser = jsonFactory.createParser(genesisFile)) { | ||
| JsonToken token; | ||
| while ((token = parser.nextToken()) != null) { | ||
| if (token == JsonToken.START_OBJECT) { | ||
| jsonBuilder.append(handleObject(parser, excludedFieldName)); | ||
| } | ||
| } | ||
| } catch (Exception e) { | ||
| throw new RuntimeException(e); | ||
| } | ||
| return jsonBuilder.toString(); | ||
| } |
There was a problem hiding this comment.
Have you checked if Jackson natively support excluding fields without having to implement the parsing methods?
There was a problem hiding this comment.
Yes, I have checked it, and this is the fastest method with the least memory usage. I will check it again in a moment.
Signed-off-by: lyfsn <dev.wangyu@proton.me>
|
@lyfsn Great work on the PR, I have few questions if you don't mind :
|
You can refer to this information first, and if you have more specific needs, please feel free to let me explain! |
|
While reviewing your PR, I saw the need for a refactor of how the genesis file and the genesis options are managed, since with the time some tech debt has accumulated, and with that I had the idea of making the load of the accounts lazy, with the hope of moving that logic in the class that manage the genesis file, will let you know when I have something ready to test. |
|
@lyfsn thanks for the troubleshooting data, awesome work.
I'm currently analyzing the regression as we know what code is causing it. |
I guess there are two main reasons:
These were the parameters for the test at that time:
Additionally, are you referring to this blog article? The article also detailed the testing conditions: |
|
are your genesis files public? I would like to test them on the refactor, if not I will try to generate one with random data |
I sent an email to your public email address; it contains the file access document. |
PR description
In summary, if a user employs a large genesis file and activates the
--genesis-state-hash-cache-enabledparameter, their node can start in under 10 seconds, requiring less than 2GB of memory. This is the essence of what this PR accomplishes.Regarding the process of serializing the string into a GenesisConfigFile, many parts of the code follow a similar logic pattern:
GenesisConfigFile.fromConfig(genesisContent).getConfigOptions(), which means obtaining config options from the GenesisConfigFile. In fact, it does not care about the rest of the content, especially the accounts information, while large genesis files often have accounts occupying a large portion.Therefore, this PR introduces a new method,
fromConfigWithoutAccounts, for all places where accounts are not needed. During the serialization of the string into a GenesisConfigFile, it ignores the accounts information. This change is should be safe because the only time accounts information is needed is during the calculation of the genesis state hash.Based on the
-genesis-state-hash-cache-enabledparameter added in feat: add --use-cached-genesis-state-hash paramater #6758, this PR further enhances the performance improvements that the parameter can offer.Originally, the parameter's purpose was to cache and skip the calculation of the genesis state hash value. Building upon the previous improvements, this PR determines that if this parameter is enabled and there is a cached genesis state hash in the database, then it will directly ignore the accounts content when reading the genesis file.
The table below outlines the comparison of node startup time and total memory reclaimed before and after the optimizations made in this PR. All tests utilized a genesis file of 1.1GB in size.
Without the
--genesis-state-hash-cache-enabledparameter enabled, the startup time reduced from 2 minutes and 53 seconds to 2 minutes and 36 seconds, and the total memory reclaimed decreased from 125.47GB to 96.53GB. The optimization effect here is modest.With the
--genesis-state-hash-cache-enabledparameter enabled, the startup time can be reduced from 50 seconds to 9 seconds, and the total memory reclaimed from 66.75GB to 1.31GB, marking a significant optimization effect.This is the unit test coverage report for the PR: Unit Test Coverage Report.
Fixed Issue(s)
Thanks for sending a pull request! Have you done the following?
doc-change-requiredlabel 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