Beefy Boxes and Bandwidth Generously Provided by pair Networks
Keep It Simple, Stupid
 
PerlMonks  

Double the speed of Imager->setpixel

by Anonymous Monk
on Dec 11, 2022 at 21:49 UTC ( [id://11148751]=perlmeditation: print w/replies, xml ) Need Help??

My app was taking 5 seconds to generate an image involving about a million calls to Imager->setpixel and this felt way too slow. NYTProf revealed the cause to be expensive sanity checking in Imager.pm at lines 3456 and 3465. Commenting those lines gets me down to 3 seconds and this now feels much less slow:
sub setpixel { my ($self, %opts) = @_; # $self->_valid_image("setpixel") or return; my $color = $opts{color}; unless (defined $color) { $color = $self->{fg}; defined $color or $color = NC(255, 255, 255); } # unless (ref $color && UNIVERSAL::isa($color, "Imager::Color")) { # unless ($color = _color($color, 'setpixel')) { # $self->_set_error("setpixel: " . Imager->errstr); # return; # } # } unless (exists $opts{'x'} && exists $opts{'y'}) { $self->_set_error('setpixel: missing x or y parameter'); return; } ... } sub _valid_image { my ($self, $method) = @_; ref $self or return Imager->_set_error("$method needs an image object"); $self->{IMG} && Scalar::Util::blessed($self->{IMG}) and return 1; my $msg = $self->{IMG} ? "images do not cross threads" : "empty inpu +t image"; $msg = "$method: $msg" if $method; $self->_set_error($msg); return; }
Should the next version of Imager have an option to disable global sanity so it can operate almost twice the usual speed?

Replies are listed 'Best First'.
Re: Double the speed of Imager->setpixel
by swl (Parson) on Dec 11, 2022 at 22:55 UTC

    Setting a flag on the object to not run validity checks is one option, and safer than a global variable.

    Imager is on GitHub at https://github.com/tonycoz/imager so you could always raise an issue and/or submit a pull request.

      That is also an option, but testing for a flag itself takes time (and adds a degree of complexity).

      My preference for this sort of thing is to have an internal method (eg _setpixel) that gets called to do the real work once all sanity checks have been performed, and where appropriate to expose that in documentation with suitable warnings.

      Quite often one method invokes other methods in the same class with parameters it has already checked for sanity, so the availability of such internal methods can also give a free speed boost even when external users do not invoke them directly.

        True.

        Looking at the setpixel method it should also be possible to write a variant that iterates over a list of values, e.g. [x1,y1,colour1, x2,y2,colour2, ...] or arrays of x, y and colour. Then the validity checks are called once per method invocation regardless of the number of colour assignments. That might be sufficient for the example in the OP.

      > Setting a flag on the object to not run validity checks is one option, and safer than a global variable.

      OP here, thanks for the replies. By "global" I meant to somehow disable all the slow sanity checks in Imager.pm. Methods like setpixel and getpixel are likely to be called a lot inside loops. Disabling the call to _valid_image on each pixel seems pretty safe so far and is a big gain. Since _valid_image is called in almost every method in Imager.pm they can all be fixed on line 658:

      sub _valid_image { return 1;
      The second hack that skips the call to _color is crude because it disables dwim on the color attribute of setpixel (color=>'blue') and requires it be supplied with an Imager::Color object (color=>Imager::Color->new('blue')). Doing that right would require more work but it hasn't broken my code so far while patching a live copy of Imager.pm. Probably because it was noticed long ago that dwim colors are to be avoided because setpixel is 2-3x faster when using color objects with rgb arrays like Imager::Color->new([255,0,0]).
Re: Double the speed of Imager->setpixel
by Anonymous Monk on Dec 12, 2022 at 16:19 UTC
    to generate an image involving about a million calls to Imager->setpixel

    ... almost certainly indicates a design flaw. Unless, of course, your pixels arrive at random unpredictable coordinates, to be scattered over a megapixel (or larger) canvas. Which I suspect they don't. (And even then it should be setscanline for each pixel with packed color as arg). Doco (emphasis mine):

    The getscanline() and setscanline() methods can work with pixels packed into scalars. This is useful to remove the cost of creating color objects, but should only be used when performance is an issue
    use strict; use warnings; use feature 'say'; use Time::HiRes 'time'; use Imager; sub get_RGB_triplet { map { int rand 256 } 0 .. 2 } { my $i = Imager-> new( xsize => 1000, ysize => 1000 ); my $t = time; for my $y ( 0 .. 999 ) { for my $x ( 0 .. 999 ) { my $c = Imager::Color-> new( get_RGB_triplet ); $i-> setpixel( x => $x, y => $y, color => $c ) } } say time - $t; } { my $i = Imager-> new( xsize => 1000, ysize => 1000 ); my $t = time; for my $y ( 0 .. 999 ) { my $packed_line = ''; for my $x ( 0 .. 999 ) { $packed_line .= pack 'C3x', get_RGB_triplet; } $i-> setscanline( y => $y, pixels => $packed_line ) } say time - $t; } __END__ 5.78085088729858 0.599802017211914
      > almost certainly indicates a design flaw. Unless, of course

      Wow that's a big difference! Thanks for the suggestion. The thing that's slowing down my program is sub _moveterm from this cool module Ham::WorldMap which draws the day/night regions on a world map:

      # _moveterm(wtab, noon, width, height) - update illuminated portion of + the globe. sub _moveterm { my ($wtab, $noon, $width, $height) = @_; my $illumMap = Imager->new(ysize => $height, xsize => $width); $illumMap = $illumMap->convert(preset => 'addalpha'); my $day = Imager::Color->new(255, 255, 255, 10); my ($i, $j, $oh, $nl, $nh); for ($i = 0; $i < $height; $i++) { if ($wtab->[$i] >= 0) { $nl = (($noon - ($wtab->[$i] / 2)) + $width) % $width; $nh = ($nl + $wtab->[$i]) - 1; $oh = ($nh - $nl) + 1; if (($nl + $oh) > $width) { for ($j = $nl; $j < $width; $j++) { $illumMap->setpixel(x => $j, y => $i, color => $da +y); } for ($j = 0; $j < ((($nl + $oh) - $width) + 1); $j++) +{ $illumMap->setpixel(x => $j, y => $i, color => $da +y); } } else { for ($j = $nl; $j < (($nl + $oh) + 1); $j++) { $illumMap->setpixel(x => $j, y => $i, color => $da +y); } } } } return $illumMap; }

        Patch to speed synopsis example execution time from 2.6 to 0.8s. Can't deny you the pleasure to replace third loop yourself, similarly to 1st and 2nd loops. Accidentally, that fragment (3d loop) isn't run with DateTime picked for synopsis, btw.

        --- WorldMap.pm.old Fri Jun 17 10:33:43 2016 +++ WorldMap.pm Tue Dec 13 10:27:14 2022 @@ -501,6 +501,7 @@ my $illumMap = Imager->new(ysize => $height, xsize => $width); $illumMap = $illumMap->convert(preset => 'addalpha'); my $day = Imager::Color->new(255, 255, 255, 10); + my $day_p = pack 'C4', 255, 255, 255, 10; my ($i, $j, $oh, $nl, $nh); for ($i = 0; $i < $height; $i++) { @@ -510,12 +511,10 @@ $oh = ($nh - $nl) + 1; if (($nl + $oh) > $width) { - for ($j = $nl; $j < $width; $j++) { - $illumMap->setpixel(x => $j, y => $i, color => $d +ay); - } - for ($j = 0; $j < ((($nl + $oh) - $width) + 1); $j++) + { - $illumMap->setpixel(x => $j, y => $i, color => $d +ay); - } + $illumMap->setscanline(x => $nl, y => $i, + pixels => $day_p x ($width - $nl - 1)); + $illumMap->setscanline(x => 0, y => $i, + pixels => $day_p x ($nl + $oh - $width)); } else { for ($j = $nl; $j < (($nl + $oh) + 1); $j++) { $illumMap->setpixel(x => $j, y => $i, color => $d +ay);
Re: Double the speed of Imager->setpixel
by Anonymous Monk on Jan 07, 2023 at 14:53 UTC
    Thanks again for the help. My attempt to speed up Ham::WorldMap by hacking Imager only doubled the speed. Swapping out setpixel for setscanline in sub _moveterm takes the synopsis code from 6 seconds to 1 local second. Here's my implementation:
    use Ham::WorldMap; # replace _moveterm with __moveterm for huge speed gain { no strict 'refs'; *{"Ham::WorldMap" . '::' . "_moveterm"} = \&__move +term } # __moveterm(wtab, noon, width, height) - update illuminated portion o +f the globe. # Speed fix from: https://www.perlmonks.org/index.pl?node_id=11148856 sub __moveterm { my ($wtab, $noon, $width, $height) = @_; my $illumMap = Imager->new(ysize => $height, xsize => $width); $illumMap = $illumMap->convert(preset => 'addalpha'); my $day_p = pack 'C4', 255, 255, 255, 10; for (my $i = 0; $i < $height; $i++) { if ($wtab->[$i] >= 0) { my $nl = (($noon - ($wtab->[$i] / 2)) + $width) % $width; my $nh = ($nl + $wtab->[$i]) - 1; my $oh = ($nh - $nl) + 1; if (($nl + $oh) > $width) { $illumMap->setscanline(x => $nl, y => $i, pixels => $day +_p x ($width - $nl)); $illumMap->setscanline(x => 0, y => $i, pixels => $day +_p x ($nl + $oh - $width + 1)); } else { $illumMap->setscanline(x => $nl, y => $i, pixels => $day +_p x ($oh + 1)); } } } return $illumMap; }

Log In?
Username:
Password:

What's my password?
Create A New User
Domain Nodelet?
Node Status?
node history
Node Type: perlmeditation [id://11148751]
Approved by Discipulus
help
Chatterbox?
and the web crawler heard nothing...

How do I use this?Last hourOther CB clients
Other Users?
Others romping around the Monastery: (6)
As of 2024-04-22 21:38 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    No recent polls found