Hi,
allan <[EMAIL PROTECTED]> said:
> hi
>
> i'm looking for a way to improve a program by using threads. i have no
> experiece at all with threads so thought i might ask this list.
>
> i have got an array of dynamic size. while "foreaching" inside this
> array there will be a call to a subroutine [which again will call
> other subs].
>
> so my thought was that maybe when one thread might get starting with
> element[0] and thus cascade into several subs, another thread might
> begin with element[1] and so on.
>
> 1) is that possible/wise ?
Seems to be OK for me...
> 2) if yes, how ?
see my comments below...
[...]
>
> use strict;
> use threads;
> use threads::shared;
>
> my @array : shared = qw (a b c d a b f g);
> my %cache : shared;
>
> # array loop
> for (my $i = 0; $i < $#array; $i++) {
> my $element = $array[$i];
> my $thr1 = threads->new(&fill_cache, $element);
> $thr1->detach;
> }
In the loop above you just start a thread and let it
die, then the loop goes on to the next thread. So,
there's no parallelism at all in your code, and, therefore,
threads are quite useless here.
Better approach (all threads are parallel now):
# array loop
my @thr=();
for (my $i = 0; $i < $#array; $i++) {
my $element = $array[$i];
$thr[$i] = threads->new(&fill_cache, $element);
}
for (my $i = 0; $i < $#array; $i++) {
$thr[$i]->detach;
}
Still there is a problem: If your threads do some amount
of work, your "main thread" will finish before its newly
created thread children. As a result your %cache will most
probably be empty (you can see this by inserting a "sleep 1"
into the fill_cache routine).
Solution: Let your main thread wait by replacing "detach":
$thr[$i]->join;
>
> foreach my $key (sort keys %cache) {
> print "$key => $cache{$key}n";
> }
>
> sub fill_cache {
> my $element = shift;
> # do some processing that might take up to 30 seconds
> if (exists($cache{$element})) {
> $cache{$element} += 1;
> } else {
> $cache{$element} = 1;
> }
> }
Here we have a "race condition"! For example: Thread 1 (with "a")
gets a "false" when checking %cache with exists(...). It will
fill in that key/value in the else part. BUT, it might happen
that _before_ this insertion takes place the other "a"-thread
comes to this if-statement, checks for existance, and gets a
"false", too! You can see this effect by inserting "sleep 1;"
just in front of "$cache{$element} = 1;".
Solution: Lock your %cache:
lock(%cache);
if (exists(...
...
# %cache is implicitly unlocked at end of sub
You might also try to use an atomic operation (I'm not quite
sure if this is really atomic) instead of the if-else-statement:
$cache{$element}++;
Hope this helps,
Eike
--
Eike Grote, ConSol Software GmbH, Muenchen
E-Mail: [EMAIL PROTECTED]
