[Bug 691299] API: GstFileMemAllocator - an allocator that uses disk storage to provide memory space

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=691299
  GStreamer | gst-plugins-base | git

--- Comment #12 from Krzysztof Konopko <krzysztof.konopko at gmail.com> 2013-07-30 22:33:30 UTC ---
(In reply to comment #10)
> Review of attachment 250246 [details]:
> 
> ::: gst-libs/gst/allocators/gstmappedfile.c
> @@ +162,3 @@
> +      maxsize);
> +
> +  if (allocator->f_offset_next + maxsize > allocator->file_size) {
> 
> I think you should implement a better block allocation strategy, that allows to
> properly deallocate and reallocate blocks
> 
> @@ +207,3 @@
> +     of blocks available or more advanced if needed. */
> +
> +#if defined (HAVE_FALLOCATE) && HAVE_DECL_FALLOC_FL_PUNCH_HOLE
> 
> So if this is not supported, the memory will slowly run full and then nothing
> can be allocated anymore? I think the allocator should not be supported at all
> then

This allocator is bound to run out of space sooner or later as it's supposed to
be intialised with a specific total size.  The reason for this is that there's
no way to reliably resize the file if any of its mmap()-ed blocks have been
given out and are in use.  I'd like it wasn't true so please let me know if I'm
wrong here.

I agree that not reusing freed blocks is poor but given my reasoning above it's
not the end of the world.  I agree reusing blocks should be supported (and
should be a condition to accept this allocator at all) but there are at least
two ways to approach it:
- declare the allocator to allocate blocks of a certain size only
This makes it dead simple as it boils down to maintaining a list of free blocks

- support arbitrary block size allocations
This requires merging subsequent free blocks (buddy allocation, defragmentation
etc.).

> 
> @@ +532,3 @@
> +
> +  gst_allocator_register (GST_ALLOCATOR_MAPPEDFILE,
> +      GST_ALLOCATOR_CAST (allocator));
> 
> Not sure if registering the allocator globally with a fixed temp template makes
> sense. What if multiple such allocators with different temp templates should be
> used?
>

Yes, that hurts me and I'm keen to make it better based on previous suggestions
(pass a file descriptor to a file opened somewhere else etc.).

Thanks for the other spots.  I fixed them immediately as they were minors.  I
need a bigger chunk of spare time to tackle bigger issues.  Any additional
feedback would be very helpful.

-- 
Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.