Skip to content

Refactor the logging system for fewer allocations #9261

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 27, 2013
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 14 additions & 11 deletions src/libstd/logging.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@

//! Logging

use fmt;
use option::*;
use os;
use rt;
use rt::logging::{Logger, StdErrLogger};
use send_str::SendStrOwned;

/// Turns on logging to stdout globally
pub fn console_on() {
Expand All @@ -37,7 +37,17 @@ pub fn console_off() {
rt::logging::console_off();
}

fn newsched_log_str(msg: ~str) {
#[cfg(stage0)]
#[doc(hidden)]
pub fn log(_level: u32, s: ~str) {
// this is a terrible approximation, but it gets the job done (for stage0 at
// least)
::io::println(s);
}

#[allow(missing_doc)]
#[cfg(not(stage0))]
pub fn log(_level: u32, args: &fmt::Arguments) {
use rt::task::Task;
use rt::local::Local;

Expand All @@ -46,20 +56,13 @@ fn newsched_log_str(msg: ~str) {
match optional_task {
Some(local) => {
// Use the available logger
(*local).logger.log(SendStrOwned(msg));
(*local).logger.log(args);
}
None => {
// There is no logger anywhere, just write to stderr
let mut logger = StdErrLogger;
logger.log(SendStrOwned(msg));
logger.log(args);
}
}
}
}

// XXX: This will change soon to not require an allocation. This is an unstable
// api which should not be used outside of the macros in ext/expand.
#[doc(hidden)]
pub fn log(_level: u32, msg: ~str) {
newsched_log_str(msg);
}
53 changes: 22 additions & 31 deletions src/libstd/rt/logging.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,12 @@
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

use fmt;
use from_str::from_str;
use libc::{uintptr_t, exit, STDERR_FILENO};
use libc::{uintptr_t, exit};
use option::{Some, None, Option};
use rt;
use rt::util::dumb_println;
use rt::crate_map::{ModEntry, iter_crate_map};
use rt::crate_map::get_crate_map;
Expand All @@ -18,7 +21,6 @@ use str::raw::from_c_str;
use u32;
use vec::ImmutableVector;
use cast::transmute;
use send_str::{SendStr, SendStrOwned, SendStrStatic};

struct LogDirective {
name: Option<~str>,
Expand Down Expand Up @@ -171,44 +173,33 @@ fn update_log_settings(crate_map: *u8, settings: ~str) {
}

pub trait Logger {
fn log(&mut self, msg: SendStr);
fn log(&mut self, args: &fmt::Arguments);
}

pub struct StdErrLogger;

impl Logger for StdErrLogger {
fn log(&mut self, msg: SendStr) {
use io::{Writer, WriterUtil};

if !should_log_console() {
return;
fn log(&mut self, args: &fmt::Arguments) {
if should_log_console() {
fmt::write(self as &mut rt::io::Writer, args);
}
}
}

let s: &str = match msg {
SendStrOwned(ref s) => {
let slc: &str = *s;
slc
},
SendStrStatic(s) => s,
};

// Truncate the string
let buf_bytes = 2048;
if s.len() > buf_bytes {
let s = s.slice(0, buf_bytes) + "[...]";
print(s);
} else {
print(s)
};

fn print(s: &str) {
let dbg = STDERR_FILENO as ::io::fd_t;
dbg.write_str(s);
dbg.write_str("\n");
dbg.flush();
}
impl rt::io::Writer for StdErrLogger {
fn write(&mut self, buf: &[u8]) {
// Nothing like swapping between I/O implementations! In theory this
// could use the libuv bindings for writing to file descriptors, but
// that may not necessarily be desirable because logging should work
// outside of the uv loop. (modify with caution)
use io::Writer;
let dbg = ::libc::STDERR_FILENO as ::io::fd_t;
dbg.write(buf);
}

fn flush(&mut self) {}
}

/// Configure logging by traversing the crate map and setting the
/// per-module global logging flags based on the logging spec
pub fn init() {
Expand Down
1 change: 1 addition & 0 deletions src/libstd/std.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,4 +224,5 @@ mod std {
pub use os;
pub use fmt;
pub use to_bytes;
pub use logging;
}
51 changes: 35 additions & 16 deletions src/libstd/sys.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,37 +125,56 @@ impl FailWithCause for &'static str {
}
}

// FIXME #4427: Temporary until rt::rt_fail_ goes away
// This stage0 version is incredibly wrong.
#[cfg(stage0)]
pub fn begin_unwind_(msg: *c_char, file: *c_char, line: size_t) -> ! {
use option::{Some, None};
use rt::in_green_task_context;
use rt::task::Task;
use rt::local::Local;
use rt::logging::Logger;
use send_str::SendStrOwned;
use str::Str;

unsafe {
let msg = str::raw::from_c_str(msg);
let file = str::raw::from_c_str(file);
if in_green_task_context() {
rterrln!("task failed at '%s', %s:%i", msg, file, line as int);
} else {
rterrln!("failed in non-task context at '%s', %s:%i",
msg, file, line as int);
}

let task: *mut Task = Local::unsafe_borrow();
if (*task).unwinder.unwinding {
rtabort!("unwinding again");
}
(*task).unwinder.begin_unwind();
}
}

// FIXME #4427: Temporary until rt::rt_fail_ goes away
#[cfg(not(stage0))]
pub fn begin_unwind_(msg: *c_char, file: *c_char, line: size_t) -> ! {
use rt::in_green_task_context;
use rt::task::Task;
use rt::local::Local;
use rt::logging::Logger;
use str::Str;

unsafe {
// XXX: Bad re-allocations. fail! needs some refactoring
let msg = str::raw::from_c_str(msg);
let file = str::raw::from_c_str(file);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Loosely related to this change: these actually allocate a new buffer, would it be possible for begin_unwind_ to be passed Rust strings and uints rather than C ones?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would be a nice change actually, it seems fairly nicely separable from this change, however, so I think I'd attempt to do it in a later commit.


// XXX: Logging doesn't work correctly in non-task context because it
// invokes the local heap
if in_green_task_context() {
// XXX: Logging doesn't work here - the check to call the log
// function never passes - so calling the log function directly.
// Be careful not to allocate in this block, if we're failing we may
// have been failing due to a lack of memory in the first place...
do Local::borrow |task: &mut Task| {
let msg = match task.name {
Some(ref name) =>
fmt!("task '%s' failed at '%s', %s:%i",
name.as_slice(), msg, file, line as int),
None =>
fmt!("task <unnamed> failed at '%s', %s:%i",
msg, file, line as int)
};

task.logger.log(SendStrOwned(msg));
let n = task.name.map(|n| n.as_slice()).unwrap_or("<unnamed>");
format_args!(|args| { task.logger.log(args) },
"task '{}' failed at '{}', {}:{}",
n, msg.as_slice(), file.as_slice(), line);
}
} else {
rterrln!("failed in non-task context at '%s', %s:%i",
Expand Down
12 changes: 9 additions & 3 deletions src/libsyntax/ext/expand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -813,13 +813,17 @@ pub fn std_macros() -> @str {
($lvl:expr, $arg:expr) => ({
let lvl = $lvl;
if lvl <= __log_level() {
::std::logging::log(lvl, fmt!(\"%?\", $arg))
format_args!(|args| {
::std::logging::log(lvl, args)
}, \"{}\", fmt!(\"%?\", $arg))
}
});
($lvl:expr, $($arg:expr),+) => ({
let lvl = $lvl;
if lvl <= __log_level() {
::std::logging::log(lvl, fmt!($($arg),+))
format_args!(|args| {
::std::logging::log(lvl, args)
}, \"{}\", fmt!($($arg),+))
}
})
)
Expand All @@ -834,7 +838,9 @@ pub fn std_macros() -> @str {
($lvl:expr, $($arg:tt)+) => ({
let lvl = $lvl;
if lvl <= __log_level() {
::std::logging::log(lvl, format!($($arg)+))
format_args!(|args| {
::std::logging::log(lvl, args)
}, $($arg)+)
}
})
)
Expand Down
3 changes: 1 addition & 2 deletions src/test/compile-fail/issue-2823.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,5 @@ impl Drop for C {

fn main() {
let c = C{ x: 2};
let d = c.clone(); //~ ERROR does not implement any method in scope
error!("%?", d.x);
let _d = c.clone(); //~ ERROR does not implement any method in scope
}
74 changes: 0 additions & 74 deletions src/test/run-fail/rt-log-trunc.rs

This file was deleted.