mir-algorithm icon indicating copy to clipboard operation
mir-algorithm copied to clipboard

Understanding rcarray better

Open jmh530 opened this issue 5 years ago • 7 comments

I'm a little confused by the behavior of the code below. Foo accepts some memory in payload, but then when I use the asRCArray function it seems as if a copy is being made.

Also, taking a slice of counts and passing it to Foo does not increment the counter, even though it seems as if there is a reference back to counts (in that changing counts results in changes in foo and not x).

I've also tried compiling this with -dip1000 and @safe: at the top.

struct Foo
{
    double[] payload;

    this(double[] x)
    {
        payload = x;
    }
    
    auto asRCArray()
    {
        import mir.rc.array: rcarray;
     	return payload.rcarray;
    }
}

void main() {
    import mir.rc.array: mininitRcarray;
    
    size_t len = 5;
    auto counts = len.mininitRcarray!double;
    size_t i = 0;
    while (i < len) {
    	counts[i] = i;
        i++;
    }
    assert(counts._counter == 1);
    Foo foo = Foo(counts[]);
    assert(counts._counter == 1); //why not 2?
    auto x = foo.asRCArray();
    counts[2] = 10;
    assert(foo.payload[2] == 10);
    //assert(x[2] == 10); //fails to compile
    assert(x._counter == 1);
    assert(counts._counter == 1); 
}

jmh530 avatar Nov 12 '20 19:11 jmh530

rcarray is a function that creates a new RCArray. The type of double RCArray is RCArray!double. RCArray!double.opIndex (the [] operator) returns a slice of the underlying memory.

DIP1000 isn't well implemented

9il avatar Nov 13 '20 06:11 9il

When you say rcarray creates a new RCArray you mean that it creates a copy, right?

I also would have thought that taking a slice with [] would increase the reference count. So for instance, I was surprised that the code below compiles. But if opIndex is just providing a copy and not a reference, then it makes sense.

void main() {
    import mir.rc.array: mininitRcarray;
    
    size_t len = 5;
    auto counts = len.mininitRcarray!double;
    assert(counts._counter == 1);
    auto x = counts;
    assert(counts._counter == 2);
    auto y = counts.asSlice;
    assert(counts._counter == 3);
    auto z = counts[];
    assert(counts._counter == 3);
}

When I modified the code from the first post as below, I was able to get some of the behavior that I was expecting, though it is not integrated into rcarray. Could this be used to give out a reference to the mir_rcarray._payload and still increment the reference counter?

struct Foo
{
    double[] payload;

    this(double[] x)
    {
        payload = x;
    }
    
    ref double[] getPayloadRef() return
    {
        import mir.rc.array: rcarray;
     	return payload;
    }
}

void main() {
    import mir.rc.array: mininitRcarray;
    
    size_t len = 5;
    auto counts = len.mininitRcarray!double;
    size_t i = 0;
    while (i < len) {
    	counts[i] = i;
        i++;
    }
    Foo foo = Foo(counts[]);
    auto x = foo.getPayloadRef();
    counts[2] = 10;
    assert(foo.payload[2] == 10);
    assert(x[2] == 10);
    assert(counts._counter == 1); 
}

jmh530 avatar Nov 13 '20 12:11 jmh530

When you say rcarray creates a new RCArray you mean that it creates a copy, right?

Yes

Could this be used to give out a reference to the mir_rcarray._payload and still increment the reference counter?

RCArray!T should be used instead.

import core.lifetime: move;
import mir.rc.array;

struct Foo
{
    RCArray!double payload;

    this(RCArray!double x)
    {
        payload = move(x);
    }
    
	double[] getPayloadRef() return scope
    {
        import mir.rc.array: rcarray;
     	return payload[];
    }
}

void main()
{    
    size_t len = 5;
    auto counts = len.mininitRcarray!double;
    size_t i = 0;
    while (i < len) {
    	counts[i] = i;
        i++;
    }
    Foo foo = Foo(counts);
    auto x = foo.getPayloadRef();
    counts[2] = 10;
    assert(foo.payload[2] == 10);
    assert(x[2] == 10);
    assert(counts._counter == 2); 
}

9il avatar Nov 13 '20 16:11 9il

@9il Thanks for the reply.

When I try to also allow it to work with a GC slice, then I get errors again.


import core.lifetime: move;
import mir.rc.array;

struct Foo
{
    RCArray!double payload;

    this(RCArray!double x)
    {
        payload = move(x);
    }
    
    this(double[] x)
    {
        payload = rcarray(x.move);
    }
    
    double[] getPayloadRef() return scope
    {
        import mir.rc.array: rcarray;
     	return payload[];
    }
}

void main()
{    
    size_t len = 5;

    double[] counts = [0, 1, 2, 3, 4];
    Foo foo = Foo(counts);
    auto x = foo.getPayloadRef();
    counts[2] = 10;
    assert(foo.payload[2] == 10); //fails
    assert(x[2] == 10); //fails
}

The particular use case I was thinking about is for my work on mir.stat's histogram, which I have picked back up work on recently. Where I came across this issue is that I have some initial HistogramAccumulator that stores count data and I want to be able to 1) allow it to work with different allocation strategies (RC/GC/custom) for the counts and 2) to be able to provide some kind of range interface (I was thinking this would be a separate struct that would be returned from a member function). I'm tempted to just make the count data always an RC array, but I haven't made up my mind.

jmh530 avatar Nov 13 '20 17:11 jmh530

maybe a GC will be good to start and then we can think about nogc api

9il avatar Nov 13 '20 17:11 9il

The current implementation is here (i.e. from before this discussion).

jmh530 avatar Nov 13 '20 22:11 jmh530

The current implementation is here (i.e. from before this discussion).

@jmh530 That is great work!

Looking forward to the PR. (WIP status PR LGTM)

Lets, don't care about an RC API for now. If we can provide a lower-level API that accepts user-provided memory, then it should be fine. A high-level API can GC-allocate memory.

9il avatar Nov 17 '20 11:11 9il