Skip to content

Commit 12bed95

Browse files
committed
Merge branch 'rumenov/scdfo' into 'master'
chore: remove the redundant SubnetConfigs type in favour of a SubnetConfig::new constructor Also don't construct the SubnetConfig as soon as the replica starts but just before the components that require it. See merge request dfinity-lab/public/ic!12564
2 parents 54071ce + cb47050 commit 12bed95

File tree

30 files changed

+601
-735
lines changed

30 files changed

+601
-735
lines changed

rs/config/src/subnet_config.rs

Lines changed: 11 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -439,92 +439,46 @@ impl CyclesAccountManagerConfig {
439439
}
440440
}
441441

442-
/// The per subnet type configuration for CoW Memory Manager
443-
#[derive(Clone)]
444-
pub struct CowMemoryManagerConfig {
445-
/// Flag to enable or disable the feature
446-
pub enabled: bool,
447-
}
448-
449-
impl CowMemoryManagerConfig {
450-
pub fn application_subnet() -> Self {
451-
Self { enabled: false }
452-
}
453-
454-
pub fn system_subnet() -> Self {
455-
Self { enabled: false }
456-
}
457-
458-
pub fn verified_application_subnet() -> Self {
459-
Self { enabled: false }
460-
}
461-
}
462-
463442
/// If a component has at least one static configuration that is different for
464443
/// different subnet types, then it is included in this struct.
465444
#[derive(Clone)]
466445
pub struct SubnetConfig {
467446
pub scheduler_config: SchedulerConfig,
468447
pub cycles_account_manager_config: CyclesAccountManagerConfig,
469-
pub cow_memory_manager_config: CowMemoryManagerConfig,
470448
}
471449

472450
impl SubnetConfig {
451+
pub fn new(own_subnet_type: SubnetType) -> Self {
452+
match own_subnet_type {
453+
SubnetType::Application => Self::default_application_subnet(),
454+
SubnetType::System => Self::default_system_subnet(),
455+
SubnetType::VerifiedApplication => Self::default_verified_application_subnet(),
456+
}
457+
}
458+
473459
/// Returns the subnet configuration for the application subnet type.
474-
pub fn default_application_subnet() -> Self {
460+
fn default_application_subnet() -> Self {
475461
Self {
476462
scheduler_config: SchedulerConfig::application_subnet(),
477463
cycles_account_manager_config: CyclesAccountManagerConfig::application_subnet(),
478-
cow_memory_manager_config: CowMemoryManagerConfig::application_subnet(),
479464
}
480465
}
481466

482467
/// Returns the subnet configuration for the system subnet type.
483-
pub fn default_system_subnet() -> Self {
468+
fn default_system_subnet() -> Self {
484469
Self {
485470
scheduler_config: SchedulerConfig::system_subnet(),
486471
cycles_account_manager_config: CyclesAccountManagerConfig::system_subnet(),
487-
cow_memory_manager_config: CowMemoryManagerConfig::system_subnet(),
488472
}
489473
}
490474

491475
/// Returns the subnet configuration for the verified application subnet
492476
/// type.
493-
pub fn default_verified_application_subnet() -> Self {
477+
fn default_verified_application_subnet() -> Self {
494478
Self {
495479
scheduler_config: SchedulerConfig::verified_application_subnet(),
496480
cycles_account_manager_config: CyclesAccountManagerConfig::verified_application_subnet(
497481
),
498-
cow_memory_manager_config: CowMemoryManagerConfig::verified_application_subnet(),
499-
}
500-
}
501-
}
502-
503-
/// A struct that holds the per subnet configuration for all the subnet types on
504-
/// the internet computer.
505-
pub struct SubnetConfigs {
506-
system_subnet: SubnetConfig,
507-
application_subnet: SubnetConfig,
508-
verified_application_subnet: SubnetConfig,
509-
}
510-
511-
impl Default for SubnetConfigs {
512-
fn default() -> Self {
513-
Self {
514-
system_subnet: SubnetConfig::default_system_subnet(),
515-
application_subnet: SubnetConfig::default_application_subnet(),
516-
verified_application_subnet: SubnetConfig::default_verified_application_subnet(),
517-
}
518-
}
519-
}
520-
521-
impl SubnetConfigs {
522-
/// Returns the appropriate subnet configuration based on the subnet type.
523-
pub fn own_subnet_config(&self, own_subnet_type: SubnetType) -> SubnetConfig {
524-
match own_subnet_type {
525-
SubnetType::Application => self.application_subnet.clone(),
526-
SubnetType::System => self.system_subnet.clone(),
527-
SubnetType::VerifiedApplication => self.verified_application_subnet.clone(),
528482
}
529483
}
530484
}

rs/cycles_account_manager/tests/cycles_account_manager.rs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use ic_base_types::NumSeconds;
2-
use ic_config::subnet_config::SubnetConfigs;
2+
use ic_config::subnet_config::SubnetConfig;
33
use ic_constants::SMALL_APP_SUBNET_MAX_SIZE;
44
use ic_cycles_account_manager::IngressInductionCost;
55
use ic_ic00_types::{CanisterIdRecord, Payload, IC_00};
@@ -296,9 +296,7 @@ fn canister_charge_for_memory_until_zero_works() {
296296
let subnet_size = SMALL_APP_SUBNET_MAX_SIZE;
297297
let mut system_state = SystemStateBuilder::new().build();
298298
let subnet_type = SubnetType::Application;
299-
let config = SubnetConfigs::default()
300-
.own_subnet_config(subnet_type)
301-
.cycles_account_manager_config;
299+
let config = SubnetConfig::new(subnet_type).cycles_account_manager_config;
302300
let gib_stored_per_second_fee = config.gib_storage_per_second_fee;
303301
let cycles_account_manager = CyclesAccountManagerBuilder::new()
304302
.with_subnet_type(subnet_type)

rs/determinism_test/src/setup.rs

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,4 @@
1-
use ic_config::{
2-
subnet_config::{SubnetConfig, SubnetConfigs},
3-
Config,
4-
};
1+
use ic_config::{subnet_config::SubnetConfig, Config};
52
use ic_cycles_account_manager::CyclesAccountManager;
63
use ic_execution_environment::ExecutionServices;
74
use ic_interfaces::execution_environment::IngressHistoryReader;
@@ -100,7 +97,7 @@ pub(crate) fn setup() -> (
10097
let subnet_id = subnet_test_id(1);
10198
let root_subnet_id = subnet_test_id(2);
10299
let (config, _) = Config::temp_config();
103-
let subnet_config = SubnetConfigs::default().own_subnet_config(subnet_type);
100+
let subnet_config = SubnetConfig::new(subnet_type);
104101
let replica_config = ReplicaConfig {
105102
node_id: NodeId::from(PrincipalId::new_node_test_id(27)),
106103
subnet_id,

rs/drun/src/lib.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
33
use crate::message::{msg_stream_from_file, Message};
44
use hex::encode;
5-
use ic_config::{subnet_config::SubnetConfigs, Config};
5+
use ic_config::{subnet_config::SubnetConfig, Config};
66
use ic_cycles_account_manager::CyclesAccountManager;
77
use ic_error_types::{ErrorCode, UserError};
88
use ic_execution_environment::ExecutionServices;
@@ -163,7 +163,7 @@ pub fn run_drun(uo: DrunOptions) -> Result<(), String> {
163163
subnet_type,
164164
} = uo;
165165
// Hardcoded magic values to create a ReplicaConfig that parses.
166-
let mut subnet_config = SubnetConfigs::default().own_subnet_config(subnet_type);
166+
let mut subnet_config = SubnetConfig::new(subnet_type);
167167

168168
// If an intruction limit was specified, update the config with the provided instruction limit.
169169
if let Some(instruction_limit) = instruction_limit {

rs/execution_environment/benchmarks/common.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
use criterion::{BatchSize, Criterion};
55
use ic_config::execution_environment::Config;
66
use ic_config::flag_status::FlagStatus;
7-
use ic_config::subnet_config::{SchedulerConfig, SubnetConfigs};
7+
use ic_config::subnet_config::{SchedulerConfig, SubnetConfig};
88
use ic_constants::SMALL_APP_SUBNET_MAX_SIZE;
99
use ic_cycles_account_manager::CyclesAccountManager;
1010
use ic_error_types::RejectCode;
@@ -225,7 +225,7 @@ where
225225
let log = no_op_logger();
226226
let own_subnet_id = subnet_test_id(1);
227227
let own_subnet_type = SubnetType::Application;
228-
let subnet_configs = SubnetConfigs::default().own_subnet_config(own_subnet_type);
228+
let subnet_configs = SubnetConfig::new(own_subnet_type);
229229
let cycles_account_manager = Arc::new(CyclesAccountManager::new(
230230
subnet_configs.scheduler_config.max_instructions_per_message,
231231
own_subnet_type,

rs/execution_environment/src/scheduler/test_utilities.rs

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use std::{
88
use ic_base_types::{CanisterId, NumBytes, SubnetId};
99
use ic_config::{
1010
flag_status::FlagStatus,
11-
subnet_config::{SchedulerConfig, SubnetConfigs},
11+
subnet_config::{SchedulerConfig, SubnetConfig},
1212
};
1313
use ic_cycles_account_manager::CyclesAccountManager;
1414
use ic_embedders::{
@@ -606,9 +606,7 @@ pub(crate) struct SchedulerTestBuilder {
606606
impl Default for SchedulerTestBuilder {
607607
fn default() -> Self {
608608
let subnet_type = SubnetType::Application;
609-
let scheduler_config = SubnetConfigs::default()
610-
.own_subnet_config(subnet_type)
611-
.scheduler_config;
609+
let scheduler_config = SubnetConfig::new(subnet_type).scheduler_config;
612610
let config = ic_config::execution_environment::Config::default();
613611
let subnet_total_memory = config.subnet_memory_capacity.get();
614612
let max_canister_memory_size = config.max_canister_memory_size.get();
@@ -643,9 +641,7 @@ impl SchedulerTestBuilder {
643641
}
644642

645643
pub fn with_subnet_type(self, subnet_type: SubnetType) -> Self {
646-
let scheduler_config = SubnetConfigs::default()
647-
.own_subnet_config(subnet_type)
648-
.scheduler_config;
644+
let scheduler_config = SubnetConfig::new(subnet_type).scheduler_config;
649645
Self {
650646
subnet_type,
651647
scheduler_config,
@@ -743,9 +739,7 @@ impl SchedulerTestBuilder {
743739
state.metadata.network_topology.nns_subnet_id = self.nns_subnet_id;
744740
state.metadata.batch_time = self.batch_time;
745741

746-
let config = SubnetConfigs::default()
747-
.own_subnet_config(self.subnet_type)
748-
.cycles_account_manager_config;
742+
let config = SubnetConfig::new(self.subnet_type).cycles_account_manager_config;
749743
if let Some(ecdsa_key) = &self.ecdsa_key {
750744
state
751745
.metadata

rs/execution_environment/src/scheduler/tests.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ use ic00::{
1111
use ic_base_types::PrincipalId;
1212
use ic_config::{
1313
execution_environment::Config as HypervisorConfig,
14-
subnet_config::{CyclesAccountManagerConfig, SchedulerConfig, SubnetConfig, SubnetConfigs},
14+
subnet_config::{CyclesAccountManagerConfig, SchedulerConfig, SubnetConfig},
1515
};
1616
use ic_embedders::wasmtime_embedder::system_api_complexity::{cpu, overhead};
1717
use ic_error_types::RejectCode;
@@ -78,7 +78,7 @@ fn complexity_env(
7878
system_calls_per_message,
7979
}: SystemCallLimits,
8080
) -> StateMachine {
81-
let subnet_config = SubnetConfigs::default().own_subnet_config(subnet_type);
81+
let subnet_config = SubnetConfig::new(subnet_type);
8282
let performance_counter_complexity = cpu::PERFORMANCE_COUNTER.get() as u64;
8383
StateMachineBuilder::new()
8484
.with_subnet_type(subnet_type)
@@ -4342,7 +4342,7 @@ fn scheduler_resets_accumulated_priorities() {
43424342
// There must twice more canisters than the scheduler cores
43434343
let num_canisters = scheduler_cores * 2;
43444344

4345-
let subnet_config = SubnetConfigs::default().own_subnet_config(SubnetType::Application);
4345+
let subnet_config = SubnetConfig::new(SubnetType::Application);
43464346
let mut test = SchedulerTestBuilder::new()
43474347
.with_scheduler_config(SchedulerConfig {
43484348
scheduler_cores,

rs/execution_environment/tests/canister_history.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use ic_config::{execution_environment::Config as HypervisorConfig, subnet_config::SubnetConfigs};
1+
use ic_config::{execution_environment::Config as HypervisorConfig, subnet_config::SubnetConfig};
22
use ic_crypto_sha::Sha256;
33
use ic_ic00_types::CanisterInstallMode::{Install, Reinstall, Upgrade};
44
use ic_ic00_types::{
@@ -64,7 +64,7 @@ fn test_setup(
6464
let test_canister_sha256 = hasher.finish();
6565

6666
// set up StateMachine
67-
let subnet_config = SubnetConfigs::default().own_subnet_config(subnet_type);
67+
let subnet_config = SubnetConfig::new(subnet_type);
6868
let env = StateMachine::new_with_config(StateMachineConfig::new(
6969
subnet_config,
7070
HypervisorConfig::default(),
@@ -589,7 +589,7 @@ fn canister_history_cleared_if_canister_out_of_cycles() {
589589
));
590590

591591
// drain cycle balance of test_canister to trigger code uninstall from system
592-
let subnet_config = SubnetConfigs::default().own_subnet_config(subnet_type);
592+
let subnet_config = SubnetConfig::new(subnet_type);
593593
let compute_percent_allocated_per_second_fee = subnet_config
594594
.cycles_account_manager_config
595595
.compute_percent_allocated_per_second_fee;

rs/execution_environment/tests/dts.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use ic_config::{
55
embedders::Config as EmbeddersConfig,
66
execution_environment::Config as HypervisorConfig,
77
flag_status::FlagStatus,
8-
subnet_config::{SchedulerConfig, SubnetConfig, SubnetConfigs},
8+
subnet_config::{SchedulerConfig, SubnetConfig},
99
};
1010
use ic_ic00_types::{
1111
CanisterIdRecord, CanisterSettingsArgsBuilder, EmptyBlob, InstallCodeArgs, Method, Payload,
@@ -109,7 +109,7 @@ fn dts_subnet_config(
109109
message_instruction_limit: NumInstructions,
110110
slice_instruction_limit: NumInstructions,
111111
) -> SubnetConfig {
112-
let subnet_config = SubnetConfigs::default().own_subnet_config(SubnetType::Application);
112+
let subnet_config = SubnetConfig::new(SubnetType::Application);
113113
SubnetConfig {
114114
scheduler_config: SchedulerConfig {
115115
max_instructions_per_install_code: message_instruction_limit,
@@ -151,7 +151,7 @@ fn dts_install_code_env(
151151
message_instruction_limit: NumInstructions,
152152
slice_instruction_limit: NumInstructions,
153153
) -> StateMachine {
154-
let subnet_config = SubnetConfigs::default().own_subnet_config(SubnetType::Application);
154+
let subnet_config = SubnetConfig::new(SubnetType::Application);
155155
StateMachine::new_with_config(StateMachineConfig::new(
156156
SubnetConfig {
157157
scheduler_config: SchedulerConfig {

rs/execution_environment/tests/execution_test.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use ic_config::{
22
execution_environment::Config as HypervisorConfig,
3-
subnet_config::{CyclesAccountManagerConfig, SubnetConfigs},
3+
subnet_config::{CyclesAccountManagerConfig, SubnetConfig},
44
};
55
use ic_ic00_types::CanisterSettingsArgsBuilder;
66
use ic_registry_subnet_type::SubnetType;
@@ -323,7 +323,7 @@ fn test_canister_stable_memory_upgrade_restart() {
323323
#[test]
324324
fn test_canister_out_of_cycles() {
325325
// Start a node with a config where all computation/storage is free.
326-
let mut subnet_config = SubnetConfigs::default().own_subnet_config(SubnetType::System);
326+
let mut subnet_config = SubnetConfig::new(SubnetType::System);
327327
let env = StateMachine::new_with_config(StateMachineConfig::new(
328328
subnet_config.clone(),
329329
HypervisorConfig::default(),
@@ -381,7 +381,7 @@ fn test_canister_out_of_cycles() {
381381

382382
#[test]
383383
fn canister_has_zero_balance_when_uninstalled_due_to_low_cycles() {
384-
let subnet_config = SubnetConfigs::default().own_subnet_config(SubnetType::Application);
384+
let subnet_config = SubnetConfig::new(SubnetType::Application);
385385
let compute_percent_allocated_per_second_fee = subnet_config
386386
.cycles_account_manager_config
387387
.compute_percent_allocated_per_second_fee;
@@ -623,7 +623,7 @@ fn can_query_cycle_balance_and_top_up_canisters() {
623623

624624
#[test]
625625
fn exceeding_memory_capacity_fails_when_memory_allocation_changes() {
626-
let subnet_config = SubnetConfigs::default().own_subnet_config(SubnetType::Application);
626+
let subnet_config = SubnetConfig::new(SubnetType::Application);
627627
let env = StateMachine::new_with_config(StateMachineConfig::new(
628628
subnet_config,
629629
HypervisorConfig {
@@ -686,7 +686,7 @@ fn assert_replied(result: Result<WasmResult, UserError>, expected: i64) {
686686

687687
#[test]
688688
fn exceeding_memory_capacity_fails_during_message_execution() {
689-
let subnet_config = SubnetConfigs::default().own_subnet_config(SubnetType::Application);
689+
let subnet_config = SubnetConfig::new(SubnetType::Application);
690690
let env = StateMachine::new_with_config(StateMachineConfig::new(
691691
subnet_config,
692692
HypervisorConfig {
@@ -745,7 +745,7 @@ fn exceeding_memory_capacity_fails_during_message_execution() {
745745

746746
#[test]
747747
fn max_canister_memory_respected_even_when_no_memory_allocation_is_set() {
748-
let subnet_config = SubnetConfigs::default().own_subnet_config(SubnetType::Application);
748+
let subnet_config = SubnetConfig::new(SubnetType::Application);
749749
let env = StateMachine::new_with_config(StateMachineConfig::new(
750750
subnet_config,
751751
HypervisorConfig {

0 commit comments

Comments
 (0)