Skip to content

Commit

Permalink
refactor: Provide a more efficient `ReplicatedState::message_memory_t…
Browse files Browse the repository at this point in the history
…aken()` method
  • Loading branch information
alin-at-dfinity committed Feb 6, 2024
1 parent 17ce8f7 commit a4895b9
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 9 deletions.
11 changes: 10 additions & 1 deletion rs/execution_environment/src/execution_environment.rs
Expand Up @@ -389,7 +389,7 @@ impl ExecutionEnvironment {
&self.metrics.canister_not_found_error
}

/// Look up the current amount of memory available on the subnet.
/// Computes the current amount of memory available on the subnet.
pub fn subnet_available_memory(&self, state: &ReplicatedState) -> SubnetAvailableMemory {
let memory_taken = state.memory_taken();
SubnetAvailableMemory::new(
Expand All @@ -405,6 +405,15 @@ impl ExecutionEnvironment {
)
}

/// Computes the current amount of message memory available on the subnet.
///
/// This is a more efficient alternative to `memory_taken()` for cases when only
/// the message memory usage is necessary.
pub fn subnet_available_message_memory(&self, state: &ReplicatedState) -> i64 {
self.config.subnet_message_memory_capacity.get() as i64
- state.message_memory_taken().get() as i64
}

/// Executes a replicated message sent to a subnet.
/// Returns the new replicated state and the number of left instructions.
#[allow(clippy::cognitive_complexity)]
Expand Down
5 changes: 1 addition & 4 deletions rs/execution_environment/src/scheduler.rs
Expand Up @@ -1160,10 +1160,7 @@ impl SchedulerImpl {
/// through message routing.
pub fn induct_messages_on_same_subnet(&self, state: &mut ReplicatedState) {
// Compute subnet available memory *before* taking out the canisters.
let mut subnet_available_memory = self
.exec_env
.subnet_available_memory(state)
.get_message_memory();
let mut subnet_available_memory = self.exec_env.subnet_available_message_memory(state);

let mut canisters = state.take_canister_states();

Expand Down
4 changes: 1 addition & 3 deletions rs/messaging/src/routing/stream_handler.rs
Expand Up @@ -812,9 +812,7 @@ impl StreamHandlerImpl {
/// Computes the subnet's available message memory, as the difference
/// between the subnet's message memory capacity and its current usage.
fn subnet_available_memory(&self, state: &ReplicatedState) -> i64 {

This comment has been minimized.

Copy link
@AleDema

AleDema Feb 11, 2024

Shouldn't this be renamed to subnet_available_message_memory to better reflect its purpose? It is functionally the same as the new method added to ExecutionEnvironment exception made for the source of subnet_message_memory_capacity.

This comment has been minimized.

Copy link
@alin-at-dfinity

alin-at-dfinity Feb 11, 2024

Author Contributor

This is a method of StreamHandler, in crate ic_messaging. We only ever deal with message memory within the crate, so it would be sort of redundant.

If you find it potentially confusing, it can definitely be renamed.

This comment has been minimized.

Copy link
@AleDema

AleDema Feb 14, 2024

That makes sense, thanks for the clarification.

let memory_taken = state.memory_taken();
let message_memory_taken = memory_taken.messages();
self.subnet_message_memory_capacity.get() as i64 - message_memory_taken.get() as i64
self.subnet_message_memory_capacity.get() as i64 - state.message_memory_taken().get() as i64
}

/// Observes "time in backlog" (since learning about their existence from
Expand Down
16 changes: 15 additions & 1 deletion rs/replicated_state/src/replicated_state.rs
Expand Up @@ -588,7 +588,7 @@ impl ReplicatedState {
.sum()
}

/// Returns the memory taken by different types of memory resources.
/// Computes the memory taken by different types of memory resources.
pub fn memory_taken(&self) -> MemoryTaken {
let (
raw_memory_taken,
Expand Down Expand Up @@ -633,6 +633,20 @@ impl ReplicatedState {
}
}

/// Computes the memory taken by messages.
///
/// This is a more efficient alternative to `memory_taken()` for cases when only
/// the message memory usage is necessary.
pub fn message_memory_taken(&self) -> NumBytes {
let canisters_memory_usage: NumBytes = self
.canisters_iter()
.map(|canister| canister.system_state.message_memory_usage())
.sum();
let subnet_memory_usage = (self.subnet_queues.memory_usage() as u64).into();

canisters_memory_usage + subnet_memory_usage
}

/// Returns the total memory taken by the ingress history in bytes.
pub fn total_ingress_memory_taken(&self) -> NumBytes {
self.metadata.ingress_history.memory_usage()
Expand Down
8 changes: 8 additions & 0 deletions rs/replicated_state/tests/replicated_state.rs
Expand Up @@ -158,6 +158,10 @@ impl ReplicatedStateFixture {
self.state.memory_taken()
}

fn message_memory_taken(&self) -> NumBytes {
self.state.message_memory_taken()
}

fn remote_subnet_input_schedule(&self, canister: &CanisterId) -> &VecDeque<CanisterId> {
self.state
.canister_state(canister)
Expand Down Expand Up @@ -189,6 +193,10 @@ fn assert_message_memory_taken(queues_memory_usage: usize, fixture: &ReplicatedS
queues_memory_usage as u64,
fixture.memory_taken().messages().get()
);
assert_eq!(
queues_memory_usage as u64,
fixture.message_memory_taken().get()
);
}

fn assert_canister_history_memory_taken(
Expand Down

0 comments on commit a4895b9

Please sign in to comment.