Jump to page: 1 2
Thread overview
[Issue 16824] std.experimental.allocator.dispose leaks memory for arrays of more than 1 dimension
Dec 22, 2016
RazvanN
Dec 23, 2016
Atila Neves
Dec 23, 2016
ZombineDev
Dec 23, 2016
Atila Neves
Dec 23, 2016
greenify
December 15, 2016
https://issues.dlang.org/show_bug.cgi?id=16824

Andrei Alexandrescu <andrei@erdani.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |andrei@erdani.com
           Assignee|nobody@puremagic.com        |razvan.nitu1305@gmail.com

--
December 22, 2016
https://issues.dlang.org/show_bug.cgi?id=16824

--- Comment #1 from Andrei Alexandrescu <andrei@erdani.com> ---
OK, so what we have here at the core is this:

    auto ints2d = allocator.makeArray!(int[])(2);
    foreach(ref ints1d; ints2d)
        ints1d = allocator.makeArray!(int)(3);

What I see here by means of manual coding:

* Create an array of int[] using an allocator
* Create a bunch of arrays of int using the same allocator

This is again by means of manual coding. The individual arrays might have been created using a different allocator, or sliced from a larger buffer.

I don't think we should expect the top-level allocator to "know" (assume really) that everything was allocated with the same allocator.

--
December 22, 2016
https://issues.dlang.org/show_bug.cgi?id=16824

Andrei Alexandrescu <andrei@erdani.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED

--
December 22, 2016
https://issues.dlang.org/show_bug.cgi?id=16824

--- Comment #2 from RazvanN <razvan.nitu1305@gmail.com> ---
(In reply to Andrei Alexandrescu from comment #1)
> OK, so what we have here at the core is this:
> 
>     auto ints2d = allocator.makeArray!(int[])(2);
>     foreach(ref ints1d; ints2d)
>         ints1d = allocator.makeArray!(int)(3);
> 
> What I see here by means of manual coding:
> 
> * Create an array of int[] using an allocator
> * Create a bunch of arrays of int using the same allocator
> 
> This is again by means of manual coding. The individual arrays might have been created using a different allocator, or sliced from a larger buffer.
> 
> I don't think we should expect the top-level allocator to "know" (assume really) that everything was allocated with the same allocator.

Thing would be great if we could test to see if the inner arrays were allocated
using the same allocator. If that is the case, then we can free the initial
array
entirely, otherwise it's the user's job to do that. Unfortunately, I do not
know
at this point if such a test is possible, but I will investigate further.

--
December 22, 2016
https://issues.dlang.org/show_bug.cgi?id=16824

--- Comment #3 from Andrei Alexandrescu <andrei@erdani.com> ---
The owns() method allows allocators to figure that out, but returns true for internal pointers as well, which means the following would not end well:

    auto ints2d = allocator.makeArray!(int[])(2);
    auto bulk = allocator.makeArray!(int[])(ints2d.length * 100);
    foreach(i; 0 .. ints2d.length)
        ints2s[i] = bulk[i * 100 .. (i + 1) * 100];

which is a customary way to save on allocations in multidimensional arrays.

@Atila, any good argument on how we can make this work? If not, I think we should close as invalid.

--
December 22, 2016
https://issues.dlang.org/show_bug.cgi?id=16824

--- Comment #4 from Andrei Alexandrescu <andrei@erdani.com> ---
(In reply to Andrei Alexandrescu from comment #3)
> The owns() method allows allocators to figure that out, but returns true for internal pointers as well, which means the following would not end well:
> 
>     auto ints2d = allocator.makeArray!(int[])(2);
>     auto bulk = allocator.makeArray!(int[])(ints2d.length * 100);
>     foreach(i; 0 .. ints2d.length)
>         ints2s[i] = bulk[i * 100 .. (i + 1) * 100];
> 
> which is a customary way to save on allocations in multidimensional arrays.
> 
> @Atila, any good argument on how we can make this work? If not, I think we should close as invalid.

Corrected code:

    auto ints2d = allocator.makeArray!(int[])(2);
    auto bulk = allocator.makeArray!int(ints2d.length * 100);
    foreach(i; 0 .. ints2d.length)
        ints2s[i] = bulk[i * 100 .. (i + 1) * 100];

--
December 23, 2016
https://issues.dlang.org/show_bug.cgi?id=16824

--- Comment #5 from Atila Neves <atila.neves@gmail.com> ---
I understand the arguments; but I still think there should be an easy, canonical way of allocating arrays of more than one dimension and be able to dispose of them without leaking memory. I didn't even know I _was_ leaking memory until I wrote that test allocator. And I only did _that_ because I'm paranoid.

--
December 23, 2016
https://issues.dlang.org/show_bug.cgi?id=16824

ZombineDev <petar.p.kirov@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |petar.p.kirov@gmail.com

--
December 23, 2016
https://issues.dlang.org/show_bug.cgi?id=16824

--- Comment #6 from Andrei Alexandrescu <andrei@erdani.com> ---
(In reply to Atila Neves from comment #5)
> I understand the arguments; but I still think there should be an easy, canonical way of allocating arrays of more than one dimension and be able to dispose of them without leaking memory. I didn't even know I _was_ leaking memory until I wrote that test allocator. And I only did _that_ because I'm paranoid.

It seems the right view is this:

* If you used a loop to allocate your multidimensional array, you're supposed
to use a loop to deallocate your multidimensional array. The library can't
guess what you did.
* If the library provides a means to dispose a multidimensional array in one
shot under certain assumptions, the library must provide a means to create a
multidimensional array in one shot such that those assumptions are fulfilled.

One way I see this moving forward is to provide the following functions:

* makeMultidimensional -> does the looparoo that Atila currently does by hand * disposeMultidimensional -> does what Razvan does now in his PR, i.e. disposes the whole array ASSUMING it had been created by makeMultidimensional

(maybe later)

* makeCompactMultidimensional -> computes the appopriate sizes and allocates one hunk that is then sliced and diced to return the multidimensional array. * disposeCompactMultidimensional -> disposes an array ASSUMING it had been created by makeCompactMultidimensional

--
December 23, 2016
https://issues.dlang.org/show_bug.cgi?id=16824

--- Comment #7 from Atila Neves <atila.neves@gmail.com> ---
I think adding the extra functions is the way forward.

--
« First   ‹ Prev
1 2