Re: [pve-devel] [pbs-devel] [PATCH proxmox-backup-qemu 8/9] add shared_cache module

[Wolfgang Bumiller <w.bumiller@proxmox.com>]

> On 06/07/2021 10:03 AM Stefan Reiter <s.reiter@proxmox.com> wrote:
> 
>  
> On 6/4/21 2:16 PM, Wolfgang Bumiller wrote:
> > On Wed, Jun 02, 2021 at 04:38:32PM +0200, Stefan Reiter wrote:
> >> Provides a shared AsyncLruCache of 256MB (w/ 4MB chunks) that can be
> >> used by multiple readers at the same time. It is dropped once no more
> >> readers exist, so the memory gets freed if all QEMU block/pbs instances
> >> disappear.
> >>
> >> Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
> >> ---
> >>   src/lib.rs          |  7 ++++++-
> >>   src/shared_cache.rs | 36 ++++++++++++++++++++++++++++++++++++
> >>   2 files changed, 42 insertions(+), 1 deletion(-)
> >>   create mode 100644 src/shared_cache.rs
> >>
> >> diff --git a/src/lib.rs b/src/lib.rs
> >> index 05d7b58..aa167f7 100644
> >> --- a/src/lib.rs
> >> +++ b/src/lib.rs
> >> @@ -25,6 +25,7 @@ mod restore;
> >>   use restore::*;
> >>   
> >>   mod tools;
> >> +mod shared_cache;
> >>   
> >>   pub const PROXMOX_BACKUP_DEFAULT_CHUNK_SIZE: u64 = 1024*1024*4;
> >>   
> >> @@ -804,7 +805,11 @@ pub extern "C" fn proxmox_restore_connect_async(
> >>   pub extern "C" fn proxmox_restore_disconnect(handle: *mut ProxmoxRestoreHandle) {
> >>   
> >>       let restore_task = handle as * mut Arc<RestoreTask>;
> >> -    unsafe { Box::from_raw(restore_task) }; //drop(restore_task)
> >> +    let restore_task = unsafe { Box::from_raw(restore_task) };
> >> +    drop(restore_task);
> >> +
> >> +    // after dropping, cache may be unused (if no other handles open)
> >> +    shared_cache::shared_chunk_cache_cleanup();
> >>   }
> >>   
> >>   /// Restore an image (sync)
> >> diff --git a/src/shared_cache.rs b/src/shared_cache.rs
> >> new file mode 100644
> >> index 0000000..bebae5b
> >> --- /dev/null
> >> +++ b/src/shared_cache.rs
> >> @@ -0,0 +1,36 @@
> >> +use once_cell::sync::OnceCell;
> >> +use proxmox_backup::backup::ChunkCache;
> >> +use proxmox_backup::tools::async_lru_cache::AsyncLruCache;
> >> +use std::sync::{Arc, Mutex};
> >> +
> >> +const SHARED_CACHE_CAPACITY: usize = 64; // 256 MB
> >> +static SHARED_CACHE: OnceCell<Mutex<Option<ChunkCache>>> = OnceCell::new();
> > 
> > OnceCell *and* Option is a bit too much, `get_shared_chunk_chache()` can
> > just initialize it in `get_or_init()` directly.
> > 
> 
> Then how do we drop the ChunkCache? I used OnceCell since 
> std::sync::Mutex doesn't seem to have a 'const new', not because of 
> 'Option' semantics.

Right. I forgot about the cleanup.
As for the const Mutex::new... don't we have parking lot in our dependency tree already? ;-)
But yeah, I guess it's fine.