-
Notifications
You must be signed in to change notification settings - Fork 897
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
optimize node startup speed and memory allocation #6952
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -91,6 +91,7 @@ | |
import org.hyperledger.besu.config.CheckpointConfigOptions; | ||
import org.hyperledger.besu.config.GenesisConfigFile; | ||
import org.hyperledger.besu.config.GenesisConfigOptions; | ||
import org.hyperledger.besu.config.JsonUtil; | ||
import org.hyperledger.besu.config.MergeConfigOptions; | ||
import org.hyperledger.besu.consensus.qbft.pki.PkiBlockCreationConfiguration; | ||
import org.hyperledger.besu.consensus.qbft.pki.PkiBlockCreationConfigurationProvider; | ||
|
@@ -117,6 +118,7 @@ | |
import org.hyperledger.besu.ethereum.api.jsonrpc.websocket.WebSocketConfiguration; | ||
import org.hyperledger.besu.ethereum.api.query.BlockchainQueries; | ||
import org.hyperledger.besu.ethereum.chain.Blockchain; | ||
import org.hyperledger.besu.ethereum.chain.VariablesStorage; | ||
import org.hyperledger.besu.ethereum.core.MiningParameters; | ||
import org.hyperledger.besu.ethereum.core.MiningParametersMetrics; | ||
import org.hyperledger.besu.ethereum.core.PrivacyParameters; | ||
|
@@ -1598,7 +1600,8 @@ private void validateChainDataPruningParams() { | |
private GenesisConfigOptions readGenesisConfigOptions() { | ||
|
||
try { | ||
final GenesisConfigFile genesisConfigFile = GenesisConfigFile.fromConfig(genesisConfig()); | ||
final GenesisConfigFile genesisConfigFile = | ||
GenesisConfigFile.fromConfigWithoutAccounts(genesisConfig()); | ||
genesisConfigOptions = genesisConfigFile.getConfigOptions(genesisConfigOverrides); | ||
} catch (final Exception e) { | ||
throw new ParameterException( | ||
|
@@ -2352,16 +2355,46 @@ private EthNetworkConfig updateNetworkConfig(final NetworkName network) { | |
} | ||
|
||
private GenesisConfigFile getGenesisConfigFile() { | ||
return GenesisConfigFile.fromConfig(genesisConfig()); | ||
return GenesisConfigFile.fromConfigWithoutAccounts(genesisConfig()); | ||
} | ||
|
||
private String genesisConfigString = ""; | ||
|
||
private String genesisConfig() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you mean the method |
||
try { | ||
return Resources.toString(genesisFile.toURI().toURL(), UTF_8); | ||
} catch (final IOException e) { | ||
throw new ParameterException( | ||
this.commandLine, String.format("Unable to load genesis URL %s.", genesisFile), e); | ||
if (!genesisConfigString.isEmpty()) { | ||
return genesisConfigString; | ||
} | ||
if (genesisStateHashCacheEnabled) { | ||
// 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In the original code flow, Here, the code is positioned at step (1). If the node has the The intent of this code is to determine if the value of Because if the |
||
} | ||
if (genesisConfigString.isEmpty()) { | ||
try { | ||
genesisConfigString = Resources.toString(genesisFile.toURI().toURL(), UTF_8); | ||
} catch (Exception e) { | ||
throw new RuntimeException(e); | ||
} | ||
} | ||
return genesisConfigString; | ||
} | ||
|
||
private static String genesisConfig(final NetworkName networkName) { | ||
|
@@ -2607,7 +2640,7 @@ protected GenesisConfigOptions getActualGenesisConfigOptions() { | |
return Optional.ofNullable(genesisConfigOptions) | ||
.orElseGet( | ||
() -> | ||
GenesisConfigFile.fromConfig( | ||
GenesisConfigFile.fromConfigWithoutAccounts( | ||
genesisConfig(Optional.ofNullable(network).orElse(MAINNET))) | ||
.getConfigOptions(genesisConfigOverrides)); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,15 +16,19 @@ | |
|
||
import org.hyperledger.besu.util.number.PositiveNumber; | ||
|
||
import java.io.File; | ||
import java.io.IOException; | ||
import java.util.Locale; | ||
import java.util.Map; | ||
import java.util.Optional; | ||
import java.util.OptionalInt; | ||
import java.util.OptionalLong; | ||
|
||
import com.fasterxml.jackson.core.JsonFactory; | ||
import com.fasterxml.jackson.core.JsonParser; | ||
import com.fasterxml.jackson.core.JsonParser.Feature; | ||
import com.fasterxml.jackson.core.JsonProcessingException; | ||
import com.fasterxml.jackson.core.JsonToken; | ||
import com.fasterxml.jackson.databind.JsonNode; | ||
import com.fasterxml.jackson.databind.ObjectMapper; | ||
import com.fasterxml.jackson.databind.node.ArrayNode; | ||
|
@@ -321,6 +325,47 @@ public static ObjectNode objectNodeFromString( | |
} | ||
} | ||
|
||
/** | ||
* Object node from string without some field. | ||
* | ||
* @param jsonData the json data | ||
* @param allowComments true to allow comments | ||
* @param withoutField the without field | ||
* @return the object node | ||
*/ | ||
public static ObjectNode objectNodeFromStringWithout( | ||
final String jsonData, final boolean allowComments, final String withoutField) { | ||
final ObjectMapper objectMapper = new ObjectMapper(); | ||
JsonFactory jsonFactory = | ||
JsonFactory.builder() | ||
.configure(JsonFactory.Feature.INTERN_FIELD_NAMES, false) | ||
.configure(JsonFactory.Feature.CANONICALIZE_FIELD_NAMES, false) | ||
.build(); | ||
jsonFactory.configure(JsonParser.Feature.ALLOW_COMMENTS, allowComments); | ||
|
||
ObjectNode root = objectMapper.createObjectNode(); | ||
|
||
try (JsonParser jp = jsonFactory.createParser(jsonData)) { | ||
if (jp.nextToken() != JsonToken.START_OBJECT) { | ||
throw new RuntimeException("Expected data to start with an Object"); | ||
} | ||
|
||
while (jp.nextToken() != JsonToken.END_OBJECT) { | ||
String fieldName = jp.getCurrentName(); | ||
if (withoutField.equals(fieldName)) { | ||
jp.nextToken(); | ||
jp.skipChildren(); | ||
} else { | ||
jp.nextToken(); | ||
root.set(fieldName, objectMapper.readTree(jp)); | ||
} | ||
} | ||
} catch (Exception e) { | ||
throw new RuntimeException(e); | ||
} | ||
return root; | ||
} | ||
|
||
/** | ||
* Gets json. | ||
* | ||
|
@@ -466,4 +511,94 @@ private static boolean validateInt(final JsonNode node) { | |
} | ||
return true; | ||
} | ||
|
||
/** | ||
* Get the JSON representation of a genesis file without a specific field. | ||
* | ||
* @param genesisFile The genesis file to read. | ||
* @param excludedFieldName The field to exclude from the JSON representation. | ||
* @return The JSON representation of the genesis file without the excluded field. | ||
*/ | ||
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(); | ||
} | ||
Comment on lines
+522
to
+541
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Have you checked if Jackson natively support excluding fields without having to implement the parsing methods? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I have checked it, and this is the fastest method with the least memory usage. I will check it again in a moment. |
||
|
||
private static String handleObject(final JsonParser parser, final String excludedFieldName) | ||
throws IOException { | ||
StringBuilder objectBuilder = new StringBuilder(); | ||
objectBuilder.append("{"); | ||
String fieldName; | ||
boolean isFirstField = true; | ||
while (parser.nextToken() != JsonToken.END_OBJECT) { | ||
fieldName = parser.getCurrentName(); | ||
if (fieldName != null && fieldName.equals(excludedFieldName)) { | ||
parser.skipChildren(); // Skip this field | ||
continue; | ||
} | ||
if (!isFirstField) objectBuilder.append(", "); | ||
parser.nextToken(); // move to value | ||
objectBuilder | ||
.append("\"") | ||
.append(fieldName) | ||
.append("\":") | ||
.append(handleValue(parser, excludedFieldName)); | ||
isFirstField = false; | ||
} | ||
objectBuilder.append("}"); | ||
return objectBuilder.toString(); | ||
} | ||
|
||
private static String handleValue(final JsonParser parser, final String excludedFieldName) | ||
throws IOException { | ||
JsonToken token = parser.getCurrentToken(); | ||
switch (token) { | ||
case START_OBJECT: | ||
return handleObject(parser, excludedFieldName); | ||
case START_ARRAY: | ||
return handleArray(parser, excludedFieldName); | ||
case VALUE_STRING: | ||
return "\"" + parser.getText() + "\""; | ||
case VALUE_NUMBER_INT: | ||
case VALUE_NUMBER_FLOAT: | ||
return parser.getNumberValue().toString(); | ||
case VALUE_TRUE: | ||
case VALUE_FALSE: | ||
return parser.getBooleanValue() ? "true" : "false"; | ||
case VALUE_NULL: | ||
return "null"; | ||
default: | ||
throw new IllegalStateException("Unrecognized token: " + token); | ||
} | ||
} | ||
|
||
private static String handleArray(final JsonParser parser, final String excludedFieldName) | ||
throws IOException { | ||
StringBuilder arrayBuilder = new StringBuilder(); | ||
arrayBuilder.append("["); | ||
boolean isFirstElement = true; | ||
while (parser.nextToken() != JsonToken.END_ARRAY) { | ||
if (!isFirstElement) arrayBuilder.append(", "); | ||
arrayBuilder.append(handleValue(parser, excludedFieldName)); | ||
isFirstElement = false; | ||
} | ||
arrayBuilder.append("]"); | ||
return arrayBuilder.toString(); | ||
} | ||
} |
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.
A
Suppliers.memoize
seems better suited for this fieldThere 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 made a commit to use Suppliers.memoize: 8ed9e4c