Skip to content

Commit

Permalink
refactor: Move log_dirty_pages to the module where it's used (#3957)
Browse files Browse the repository at this point in the history
Small refactoring that moves the function closer to where it's used. The
top level `execution_environment.rs` is quite big already, anything that
does not need to be there, better not be there.
  • Loading branch information
dsarlis authored Feb 14, 2025
1 parent 882e7af commit 4627f33
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 22 deletions.
4 changes: 2 additions & 2 deletions rs/execution_environment/src/execution/call_or_task.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@

use crate::execution::common::{
action_to_response, apply_canister_state_changes, finish_call_with_error,
ingress_status_with_processing_state, update_round_limits, validate_message,
ingress_status_with_processing_state, log_dirty_pages, update_round_limits, validate_message,
wasm_result_to_query_response,
};
use crate::execution_environment::{
log_dirty_pages, ExecuteMessageResult, PausedExecution, RoundContext, RoundLimits,
ExecuteMessageResult, PausedExecution, RoundContext, RoundLimits,
};
use crate::metrics::CallTreeMetrics;
use ic_base_types::CanisterId;
Expand Down
17 changes: 16 additions & 1 deletion rs/execution_environment/src/execution/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use ic_error_types::{ErrorCode, RejectCode, UserError};
use ic_interfaces::execution_environment::{
HypervisorError, HypervisorResult, SubnetAvailableMemory, WasmExecutionOutput,
};
use ic_logger::{error, fatal, warn, ReplicaLogger};
use ic_logger::{error, fatal, info, warn, ReplicaLogger};
use ic_management_canister_types_private::CanisterStatusType;
use ic_registry_subnet_type::SubnetType;
use ic_replicated_state::{
Expand Down Expand Up @@ -584,6 +584,21 @@ pub(crate) fn finish_call_with_error(
}
}

/// Helper method for logging dirty pages.
pub fn log_dirty_pages(
log: &ReplicaLogger,
canister_id: &CanisterId,
method_name: &str,
dirty_pages: usize,
instructions: NumInstructions,
) {
let output_message = format!(
"Executed {canister_id}::{method_name}: dirty_4kb_pages = {dirty_pages}, instructions = {instructions}"
);
info!(log, "{}", output_message.as_str());
eprintln!("{output_message}");
}

#[cfg(test)]
mod test {
use super::wasm_result_to_query_response;
Expand Down
3 changes: 2 additions & 1 deletion rs/execution_environment/src/execution/install_code.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

use std::path::{Path, PathBuf};

use crate::execution::common::log_dirty_pages;
use ic_base_types::{CanisterId, NumBytes, PrincipalId};
use ic_config::flag_status::FlagStatus;
use ic_embedders::wasm_executor::{CanisterStateChanges, ExecutionStateChanges};
Expand Down Expand Up @@ -30,7 +31,7 @@ use crate::{
CanisterManagerError, CanisterMgrConfig, DtsInstallCodeResult, InstallCodeResult,
},
canister_settings::{validate_canister_settings, CanisterSettings},
execution_environment::{log_dirty_pages, RoundContext},
execution_environment::RoundContext,
CompilationCostHandling, RoundLimits,
};
use ic_replicated_state::canister_state::execution_state::WasmExecutionMode;
Expand Down
5 changes: 2 additions & 3 deletions rs/execution_environment/src/execution/response.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,10 @@ use ic_utils_thread::deallocator_thread::DeallocationSender;
use ic_wasm_types::WasmEngineError::FailedToApplySystemChanges;

use crate::execution::common::{
self, action_to_response, apply_canister_state_changes, update_round_limits,
self, action_to_response, apply_canister_state_changes, log_dirty_pages, update_round_limits,
};
use crate::execution_environment::{
log_dirty_pages, ExecuteMessageResult, ExecutionResponse, PausedExecution, RoundContext,
RoundLimits,
ExecuteMessageResult, ExecutionResponse, PausedExecution, RoundContext, RoundLimits,
};
use crate::metrics::CallTreeMetrics;
use ic_config::flag_status::FlagStatus;
Expand Down
15 changes: 0 additions & 15 deletions rs/execution_environment/src/execution_environment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,21 +203,6 @@ pub fn as_num_instructions(a: RoundInstructions) -> NumInstructions {
NumInstructions::from(u64::try_from(a.get()).unwrap_or(0))
}

/// Helper method for logging dirty pages.
pub fn log_dirty_pages(
log: &ReplicaLogger,
canister_id: &CanisterId,
method_name: &str,
dirty_pages: usize,
instructions: NumInstructions,
) {
let output_message = format!(
"Executed {canister_id}::{method_name}: dirty_4kb_pages = {dirty_pages}, instructions = {instructions}"
);
info!(log, "{}", output_message.as_str());
eprintln!("{output_message}");
}

/// Contains limits (or budget) for various resources that affect duration of
/// a round such as
/// - executed instructions,
Expand Down

0 comments on commit 4627f33

Please sign in to comment.