Jump to content

Max execution time exceeded in Pageimage.php for specific pages


snck
 Share

Recommended Posts

Hi there,

I am having a strange problem. I have a bunch of sites that have similar content (text, images, map markers), but I am not able to edit two specific pages in the backend. I always get the following error (or 503 service unavailable):

Maximum execution time of 30 seconds exceeded (Zeile 520 in [...]/wire/core/Pageimage.php)

Line 520 in Pageimage.php:

$imagick->readImage($filename);

All of the images used on all of the pages are SVG drawings and I have no clue why there are no problems with the majority of pages but just two of them. Maybe one of you guys has experienced a similar problem with ImageMagick and SVGs?

I have debug mode enabled, but I only get these errors and nothing more specific. Is there any smart way to find the source of the error?

In the assets folder for all of the pages there are only SVGs so I expect no conversion to be done. If no conversion is happening, why would the script run into the max execution time? 

I appreciate your thoughts on this.

Cheers,
Flo

Link to comment
Share on other sites

  • 1 month later...
On 4/24/2020 at 10:24 AM, kongondo said:

@snck

Did you get this resolved?

I further investigated the issue by reviewing all of the SVG files used on the pages that have thrown errors and comparing them to files from pages that were editable without problems. I realised that the guys who generated the files (architecture students) used different software in their process (Adobe Illustrator, Vectorworks etc.) which led to really complex files. I ran some of them through an online SVG minification tool which helped to resolve the issue in some cases and instructed the students to develop a kind of maximum complexity guideline for their files.

To make the backend usable again and offer them a chance to exchange their "bad" files, it raised the max_execution_time temporarily in the .htaccess:

php_value max_execution_time 120

 

  • Like 2
Link to comment
Share on other sites

@snck,

Thanks for reporting back.

12 minutes ago, snck said:

used different software in their process (Adobe Illustrator, Vectorworks etc.) which led to really complex files.

I don't know much about SVGs, excuse my ignorance please. Was the issue the complexity of the files or the size of the files? I am guessing though that complex files would probably also be large in size. 

Thanks.

  • Like 1
Link to comment
Share on other sites

18 minutes ago, kongondo said:

@snck,

Thanks for reporting back.

I don't know much about SVGs, excuse my ignorance please. Was the issue the complexity of the files or the size of the files? I am guessing though that complex files would probably also be large in size. 

Thanks.

Size alone should not be a problem (considering that I have used a lot of much bigger JPEG images with IMagick). In some cases the SVGs consisted of many 1000 paths and a lot of class definitions for their styling. I do not know the how efficient the rasterizing in IMagick works, but I guess it was the bootleneck in this specific case.

  • Like 2
Link to comment
Share on other sites

In reality, that getImageInfoSVG() method should probably try to get the dimensions from the viewbox attribute in the SVG (if actual width and height attributes are not available), before it resorts to using imagick to get them. That would remove the need to rasterize just to get the dimensions.

  • Like 2
Link to comment
Share on other sites

I have personally found errors related to imagick and pageimage.php to be misleading as the failure can happen at the server / imagick module level and can be due to issues with multi-threading. Are you in a multi-threaded (ie two or more cpu cores) environment and are the timeouts happening much faster than 30 seconds?

  • Like 1
Link to comment
Share on other sites

  • 1 year later...
On 4/29/2020 at 7:52 PM, adrian said:

In reality, that getImageInfoSVG() method should probably try to get the dimensions from the viewbox attribute in the SVG (if actual width and height attributes are not available), before it resorts to using imagick to get them. That would remove the need to rasterize just to get the dimensions.

@adrian Again had problems with this and got back to your idea. Thank you so much!

This quick fix might be not the most beautiful/efficient way, but worked fine for me (and might also for someone else):

/**
  * Gets the image info/size of an SVG
  *
  * Returned width and height values may be integers OR percentage strings.
  *
  * #pw-internal
  *
  * @param string $filename Optional filename to check
  * @return array of width and height
  *
  */
protected function getImageInfoSVG($filename = '') {
  $width = 0;
  $height = 0;
  if(!$filename) $filename = $this->filename;
  $xml = @file_get_contents($filename);
  
  if($xml) {
    $a = @simplexml_load_string($xml)->attributes();
    if((int) $a->width > 0) $width = (int) $a->width;
    if((int) $a->height > 0) $height = (int) $a->height;

    // start viewbox fix
    if(!$width || !$height){
      if($a->viewBox != ""){
        // get rid of commas: replace them with spaces
        $a->viewBox = str_replace(",", " ", $a->viewBox);
        // get rid of (now possible) double spaces: replace them with a single space
        $a->viewBox = str_replace("  ", " ", $a->viewBox);
        // get single values
        $viewbox = explode(" ", $a->viewBox);
        
        if(count($viewbox) === 4){
          // we need 4 values
          $min_x = (float) $viewbox[0];
          $min_y = (float) $viewbox[1];
          $max_x = (float) $viewbox[2];
          $max_y = (float) $viewbox[3];

		  $width = $max_x - $min_x;
          $height = $max_y - $min_y;
        }
      }
    }
    // end viewbox fix
  }
  
  if((!$width || !$height) && (extension_loaded('imagick') || class_exists('\IMagick'))) {
    try {
      $imagick = new \Imagick();
      $imagick->readImage($filename);
      $width = $imagick->getImageWidth();
      $height = $imagick->getImageHeight();
    } catch(\Exception $e) {
      // fallback to 100%
    }
  }
  
  if($width < 1) $width = '100%';
  if($height < 1) $height = '100%';
  
  return array(
    'width' => $width, 
    'height' => $height
  ); 
}

 

  • Like 1
Link to comment
Share on other sites

Hi @snck - nice work. I really do think this should be incorporated into the core, although I am a bit confused by the calculation method you have for getting the width / height. I feel like the width and height should be take directly from [2] and [3]. Even if the min_x or min_y aren't set to 0, surely that doesn't change the width of the canvas area that the illustration is on which is what we want the width / height of.

Do you agree, or am I missing something?

We need to get Ryan's attention on this - maybe a Github issue?

 

 

  • Like 1
Link to comment
Share on other sites

2 hours ago, adrian said:

Hi @snck - nice work. I really do think this should be incorporated into the core, although I am a bit confused by the calculation method you have for getting the width / height. I feel like the width and height should be take directly from [2] and [3]. Even if the min_x or min_y aren't set to 0, surely that doesn't change the width of the canvas area that the illustration is on which is what we want the width / height of.

Do you agree, or am I missing something?

We need to get Ryan's attention on this - maybe a Github issue?

 

 

Hey @adrian, you are right. I was thinking of the viewBox as a set of coordinates in x/y dimension that are defining the corners of the viewbox relative to the origin of the coordinate system (this is why i called them max_x and max_y), which is obviously wrong. It should look like this:

/**
  * Gets the image info/size of an SVG
  *
  * Returned width and height values may be integers OR percentage strings.
  *
  * #pw-internal
  *
  * @param string $filename Optional filename to check
  * @return array of width and height
  *
  */
protected function getImageInfoSVG($filename = '') {
  $width = 0;
  $height = 0;
  if(!$filename) $filename = $this->filename;
  $xml = @file_get_contents($filename);
  
  if($xml) {
    $a = @simplexml_load_string($xml)->attributes();
    if((int) $a->width > 0) $width = (int) $a->width;
    if((int) $a->height > 0) $height = (int) $a->height;

    // start viewbox fix
    if(!$width || !$height){
      if($a->viewBox != ""){
        // get rid of commas: replace them with spaces
        $a->viewBox = str_replace(",", " ", $a->viewBox);
        // get rid of (now possible) double spaces: replace them with a single space
        $a->viewBox = str_replace("  ", " ", $a->viewBox);
        // get single values
        $viewbox = explode(" ", $a->viewBox);
        
        if(count($viewbox) === 4){
          // we need 4 values, even though we are just using 2
		  $width = (float) $viewbox[2];
          $height = (float) $viewbox[3];
        }
      }
    }
    // end viewbox fix
  }
  
  if((!$width || !$height) && (extension_loaded('imagick') || class_exists('\IMagick'))) {
    try {
      $imagick = new \Imagick();
      $imagick->readImage($filename);
      $width = $imagick->getImageWidth();
      $height = $imagick->getImageHeight();
    } catch(\Exception $e) {
      // fallback to 100%
    }
  }
  
  if($width < 1) $width = '100%';
  if($height < 1) $height = '100%';
  
  return array(
    'width' => $width, 
    'height' => $height
  ); 
}

@ryan, maybe something like this could be a useful addition to the core? 

  • Like 1
Link to comment
Share on other sites

I'm not sure if I understand everything 100% but as I was mentioned here (seems to be removed now) I read the thread and it reminds me a bit of a problem we had recently when using SVGs on one of our websites. SVGs didn't show up on the frontend and also the PW backend had problems (it showed them in the small preview but not when viewed via the lightbox). The problem was that the SVGs did not have dimensions specified, so I'm wondering if that could be related?

Could maybe PW check SVGs on upload and add with and height automatically if they are not present? Would that also fix your issue?

Please ignore if that is not related or nonsense ? 

Link to comment
Share on other sites

Just now, bernhard said:

Please ignore if that is not related or nonsense

It's not nonsense, but SVGs don't need to have width / height attributes. It's perfectly acceptable for these to be missing, but it can result in some display issues depending on how they are output. I don't think PW should automatically add these (at least without it being optional), but on some sites I do this via a hook because I do want to ensure those attributes are set. If they're not set, I get them from the viewbox and add them - similarly to the way @snck is showing above.

 

Link to comment
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now
 Share

×
×
  • Create New...