log icon indicating copy to clipboard operation
log copied to clipboard

Is it necessary to use Ordering::SeqCst in the log! ??

Open relufi opened this issue 3 years ago • 4 comments

pub fn logger() -> &'static dyn Log {
   // Is it necessary to use Ordering::SeqCst in log!
    if STATE.load(Ordering::SeqCst) != INITIALIZED {
        static NOP: NopLogger = NopLogger;
        &NOP
    } else {
        unsafe { LOGGER }
    }
}

See the implementation of std::lazy::SyncOnceCell or parking_lot::Once

relufi avatar Apr 29 '22 03:04 relufi

I tried to use a more relaxed order and wrote a test https://github.com/relufi/log/blob/use-relaxed-memory-barriers/src/lib.rs

However, when I tried to change all the memory order to Relaxed and tested on the arm platform, there was no visibility problem either.

relufi avatar Apr 29 '22 03:04 relufi

I think the ordering there could probably be downgraded so long as we always see the store of STATE after the store of LOGGER. The orderings on initialization are also pretty conservative, but it's always hard to tell whether that's because they're necessary or because they haven't been optimized.

KodrAus avatar May 04 '22 01:05 KodrAus

Reference std::sync::Once SeqCst is not necessary, I am concerned that using SeqCst in every log! will affect performance (log! is too common and SeqCst is too expensive on some platforms). If you're worried about problems you can just introduce std::sync::Once or parking_lot::Once to avoid them.

relufi avatar May 04 '22 03:05 relufi

I am using a translation tool, so please bear with me if I make any mistakes.

relufi avatar May 04 '22 03:05 relufi