p5p summary: Improving threads::shared ?

(Apologies for not tracking p5p closely, I just
don't have time these days)

I've been pouring over the p5p archives trying to find
what the subject p5p summary item is about, but wo/ any luck.
Can someone point out the relevent thread title, or maybe
summarize what "threads::shared could share aggregates
properly with only Perl level changes to shared.pm"
means ?

TIA,
Dean Arnold

Dean Arnold wrote:
>  I've been pouring over the p5p archives trying to find
>  what the subject p5p summary item is about, but wo/ any luck.
>  Can someone point out the relevent thread title, or maybe
>  summarize what "threads::shared could share aggregates
>  properly with only Perl level changes to shared.pm"
>  means ?

It means that something like this would DWIM:

my $x : shared;
$x = [ { 'complex' => 'aggregate' }, [ qw/ currently not sharable / ] ];

To do that now you'd have to do:

my $x : shared;
$x = &share([]);
push(@$x, &share({}), &share([]));
$$x[0]{'complex'} = 'aggregate';
push(@{$$x[1]}, qw/ currently not sharable /);

The problem comes with copying complex structures:

my $x = [ { 'complex' => 'aggregate' }, [ qw/ currently not sharable / ] ];

my $y : shared;
$y = $x;

How do you handle the assignment?  With regular scalars, $x
and $y share copies of the same structure such that changes
via $x are visible via $y.  However, with the above, you'd
have to clone the structure pointed to by $x and assign that
clone to $y such that they would be independent.

And it gets worse:  Suppose the hash inside $x is already
shared.  Technically, you don't need to clone that portion.

And then you'd have to distinguish (in code) how to
differentiant between the above, and say $z = $x when both
are already shared (in which case no cloning is needed).

The rules would need to be defined in detail and then
documented clearly so that programming don't get befuddled
trying to figure them out.

Thread::Queue has the Perl code needed for making 'complete'
shared clones of data structures (i.e., all parts whether
shared or not are cloned).  Tweaking it to not clone already
shared portions is trivial.

Then we just need a tie-in with the assignment operator.  Or
perhaps we could just provide a function:

$y = shared_clone($x);

Jerry D. Hedden wrote:
> Dean Arnold wrote: 
>
> It means that something like this would DWIM:
>
>     my $x : shared;
>     $x = [ { 'complex' => 'aggregate' }, [ qw/ currently not sharable / ] 
];
>

<snip/>

>
> Thread::Queue has the Perl code needed for making 'complete'
> shared clones of data structures (i.e., all parts whether
> shared or not are cloned).  Tweaking it to not clone already
> shared portions is trivial.
>

I see someone's been busy ;^)

Alas, there's one catch that the current T::Q implementation
doesn't cover: recursive structures:
(Using AS Perl 5.8.8, WinXP, T::Q 2.06)

use strict;
use warnings;
use threads;
use threads::shared;
use Thread::Queue;
use Data::Dumper;

my $q = Thread::Queue->new();

my $x = [ { 'complex' => 'aggregate' },
[ qw/ currently not sharable / ] ];

my $thrd = threads->create(\&receiver, $q);

$q->enqueue($x);

$thrd->join();

print "*** non-recursive OK\n";

#
#	Make it a recursive structure
#	(!!! this will choke)
#
push @$x, $x;

$thrd = threads->create(\&receiver, $q);

$q->enqueue($x);

$thrd->join();

print "*** recursive OK\n";


sub receiver {
my $q = shift;

my $data = $q->dequeue();
print "** in child:\n", Dumper($data), "\n";
}


The 2nd part emits a bunch of

"Deep recursion on anonymous subroutine at C:/Perl/lib/Thread/Queue.pm line 
181"
warnings and then hangs.

While I agree that recursive structures should be discouraged, and don't
expect T::Q (or threads::shared, ftm) to try to trap them, the T::Q POD
probably needs to include a warning about it. And in the case of queueing
blessed objects, I'd expect instances of circular refs between objects
not to be uncommon.

FWIW: I had looked into this issue for Thread::Queue::Duplex and
Thread::Apartment, and punted the deepcopy clone to Storable instead
because of the circular ref issue.

OTOH, if a bitflag could be grabbed
somewhere to do a mark/sweep, this might be solvable
Or maybe using a fieldhash to key the original ref thats
being cloned, do a quick lookup, and if it exists, then skip it
ala testing for shared-ness ? kinda slow, and maybe a memory pig
for deep structures, but should solve the issue.

> Then we just need a tie-in with the assignment operator.  Or
> perhaps we could just provide a function:
>
>     $y = shared_clone($x);
>

Would an assignment op overload work ?
I.e., if the LHS was already shared(), then the = overload
would do the deepcopy ? Or would that break the
XS tie/magic side of the code ?

I guess it wouldn't break *if* the clone operation gets pushed
down to XS; then the STORE operation just detects the
value as an unshared ref (as it already does), and performs
the deepcopy. At which point some add'l optimization (e.g.,
grabbing the global lock once for the entire copy operation)
could be applied.

At which point, turning an existing populated hash/array
into a shared variable could also be updated to preserve the contents.

- Dean

Dean Arnold wrote:
> Jerry D. Hedden wrote: 
>
> <snip/>
> 
>
> I see someone's been busy ;^)
>
> Alas, there's one catch that the current T::Q implementation
> doesn't cover: recursive structures:
> (Using AS Perl 5.8.8, WinXP, T::Q 2.06)
>

<snip/>

> OTOH, if a bitflag could be grabbed
> somewhere to do a mark/sweep, this might be solvable
> Or maybe using a fieldhash to key the original ref thats
> being cloned, do a quick lookup, and if it exists, then skip it

<snip/>

Momentary insanity:
Why not reuse the existing perl_clone code ?
Assuming an appropriate entry point could be
found *and* perl_clone can be fooled into
using the shared global interpretter instead
of creating a new interpretter, it should be
possible to do a complete clone wo/ circular issues.
And it would presumably support creation of
shared filehandles.

OTOH, the last time I tried to decipher perl_clone(),
I ended up w/ a 3 day headache, so it may be
intractable.

- Dean

Dean Arnold wrote:
> Alas, there's one catch that the current T::Q
> implementation doesn't cover: recursive structures:

Oh, crud.  Thanks for catching that.

> Or maybe using a fieldhash to key the original ref thats
> being cloned, do a quick lookup, and if it exists, then
> skip it

That was my thought, too.  I'll work on it.

>  Momentary insanity: Why not reuse the existing perl_clone
>  code ?

Egads!  I've no clue about this.

> Would an assignment op overload work?  I.e., if the LHS
> was already shared(), then the = overload would do the
> deepcopy? Or would that break the XS tie/magic side of the
> code?

I had the same idea.  I can't think of why it might conflict
with anything the XS code does (but that thought doesn't
have much to back it up).

What are your thoughts regarding structures that mix shared
and non-shared elements?  Should a whole separate clone be
made, or should that shared portions (i.e., refs) be used in
the copy?  (I'm thinking the latter, while messier, makes
more sense.)

Jerry D. Hedden wrote:
>
> What are your thoughts regarding structures that mix shared
> and non-shared elements?  Should a whole separate clone be
> made, or should that shared portions (i.e., refs) be used in
> the copy?  (I'm thinking the latter, while messier, makes
> more sense.)
>

I agree. That most closely matches perl_clone behavior,
ie., preserving shared variables between threads.
I don't know that its messier, but it will certainly need to
be documented.

BTW: have you looked into reusing or integrating with any of the
Clone modules ? At present, I don't think they handle
shared variables, but they're the usual mechanism
for cloning structures, and a couple of them use XS for
performance.

- Dean

Dean Arnold wrote:
>  BTW: have you looked into reusing or integrating with any
>  of the Clone modules ? At present, I don't think they
>  handle shared variables, but they're the usual mechanism
>  for cloning structures, and a couple of them use XS for
>  performance.

I'm not familiar with any of them.  However, if they don't
handled shared variables, I'm not sure they'd be useful
unless they were modified to do make shared copies at each
step.

As a first step, I'll work on fixing the circular references
issue in Thread::Queue.  (I think I have it done, but need
to test it.)  Then work on moving that code to
threads::shared in conjunction with overloading the '='
operator.  After that, I could look into other modules
and/or further optimizations.

I've attached my reworked version of T::Q that I think takes
care of circular references.  Would you mind giving it a
going over?  (It passes all the currents tests in its test
suite, so I know I didn't break anything.)  Thanks.

Jerry D. Hedden wrote:
>
> I've attached my reworked version of T::Q that I think takes
> care of circular references.  Would you mind giving it a
> going over?  (It passes all the currents tests in its test
> suite, so I know I didn't break anything.)  Thanks.

My sample worked OK *after* I removed the Data::Dumper
print; apparently, it has some issues w/ circular
references as well (I tried setting Deepcopy and
Purity, to no avail); I suspect its due to the fact
that references to shared elements don't have the same
stringified value, so it doesn't actually look like
a circular ref.

Anyway, my updated version of the test follows.

- Dean

use strict;
use warnings;
use threads;
use threads::shared;
use Thread::Queue;
use Data::Dumper;

my $q = Thread::Queue->new();

my $x = [ { 'complex' => 'aggregate' },
[ qw/ currently not sharable / ] ];

my $thrd = threads->create(\&receiver, $q);

$q->enqueue($x);

$thrd->join();

print "*** non-recursive OK\n";

#
#	Make it a recursive structure
#	(!!! this will choke)
#
push @$x, $x;

$thrd = threads->create(\&receiver, $q);

$q->enqueue($x);

print "*** enq'd\n";

$thrd->join();

print "*** recursive OK\n";


sub receiver {
my $q = shift;
print "*** starting child\n";
my $data = $q->dequeue();
print "*** deq'd\n";

if ($#$data == 1) {
print "** in child:\n", Dumper($data), "\n";
}
elsif (($#$data != 2) || (ref $data->[2] ne 'ARRAY')) {
print "NOT OK: $#$data $data $data->[2]\n";
}
else {
print "** in child:\n", Dumper($data->[0], $data->[1]), "\n";
}
}

erry D. Hedden wrote:
>     Look at $x:
>     REF(0x144f8f0)
>     REF(0x144f8f0)
>     REF(0x144f8f0)
>     REF(0x144f8f0)
>     REF(0x144f8f0)
>     REF(0x144f8f0)
>
>     First look at $y:
>     SCALAR(0x1423c70)
>     SCALAR(0x1423ad8)
>     SCALAR(0x14bd968)
>     SCALAR(0x14bd980)
>     SCALAR(0x14bd998)
>     SCALAR(0x14bd9b0)
>
>     Second look at $y:
>     REF(0x1423c70)
>     REF(0x1423ad8)
>     REF(0x14bd968)
>     REF(0x14bd980)
>     REF(0x14bd998)
>     SCALAR(0x14bd9b0)
>
>  Seems to me that this is a bug.  It shows that
>  threads::shared isn't detecting that it's dealing with a
>  circular reference.  With each level that the app traverses,
>  threads::shared "unrolls" another version of the shared
>  variable.

>  The more I think about it, the more I'm convinced this needs
>  to be fixed.  The original variable $x is a circular
>  reference, and it's finiteness is detectable.  The copy made
>  to $y has become an infinitely deep reference with lazy
>  evaluation.

I've been looking at how to fix this, but know understanding
of the internals of threads::shared is weak.

Conceptually, I think it would require keeping a weak
reference of each private SV associated with a shared SV.
This would need to be done on a per-thread basis.  Then when
a thread tries to reference a shared SV, a lookup would be
made to see if a private SV already exists (and still
exists) for that thread.  If so, that is returned (and the
ref count of the weak ref is incremented?).  If not, a new
private SV is created, and a weak ref to it is appropriately
stored.

Is this a logical approach?  If so, is it doable?

>  Is this a logical approach?  If so, is it doable?

If circular references can't be fully supported in
threads::shared, then I need to document this in its POD and
in Thread::Queue's POD, too.  Do you agree?