From: sln on
On Mon, 09 Aug 2010 00:23:42 +0700, michael20545 <a(a)a.com> wrote:

>Hi all,
>
>I often need to convert 100s of SVG files to EPS format with Inkscape.
>It is continuous process, so I decided to use Perl threads for
>performing several jobs simultaneously. I'm new to Perl threads. Here is
>my script.
>
>
>use strict;
>use threads;
>use threads::shared;
>use Cwd;
>my $i : shared =0;
>my $dir=getcwd();
>my @files=<*.svg>;
>threads->new(\&convert_to_eps,1)->join();
>threads->new(\&convert_to_eps,2)->join();
>threads->new(\&convert_to_eps,3)->join();
>
>sub convert_to_eps
>{
> my $thread_num=$_[0];
> while($i<=$#files)
> {
> my $svg=$files[$i++];
> my $eps=$svg;
> $eps=~ s/\.svg/.eps/;
> print "$thread_num $svg\n";
> `inkscape -f "$dir\\$svg" -T -C -E "$dir\\$eps"`;
> }
>}
>
>Here works only first thread. What is wrong?
>
>

In addition to the join() going to wait on the thread as was
mentioned, I would think you need a lock around variable $i
since it is shared and incremented amongst the three threads
and it is not atomic for the assignment (increment maybe).
Seems for Windows this lock is like a mutex.

I tried an experiment without the lock and slept 1 second,
hit pause/un-pause a few times, then started getting random
duplicates. With the lock, it works fine. You can comment the
lock for a test of with/without.

-sln

----------------------
use strict;
use threads;
use threads::shared;
use Cwd;

#
my $i : shared = 0;
my $dir = getcwd();
my @files = <*>;

my @threads;
for (1 .. 3) {
push @threads, threads->new( \&show_files, $_);
}
for (@threads) {
$_->join();
}

#
sub show_files {
my ($thread_num) = @_;
while( $i <= $#files ) {
my ($local_file, $local_i);
{
lock $i;
$local_file = $files[$i++];
$local_i = $i;
}
print "$thread_num, $local_i, $dir/$local_file\n";
sleep(1);
}
}

From: sln on
On Sun, 08 Aug 2010 12:21:20 -0700, sln(a)netherlands.com wrote:

>On Mon, 09 Aug 2010 00:23:42 +0700, michael20545 <a(a)a.com> wrote:
>
>>Hi all,
>>
>>I often need to convert 100s of SVG files to EPS format with Inkscape.
>>It is continuous process, so I decided to use Perl threads for
>>performing several jobs simultaneously. I'm new to Perl threads. Here is
>>my script.
>>
>>
>>use strict;
>>use threads;
>>use threads::shared;
>>use Cwd;
>>my $i : shared =0;
>>my $dir=getcwd();
>>my @files=<*.svg>;
>>threads->new(\&convert_to_eps,1)->join();
>>threads->new(\&convert_to_eps,2)->join();
>>threads->new(\&convert_to_eps,3)->join();
>>
>>sub convert_to_eps
>>{
>> my $thread_num=$_[0];
>> while($i<=$#files)
>> {
>> my $svg=$files[$i++];
>> my $eps=$svg;
>> $eps=~ s/\.svg/.eps/;
>> print "$thread_num $svg\n";
>> `inkscape -f "$dir\\$svg" -T -C -E "$dir\\$eps"`;
>> }
>>}
>>
>>Here works only first thread. What is wrong?
>>
>>
>
>In addition to the join() going to wait on the thread as was
>mentioned, I would think you need a lock around variable $i
>since it is shared and incremented amongst the three threads
>and it is not atomic for the assignment (increment maybe).
>Seems for Windows this lock is like a mutex.
>
>I tried an experiment without the lock and slept 1 second,
>hit pause/un-pause a few times, then started getting random
>duplicates. With the lock, it works fine. You can comment the
>lock for a test of with/without.
>
>-sln
>
>----------------------
>use strict;
>use threads;
>use threads::shared;
>use Cwd;
>
>#
> my $i : shared = 0;
> my $dir = getcwd();
> my @files = <*>;
>
> my @threads;
> for (1 .. 3) {
> push @threads, threads->new( \&show_files, $_);
> }
> for (@threads) {
> $_->join();
> }
>

The while condition has to be protected as well.
Something like a while(1) condition then:
#
sub show_files {
my ($thread_num) = @_;
LOOP:
while( 1 ) {
my ($local_file, $local_i);
{
lock $i;
last LOOP if $i > $#files;
$local_file = $files[$i++];
$local_i = $i;
}
print "$thread_num, $local_i, $dir/$local_file\n";
# sleep(1);
}
}

>#
> sub show_files {
> my ($thread_num) = @_;
> while( $i <= $#files ) {
> my ($local_file, $local_i);
> {
> lock $i;
> $local_file = $files[$i++];
> $local_i = $i;
> }
> print "$thread_num, $local_i, $dir/$local_file\n";
> sleep(1);
> }
> }

From: sln on
On Sun, 08 Aug 2010 12:40:09 -0700, sln(a)netherlands.com wrote:

>The while condition has to be protected as well.
>Something like a while(1) condition then:

Or, better to just return:

#
sub show_files {
my ($thread_num) = @_;
while( 1 ) {
my ($local_file, $local_i);
{
lock $i;
return if $i > $#files;
$local_file = $files[$i++];
$local_i = $i;
}
print "$thread_num, $local_i, $dir/$local_file\n";
}
}

From: Wolf Behrenhoff on
On 08.08.2010 21:40, sln(a)netherlands.com wrote:
> The while condition has to be protected as well.
> Something like a while(1) condition then:

.... whatever else needs locking. It is easy to forget some locking
somewhere.

So my suggestion is to
use Thread::Queue;

then enqueue all svg filenames in a queue. Now create threads which
convert files while dequeue_nb retuns a filename.

Wolf
From: sln on
On Mon, 09 Aug 2010 10:25:29 +0200, Wolf Behrenhoff <NoSpamPleaseButThisIsValid3(a)gmx.net> wrote:

>On 08.08.2010 21:40, sln(a)netherlands.com wrote:
>> The while condition has to be protected as well.
>> Something like a while(1) condition then:
>
>... whatever else needs locking. It is easy to forget some locking
>somewhere.
>
>So my suggestion is to
>use Thread::Queue;
>
>then enqueue all svg filenames in a queue. Now create threads which
>convert files while dequeue_nb retuns a filename.
>
>Wolf

Of course, but the OP is taking small steps first.

-sln