Libguestfs - Sep 2016 - Re: [PATCH] Improve memory management of nbdkit python plugin example

Hi,

the nbdkit python plugin example has suboptimal memory management:
- it creates the disk image as a string on init
- it casts the string to bytearray on every read
- it copies the string before and the string after the written region,
then reassembles those pieces together with the written region to a new
disk image string

This is not a problem as long as the image is small, but in my tests
with a 5 GB sized image nbdkit already used 15 GB RAM directly after
startup, and even more (20-25 GB) on the first write.

This changes the code to use bytearray everywhere and use the proper
methods to change bytearray objects directly. With the patch applied,
nbdkit with a 5 GB image will still only use 5 GB RAM even during heavy
read/write activity.

Regards,
Carl-Daniel

diff -r 521366d1854b -r d7d5078d08c7 plugins/python/example.py
--- a/plugins/python/example.py	Sun Jul 10 17:10:30 2016 +0100
+++ b/plugins/python/example.py	Sun Sep 25 05:04:02 2016 +0200
@@ -29,7 +29,7 @@
 # reconnect to the same server you should see the same disk.  You
 # could also put this into the handle, so there would be a fresh disk
 # per handle.
-disk = "\0" * (1024*1024);
+disk = bytearray(1024 * 1024)
 
 # This just prints the extra command line parameters, but real plugins
 # should parse them and reject any unknown parameters.
@@ -50,9 +50,9 @@
 
 def pread(h, count, offset):
     global disk
-    return bytearray (disk[offset:offset+count])
+    return disk[offset:offset+count]
 
 def pwrite(h, buf, offset):
     global disk
     end = offset + len (buf)
-    disk = disk[:offset] + buf + disk[end:]
+    disk[offset:end] = buf

Hi,

On Monday, 26 September 2016 17:07:41 CEST Carl-Daniel Hailfinger
wrote:
> the nbdkit python plugin example has suboptimal memory management:
> - it creates the disk image as a string on init
> - it casts the string to bytearray on every read
> - it copies the string before and the string after the written region,
> then reassembles those pieces together with the written region to a new
> disk image string
> 
> This is not a problem as long as the image is small, but in my tests
> with a 5 GB sized image nbdkit already used 15 GB RAM directly after
> startup, and even more (20-25 GB) on the first write.
> 
> This changes the code to use bytearray everywhere and use the proper
> methods to change bytearray objects directly. With the patch applied,
> nbdkit with a 5 GB image will still only use 5 GB RAM even during heavy
> read/write activity.
> 
> Regards,
> Carl-Daniel
> 
> diff -r 521366d1854b -r d7d5078d08c7 plugins/python/example.py
> --- a/plugins/python/example.py	Sun Jul 10 17:10:30 2016 +0100
> +++ b/plugins/python/example.py	Sun Sep 25 05:04:02 2016 +0200
> @@ -29,7 +29,7 @@
>  # reconnect to the same server you should see the same disk.  You
>  # could also put this into the handle, so there would be a fresh disk
>  # per handle.
> -disk = "\0" * (1024*1024);
> +disk = bytearray(1024 * 1024)
>  
>  # This just prints the extra command line parameters, but real plugins
>  # should parse them and reject any unknown parameters.
> @@ -50,9 +50,9 @@
>  
>  def pread(h, count, offset):
>      global disk
> -    return bytearray (disk[offset:offset+count])
> +    return disk[offset:offset+count]
>  
>  def pwrite(h, buf, offset):
>      global disk
>      end = offset + len (buf)
> -    disk = disk[:offset] + buf + disk[end:]
> +    disk[offset:end] = buf
I'd look fine to me -- I guess it should work with both Python 2 and 3,
right?

Thanks,
-- 
Pino Toscano

Hi,

On 26.09.2016 17:19, Pino Toscano wrote:
> On Monday, 26 September 2016 17:07:41 CEST Carl-Daniel Hailfinger wrote:
>> the nbdkit python plugin example has suboptimal memory management:
>> - it creates the disk image as a string on init >> - it casts the
string to bytearray on every read >> - it copies the string before and
the string after the written region, >> then reassembles those pieces
together with the written region to a new >> disk image string >>
>>
This is not a problem as long as the image is small, but in my tests >>
with a 5 GB sized image nbdkit already used 15 GB RAM directly after >>
startup, and even more (20-25 GB) on the first write. >> >> This
changes
the code to use bytearray everywhere and use the proper >> methods to
change bytearray objects directly. With the patch applied, >> nbdkit
with a 5 GB image will still only use 5 GB RAM even during heavy >>
read/write activity. >> >> Regards, >> Carl-Daniel >>
>> diff -r
521366d1854b -r d7d5078d08c7 plugins/python/example.py >> ---
a/plugins/python/example.py    Sun Jul 10 17:10:30 2016 +0100 >> +++
b/plugins/python/example.py    Sun Sep 25 05:04:02 2016 +0200 >> @@
-29,7 +29,7 @@ >>  # reconnect to the same server you should see the
same disk.  You >>  # could also put this into the handle, so there
would be a fresh disk >>  # per handle. >> -disk = "\0" *
(1024*1024);
>> +disk = bytearray(1024 * 1024) >>  >>  # This just prints
the extra
command line parameters, but real plugins >>  # should parse them and
reject any unknown parameters. >> @@ -50,9 +50,9 @@ >>  >> 
def pread(h,
count, offset): >>      global disk >> -    return bytearray
(disk[offset:offset+count]) >> +    return disk[offset:offset+count]
>> 
>>  def pwrite(h, buf, offset): >>      global disk >>     
end = offset
+ len (buf) >> -    disk = disk[:offset] + buf + disk[end:] >> +   
disk[offset:end] = buf > > I'd look fine to me -- I guess it should
work
with both Python 2 and 3, > right?

Indeed. By the way, does anyone know if there is a Python 3 plugin
infrastructure for nbdkit?

Regards,
Carl-Daniel

Hi,

sorry for the resend. My mailer mangled the previous attempt.

On 26.09.2016 17:19, Pino Toscano wrote:
> On Monday, 26 September 2016 17:07:41 CEST Carl-Daniel Hailfinger wrote:
>> the nbdkit python plugin example has suboptimal memory management:
>> - it creates the disk image as a string on init
>> - it casts the string to bytearray on every read
>> - it copies the string before and the string after the written region,
>> then reassembles those pieces together with the written region to a new
>> disk image string
>>
>> This is not a problem as long as the image is small, but in my tests
>> with a 5 GB sized image nbdkit already used 15 GB RAM directly after
>> startup, and even more (20-25 GB) on the first write.
>>
>> This changes the code to use bytearray everywhere and use the proper
>> methods to change bytearray objects directly. With the patch applied,
>> nbdkit with a 5 GB image will still only use 5 GB RAM even during heavy
>> read/write activity.
>>
>> Regards,
>> Carl-Daniel
>>
>> diff -r 521366d1854b -r d7d5078d08c7 plugins/python/example.py
>> --- a/plugins/python/example.py	Sun Jul 10 17:10:30 2016 +0100
>> +++ b/plugins/python/example.py	Sun Sep 25 05:04:02 2016 +0200
>> @@ -29,7 +29,7 @@
>>  # reconnect to the same server you should see the same disk.  You
>>  # could also put this into the handle, so there would be a fresh disk
>>  # per handle.
>> -disk = "\0" * (1024*1024);
>> +disk = bytearray(1024 * 1024)
>>  
>>  # This just prints the extra command line parameters, but real plugins
>>  # should parse them and reject any unknown parameters.
>> @@ -50,9 +50,9 @@
>>  
>>  def pread(h, count, offset):
>>      global disk
>> -    return bytearray (disk[offset:offset+count])
>> +    return disk[offset:offset+count]
>>  
>>  def pwrite(h, buf, offset):
>>      global disk
>>      end = offset + len (buf)
>> -    disk = disk[:offset] + buf + disk[end:]
>> +    disk[offset:end] = buf
> I'd look fine to me -- I guess it should work with both Python 2 and 3,
> right?
Indeed. By the way, does anyone know if there is a Python 3 plugin
infrastructure for nbdkit?

Regards,
Carl-Daniel