Opened 3 years ago

Last modified 19 months ago

#5556 new feature request

Support pin-changing on ByteArray#s

Reported by: pumpkin Owned by:
Priority: normal Milestone: 7.6.2
Component: Compiler Version: 7.2.1
Keywords: Cc: AntoineLatter, v.dijk.bas@…, as@…
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Difficulty:
Test Case: Blocked By:
Blocking: Related Tickets:

Description

I'm mostly posting this here as a chance for discussion for this feature, as I've wanted it a few times before and because Roman Leshchinskiy mentioned it on reddit and reminded me.

<quoting>

IIRC, the basic idea was to have an operation like:

pin# :: ByteArray# -> ByteArray#

Then you could say:

let y = pin# x

and y would have the same contents as x but be pinned. Whether it is the same memory block as x would be implementation-dependent but with GHC, it would be. When GHC would garbage collect y, it would unpin x (if x is still alive). You would also have unpin# so if you said:

let z = unpin# y

then z will become unpinned when y is garbage collected.

</quoting>

I mostly care because it's unfortunate to have to decide up front whether you want to suffer from memory fragmentation or to support foreign bindings. This could for example let us break the distinction between Data.Vector.Unboxed and Data.Vector.Storable. From a higher level, you might have a withPinned function that would temporarily pin (without copying) an array while you make foreign calls with it (even across multiple foreign calls), and then would unpin it when you're done.

I'm not sure what this would entail on the actual GC/runtime side of things, but I figured it'd be worth discussing.

Change History (8)

comment:1 follow-up: Changed 3 years ago by AntoineLatter

  • Cc AntoineLatter added

I think Johan suggested a while back that it might be easier to have an 'isPinned' primop, and then implement 'withPinned' in terms of that (and copying, for unpinned ByteArrays?).

comment:2 in reply to: ↑ 1 Changed 3 years ago by AntoineLatter

Replying to AntoineLatter:

I think Johan suggested a while back that it might be easier to have an 'isPinned' primop, and then implement 'withPinned' in terms of that (and copying, for unpinned ByteArrays?).

I found what I'd tried towards this. I'm fairly inept at Cmm so I don't think I ever got it to build, but you can get the general idea:

stg_pinnedByteArrayzh
{
    /* Args: R1 = pointer to heap data */

    // Is the byte array pinned?
    // This assumes that large objects are pinned

    W_ ptr, bd, ret;
    REP_bdescr_flags flags;

    ptr = R1;
    bd = Bdescr(ptr);    
    flags = bdescr_flags(bd);

    if ( flags & (BF_LARGE::REP_bdescr_flags | BF_PINNED::REP_bdescr_flags) == 0::REP_bdescr_flags )
    {
      ret = 0::W_;
    } else {
      ret = 1::W_;
    }

    RET_N(ret);
} 

comment:3 Changed 3 years ago by pumpkin

It'd be nice to avoid copying it altogether, if possible. Roman suggested this might be possible in what I quoted. But yeah, an isPinned would also be helpful

comment:4 Changed 3 years ago by basvandijk

  • Cc v.dijk.bas@… added

comment:5 Changed 3 years ago by rl

Thinking about this a bit more, I think I suggested to have separate types for pinned and unpinned ByteArrays, with pin and unpin as primitives. I don't really see how isPinned would ever be useful except for checking if an array is pinned and creating a pinned copy if it isn't. It makes more sense to just provide a primop which does that. Incidentally, if we had ByteArray and PinnedByteArray then the latter could just be a ForeignPtr in compilers which don't support pinning. But GHC could be cleverer, of course. We'd probably want pin and unpin to copy for very small arrays and not copy for larger ones (but the threshold would be much lower than what GHC uses for deciding whether to pin a ByteArray by default).

I'm not a big friend of withPinned. It seems horribly dangerous, even more so that withForeignPtr.

comment:6 Changed 3 years ago by thoughtpolice

  • Cc as@… added

comment:7 Changed 2 years ago by igloo

  • Milestone set to 7.6.1

comment:8 Changed 19 months ago by igloo

  • Milestone changed from 7.6.1 to 7.6.2
Note: See TracTickets for help on using tickets.