Skip to content

Commit

Permalink
Make env_logger an optional dependency (#1226)
Browse files Browse the repository at this point in the history
Now the built-in `env_logger` is guarded behind a Cargo feature
"builtin_env_logger". It is a default feature, but can be disabled in
Cargo.toml by setting `dependencies.mmtk.default-features = false`. In
this way, VM bindings that want to implement its own logger can remove
the `env_logger` crate from its dependencies.

Fixes: #744
  • Loading branch information
wks authored Nov 6, 2024
1 parent 3830168 commit 3575521
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 15 deletions.
9 changes: 7 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ crossbeam = "0.8.1"
delegate = "0.12.0"
downcast-rs = "1.1.1"
enum-map = "2.4.2"
env_logger = "0.11.3"
env_logger = { version = "0.11.3", optional = true }
is-terminal = "0.4.7"
itertools = "0.12.0"
jemalloc-sys = { version = "0.5.3", features = ["disable_initial_exec_tls"], optional = true }
Expand Down Expand Up @@ -66,7 +66,12 @@ name = "main"
harness = false

[features]
default = []
default = ["builtin_env_logger"]

# Built-in env_logger. This feature is enabled by default.
# The user can disable this default feature to remove `env_logger` from the dependencies.
# See `crate::util::logger` for more details.
builtin_env_logger = ["dep:env_logger"]

# This feature is only supported on x86-64 for now
# It's manually added to CI scripts
Expand Down
11 changes: 4 additions & 7 deletions src/memory_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,9 @@ use crate::vm::VMBinding;
/// supported. Currently we assume a binding will only need one MMTk instance. Note that GC is enabled by default and the binding should
/// implement `VMCollection::is_collection_enabled()` if it requires that the GC should be disabled at a particular time.
///
/// Note that this method will attempt to initialize a logger. If the VM would like to use its own logger, it should initialize the logger before calling this method.
/// This method will attempt to initialize the built-in `env_logger` if the Cargo feature "builtin_env_logger" is enabled (by default).
/// If the VM would like to use its own logger, it should disable the default feature "builtin_env_logger" in `Cargo.toml`.
///
/// Note that, to allow MMTk to do GC properly, `initialize_collection()` needs to be called after this call when
/// the VM's thread system is ready to spawn GC workers.
///
Expand All @@ -51,12 +53,7 @@ use crate::vm::VMBinding;
/// Arguments:
/// * `builder`: The reference to a MMTk builder.
pub fn mmtk_init<VM: VMBinding>(builder: &MMTKBuilder) -> Box<MMTK<VM>> {
match crate::util::logger::try_init() {
Ok(_) => debug!("MMTk initialized the logger."),
Err(_) => debug!(
"MMTk failed to initialize the logger. Possibly a logger has been initialized by user."
),
}
crate::util::logger::try_init();
#[cfg(all(feature = "perf_counter", target_os = "linux"))]
{
use std::fs::File;
Expand Down
41 changes: 35 additions & 6 deletions src/util/logger.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,38 @@
use log::{self, SetLoggerError};
//! This module provides a built-in logger implementation.
//!
//! The built-in logger implementation uses the `env_logger` crate. It is enabled by the Cargo
//! feature "builtin_env_logger" which is enabled by default. When enabled, it will be initialized
//! in [`crate::memory_manager::mmtk_init`] and will show logs of levels INFO or lower (the lower,
//! the more important).
//!
//! This provides convenient out-of-the-box experience for binding developers so that they can see
//! logs when using MMTk without configuration, and can easily configure log levels from environment
//! variables. Some bindings may wish to choose a different implementation, or implement their own
//! logging implementations to integrate with the existing logging frameworks of their VMs. In such
//! cases, the binding can disable the Cargo feature "builtin_env_logger" and register their own
//! implementations with the `log` crate.

/// Attempt to init a env_logger for MMTk.
pub fn try_init() -> Result<(), SetLoggerError> {
env_logger::try_init_from_env(
// By default, use info level logging.
env_logger::Env::default().filter_or(env_logger::DEFAULT_FILTER_ENV, "info"),
)
/// Does nothing if the "builtin_env_logger" feature is disabled.
pub(crate) fn try_init() {
cfg_if::cfg_if! {
if #[cfg(feature = "builtin_env_logger")] {
let result = env_logger::try_init_from_env(
// By default, show info level logging.
env_logger::Env::default().filter_or(env_logger::DEFAULT_FILTER_ENV, "info"),
);

match result {
Ok(()) => {
debug!("MMTk initialized the logger.");
}
Err(e) => {
// Currently `log::SetLoggerError` can only be raised for one reason: the logger has already been initialized.
debug!("MMTk failed to initialize the built-in env_logger: {e}");
}
}
} else {
debug!("MMTk didn't initialize the built-in env_logger. The Cargo feature \"builtin_env_logger\" is not enabled.");
}
}
}

0 comments on commit 3575521

Please sign in to comment.