Browse Source

Improve logging in multiple ways.

- Let `Writer` hold the lock.

- Rename `Writer` to `LogWriter`

- Fix warnings in tests.

- Remove forgotten file.
ticki 8 years ago
parent
commit
fd7a7bc78c
7 changed files with 42 additions and 89 deletions
  1. 4 3
      Cargo.toml
  2. 0 66
      src/breaker.rs
  3. 11 0
      src/lazy_init.rs
  4. 1 1
      src/lib.rs
  5. 1 4
      src/log.rs
  6. 24 14
      src/write.rs
  7. 1 1
      tests/arc.rs

+ 4 - 3
Cargo.toml

@@ -38,9 +38,10 @@ default = ["allocator", "clippy"]
 alloc_id = []
 allocator = []
 debug_tools = []
-libc_write = []
-log = ["libc_write", "alloc_id"]
+log = ["write", "alloc_id"]
+no_log_lock = ["log"]
 security = []
-testing = ["log", "libc_write", "debug_tools"]
+testing = ["log", "write", "debug_tools"]
 unsafe_no_brk_lock = []
 unsafe_no_mutex_lock = []
+write = []

+ 0 - 66
src/breaker.rs

@@ -1,66 +0,0 @@
-use prelude::*;
-
-trait Breaker {
-    /// Allocate _fresh_ space.
-    ///
-    /// "Fresh" means that the space is allocated through the breaker.
-    ///
-    /// The returned pointer is guaranteed to be aligned to `align`.
-    fn alloc_fresh(pool: &mut Vec<Block>, size: usize, align: usize) -> Block;
-}
-
-/// Canonicalize a BRK request.
-///
-/// Syscalls can be expensive, which is why we would rather accquire more memory than necessary,
-/// than having many syscalls acquiring memory stubs. Memory stubs are small blocks of memory,
-/// which are essentially useless until merge with another block.
-///
-/// To avoid many syscalls and accumulating memory stubs, we BRK a little more memory than
-/// necessary. This function calculate the memory to be BRK'd based on the necessary memory.
-///
-/// The return value is always greater than or equals to the argument.
-#[inline]
-fn canonicalize_brk(min: usize) -> usize {
-    /// The BRK multiplier.
-    ///
-    /// The factor determining the linear dependence between the minimum segment, and the acquired
-    /// segment.
-    const BRK_MULTIPLIER: usize = 2;
-    /// The minimum size to be BRK'd.
-    const BRK_MIN: usize = 1024;
-    /// The maximal amount of _extra_ elements.
-    const BRK_MAX_EXTRA: usize = 4 * 65536;
-
-    let res = cmp::max(BRK_MIN, min.saturating_add(cmp::min(BRK_MULTIPLIER * min, BRK_MAX_EXTRA)));
-
-    // Make some handy assertions.
-    debug_assert!(res >= min, "Canonicalized BRK space is smaller than the one requested.");
-
-    res
-}
-
-struct Sbrk;
-
-impl Breaker for Sbrk {
-    fn ocbtain(size: usize, align: usize) -> (Block, Block, Block) {
-        // Logging.
-        log!(self.pool;self.pool.len(), "Obtaining a block of size, {}, and alignment {}.", size, align);
-
-        // Calculate the canonical size (extra space is allocated to limit the number of system calls).
-        let brk_size = canonicalize_brk(size) + align;
-
-        // Use SBRK to allocate extra data segment. The alignment is used as precursor for our
-        // allocated block. This ensures that it is properly memory aligned to the requested value.
-        let (alignment_block, rest) = Block::brk(brk_size).align(align)
-            .expect("Unable to align block to the requested align.");
-
-        // Split the block to leave the excessive space.
-        let (res, excessive) = rest.split(size);
-
-        // Make some assertions.
-        debug_assert!(res.aligned_to(align), "Alignment failed.");
-        debug_assert!(res.size() + alignment_block.size() + excessive.size() == brk_size, "BRK memory leak.");
-
-        (alignment_block, res, excessive)
-    }
-}

+ 11 - 0
src/lazy_init.rs

@@ -103,4 +103,15 @@ mod test {
         lazy.get();
         assert!(is_called.get());
     }
+
+    #[test]
+    #[should_panic]
+    #[allow(unused_assignments)]
+    fn test_unreachable() {
+        let mut a = LazyInit::new(|| 2);
+
+        a = LazyInit::unreachable();
+
+        a.get();
+    }
 }

+ 1 - 1
src/lib.rs

@@ -25,7 +25,7 @@
 #[macro_use]
 extern crate unborrow;
 
-#[cfg(feature = "libc_write")]
+#[cfg(feature = "write")]
 #[macro_use]
 mod write;
 #[macro_use]

+ 1 - 4
src/log.rs

@@ -24,11 +24,8 @@ macro_rules! log {
             use {write, log};
             use log::internal::IntoCursor;
 
-            // To avoid cluttering the lines, we acquire a lock.
-            let _lock = write::LINE_LOCK.lock();
-
             // Print the pool state.
-            let mut log = write::Writer::new();
+            let mut log = write::LogWriter::new();
             let _ = write!(log, "({:2})   {:10?} : ", $bk.id, log::internal::BlockLogger {
                 cur: $cur.clone().into_cursor(),
                 blocks: &$bk.pool,

+ 24 - 14
src/write.rs

@@ -1,4 +1,4 @@
-//! Direct libc-based write for internal debugging.
+//! Direct shim-based write for internal debugging.
 //!
 //! This will replace the assertion macros to avoid deadlocks in panics, by utilizing a
 //! non-allocating writing primitive.
@@ -7,26 +7,39 @@ use prelude::*;
 
 use core::fmt;
 
-use sys;
+use {sys, sync};
 
-/// The line lock.
+/// The log lock.
 ///
-/// This lock is used to avoid bungling and intertwining lines.
-pub static LINE_LOCK: Mutex<()> = Mutex::new(());
+/// This lock is used to avoid bungling and intertwining the log.
+#[cfg(not(feature = "no_log_lock"))]
+pub static LOG_LOCK: Mutex<()> = Mutex::new(());
 
 /// A log writer.
 ///
 /// This writes to  `sys::log`.
-pub struct Writer;
+pub struct LogWriter {
+    /// The inner lock.
+    #[cfg(not(feature = "no_log_lock"))]
+    _lock: sync::MutexGuard<'static, ()>,
+}
 
-impl Writer {
+impl LogWriter {
     /// Standard error output.
-    pub fn new() -> Writer {
-        Writer
+    pub fn new() -> LogWriter {
+        #[cfg(feature = "no_log_lock")]
+        {
+            LogWriter {}
+        }
+
+        #[cfg(not(feature = "no_log_lock"))]
+        LogWriter {
+            _lock: LOG_LOCK.lock(),
+        }
     }
 }
 
-impl fmt::Write for Writer {
+impl fmt::Write for LogWriter {
     fn write_str(&mut self, s: &str) -> fmt::Result {
         if sys::log(s).is_err() {
             Err(fmt::Error)
@@ -50,10 +63,7 @@ macro_rules! assert {
         use core::fmt::Write;
 
         if !$e {
-            // To avoid cluttering the lines, we acquire a lock.
-            let _lock = write::LINE_LOCK.lock();
-
-            let mut log = write::Writer::new();
+            let mut log = write::LogWriter::new();
             let _ = write!(log, "assertion failed at {}:{}: `{}` - ", file!(),
                            line!(), stringify!($e));
             let _ = writeln!(log, $( $arg ),*);

+ 1 - 1
tests/arc.rs

@@ -13,7 +13,7 @@ fn main() {
         let child_numbers = shared_numbers.clone();
 
         thread::spawn(move || {
-            let local_numbers = &child_numbers[..];
+            let _local_numbers = &child_numbers[..];
 
             // Work with the local numbers
         });