matklad / once_cell

Rust library for single assignment cells and lazy statics without macros
Apache License 2.0
1.84k stars 110 forks source link

add sync::OnceCell::get_unchecked #60

Closed matklad closed 5 years ago

matklad commented 5 years ago

Note that we intentionally don't add unsync::OnceCell::get_unchecked:

matklad commented 5 years ago

I believe this is correct semantically. However, I am worry about social consequences here: by having this method, we create a risk of this method being misused. It's unclear to me if potential performance benefits (which are real, see #59) outweigh the risk of misuse.

matklad commented 5 years ago

I did a quick experiment to check if std::hint::unrecheable_unchecked can be used on the call site to eliminate the load. It looks like it doesn't help :(

#[inline(never)]
#[no_mangle]
pub fn quux_checked(cell: &OnceCell<String>) -> &String {
    cell.get().unwrap()
}

#[inline(never)]
#[no_mangle]
pub fn quux_unchecked(cell: &OnceCell<String>) -> &String {
    unsafe { cell.get_unchecked() }
}

#[inline(never)]
#[no_mangle]
pub fn quux_manual_unchecked(cell: &OnceCell<String>) -> &String {
    unsafe {
        match cell.get() {
            Some(value) => value,
            None => std::hint::unreachable_unchecked(),
        }
    }
}
0000000000000000 <quux_checked>:
   0:   50                      push   rax
   1:   48 8b 0f                mov    rcx,QWORD PTR [rdi]
   4:   48 83 f9 02             cmp    rcx,0x2
   8:   75 09                   jne    13 <quux_checked+0x13>
   a:   48 89 f8                mov    rax,rdi
   d:   48 83 c0 08             add    rax,0x8
  11:   59                      pop    rcx
  12:   c3                      ret    
  13:   48 8d 3d 00 00 00 00    lea    rdi,[rip+0x0]        # 1a <quux_checked+0x1a>
  1a:   ff 15 00 00 00 00       call   QWORD PTR [rip+0x0]        # 20 <quux_checked+0x20>
  20:   0f 0b                   ud2    

Disassembly of section .text.quux_unchecked:

0000000000000000 <quux_unchecked>:
   0:   48 8d 47 08             lea    rax,[rdi+0x8]
   4:   c3                      ret    

Disassembly of section .text.quux_manual_unchecked:

0000000000000000 <quux_manual_unchecked>:
   0:   48 8b 0f                mov    rcx,QWORD PTR [rdi]
   3:   48 83 c7 08             add    rdi,0x8
   7:   31 c0                   xor    eax,eax
   9:   48 83 f9 02             cmp    rcx,0x2
   d:   48 0f 44 c7             cmove  rax,rdi
  11:   c3                      ret    
matklad commented 5 years ago

bors r+

CI failied due to miri failure on nightly, let's see if its any better now...

bors[bot] commented 5 years ago

Build failed

matklad commented 5 years ago

merging manually, as miri failure seems unrelated.

I am still not sure if it is definitelly worth it, but, becuase the method is usnafe and makes sense, I feel like it's ok to merge this.