all lists on lists.proxmox.com
 help / color / mirror / Atom feed
* Re: [pve-devel] [pbs-devel] [PATCH proxmox-backup-qemu 8/9] add shared_cache module
@ 2021-06-07  8:07 ` Wolfgang Bumiller
  0 siblings, 0 replies; 4+ messages in thread
From: Wolfgang Bumiller @ 2021-06-07  8:07 UTC (permalink / raw)
  To: Stefan Reiter; +Cc: pve-devel, pbs-devel


> 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.




^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [pbs-devel] [PATCH proxmox-backup-qemu 8/9] add shared_cache module
@ 2021-06-07  8:07 ` Wolfgang Bumiller
  0 siblings, 0 replies; 4+ messages in thread
From: Wolfgang Bumiller @ 2021-06-07  8:07 UTC (permalink / raw)
  To: Stefan Reiter; +Cc: pve-devel, pbs-devel


> 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.




^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [pve-devel] [pbs-devel] [PATCH proxmox-backup-qemu 8/9] add shared_cache module
  2021-06-04 12:16   ` [pve-devel] [pbs-devel] " Wolfgang Bumiller
@ 2021-06-07  8:03     ` Stefan Reiter
  0 siblings, 0 replies; 4+ messages in thread
From: Stefan Reiter @ 2021-06-07  8:03 UTC (permalink / raw)
  To: Wolfgang Bumiller; +Cc: pve-devel, pbs-devel

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.

>> +
>> +pub fn get_shared_chunk_cache() -> ChunkCache {
>> +    let mut guard = SHARED_CACHE
>> +        .get_or_init(|| Mutex::new(None))
>> +        .lock()
>> +        .unwrap();
>> +    match &*guard {
>> +        Some(cache) => Arc::clone(cache),
>> +        None => {
>> +            let cache = Arc::new(AsyncLruCache::new(SHARED_CACHE_CAPACITY));
>> +            *guard = Some(Arc::clone(&cache));
>> +            cache
>> +        }
>> +    }
>> +}
>> +
>> +pub fn shared_chunk_cache_cleanup() {
>> +    let mut guard = SHARED_CACHE
>> +        .get_or_init(|| Mutex::new(None))
>> +        .lock()
>> +        .unwrap();
>> +    if let Some(arc) = guard.as_ref() {
>> +        let refcount = Arc::strong_count(arc);
>> +        if refcount == 1 {
>> +            // no one else is using the cache anymore, drop it
>> +            let _drop = guard.take();
>> +        }
>> +    }
>> +}
>> -- 
>> 2.30.2




^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [pve-devel] [pbs-devel] [PATCH proxmox-backup-qemu 8/9] add shared_cache module
  2021-06-02 14:38 ` [pve-devel] [PATCH proxmox-backup-qemu 8/9] add shared_cache module Stefan Reiter
@ 2021-06-04 12:16   ` Wolfgang Bumiller
  2021-06-07  8:03     ` Stefan Reiter
  0 siblings, 1 reply; 4+ messages in thread
From: Wolfgang Bumiller @ 2021-06-04 12:16 UTC (permalink / raw)
  To: Stefan Reiter; +Cc: pve-devel, pbs-devel

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.

> +
> +pub fn get_shared_chunk_cache() -> ChunkCache {
> +    let mut guard = SHARED_CACHE
> +        .get_or_init(|| Mutex::new(None))
> +        .lock()
> +        .unwrap();
> +    match &*guard {
> +        Some(cache) => Arc::clone(cache),
> +        None => {
> +            let cache = Arc::new(AsyncLruCache::new(SHARED_CACHE_CAPACITY));
> +            *guard = Some(Arc::clone(&cache));
> +            cache
> +        }
> +    }
> +}
> +
> +pub fn shared_chunk_cache_cleanup() {
> +    let mut guard = SHARED_CACHE
> +        .get_or_init(|| Mutex::new(None))
> +        .lock()
> +        .unwrap();
> +    if let Some(arc) = guard.as_ref() {
> +        let refcount = Arc::strong_count(arc);
> +        if refcount == 1 {
> +            // no one else is using the cache anymore, drop it
> +            let _drop = guard.take();
> +        }
> +    }
> +}
> -- 
> 2.30.2




^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-06-07  8:08 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-07  8:07 [pve-devel] [pbs-devel] [PATCH proxmox-backup-qemu 8/9] add shared_cache module Wolfgang Bumiller
2021-06-07  8:07 ` Wolfgang Bumiller
  -- strict thread matches above, loose matches on Subject: below --
2021-06-02 14:38 [pve-devel] [PATCH 0/9] Improve live-restore speed and replace AsyncIndexReader Stefan Reiter
2021-06-02 14:38 ` [pve-devel] [PATCH proxmox-backup-qemu 8/9] add shared_cache module Stefan Reiter
2021-06-04 12:16   ` [pve-devel] [pbs-devel] " Wolfgang Bumiller
2021-06-07  8:03     ` Stefan Reiter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal