From cea197ed48c6ff2beff5db329c459394d9cc34ec Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Fri, 12 Jan 2024 14:01:21 +1100 Subject: [PATCH 1/2] Optimise no-op changes in VC API --- validator_client/src/http_api/mod.rs | 50 ++++++++++++++++++++-------- 1 file changed, 37 insertions(+), 13 deletions(-) diff --git a/validator_client/src/http_api/mod.rs b/validator_client/src/http_api/mod.rs index c65beb7390a..ca275ad0006 100644 --- a/validator_client/src/http_api/mod.rs +++ b/validator_client/src/http_api/mod.rs @@ -679,7 +679,16 @@ pub fn serve( let maybe_graffiti = body.graffiti.clone().map(Into::into); let initialized_validators_rw_lock = validator_store.initialized_validators(); - let mut initialized_validators = initialized_validators_rw_lock.write(); + let initialized_validators = initialized_validators_rw_lock.upgradable_read(); + + // Do not make any changes if all fields are identical or unchanged. + fn equal_or_none( + current_value: Option, + new_value: Option, + ) -> bool { + new_value.is_none() || current_value == new_value + } + match ( initialized_validators.is_enabled(&validator_pubkey), initialized_validators.validator(&validator_pubkey.compress()), @@ -689,25 +698,40 @@ pub fn serve( validator_pubkey ))), (Some(is_enabled), Some(initialized_validator)) - if Some(is_enabled) == body.enabled - && initialized_validator.get_gas_limit() == body.gas_limit - && initialized_validator.get_builder_proposals() - == body.builder_proposals - && initialized_validator.get_graffiti() == maybe_graffiti => + if equal_or_none(Some(is_enabled), body.enabled) + && equal_or_none( + initialized_validator.get_gas_limit(), + body.gas_limit, + ) + && equal_or_none( + initialized_validator.get_builder_proposals(), + body.builder_proposals, + ) + && equal_or_none( + initialized_validator.get_graffiti(), + maybe_graffiti, + ) => { Ok(()) } (Some(_), _) => { + // Upgrade read lock only in the case where a write is actually + // required. + let mut initialized_validators_write = + parking_lot::RwLockUpgradableReadGuard::upgrade( + initialized_validators, + ); if let Some(handle) = task_executor.handle() { handle .block_on( - initialized_validators.set_validator_definition_fields( - &validator_pubkey, - body.enabled, - body.gas_limit, - body.builder_proposals, - body.graffiti, - ), + initialized_validators_write + .set_validator_definition_fields( + &validator_pubkey, + body.enabled, + body.gas_limit, + body.builder_proposals, + body.graffiti, + ), ) .map_err(|e| { warp_utils::reject::custom_server_error(format!( From bbf1812030e70aa614824bee6dd5ed9488ce767c Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Mon, 15 Jan 2024 12:20:12 +1100 Subject: [PATCH 2/2] Handle another no-op case --- validator_client/src/http_api/mod.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/validator_client/src/http_api/mod.rs b/validator_client/src/http_api/mod.rs index ca275ad0006..285906342de 100644 --- a/validator_client/src/http_api/mod.rs +++ b/validator_client/src/http_api/mod.rs @@ -697,6 +697,8 @@ pub fn serve( "no validator for {:?}", validator_pubkey ))), + // If all specified parameters match their existing settings, then this + // change is a no-op. (Some(is_enabled), Some(initialized_validator)) if equal_or_none(Some(is_enabled), body.enabled) && equal_or_none( @@ -714,6 +716,16 @@ pub fn serve( { Ok(()) } + // Disabling an already disabled validator *with no other changes* is a + // no-op. + (Some(false), None) + if body.enabled.map_or(true, |enabled| !enabled) + && body.gas_limit.is_none() + && body.builder_proposals.is_none() + && maybe_graffiti.is_none() => + { + Ok(()) + } (Some(_), _) => { // Upgrade read lock only in the case where a write is actually // required.