Currently we get a file_exists() and a @getimagesize() for each and every call to theme_image().
The images that are shipped with the chessboard project don't need this. Even for images from other places we can really lower the number of calls to these functions.

We could provide a way to pass hardcoded attributes to theme_chessboard() in a new code branche... But right now I just want to fix this in a way that helps porting to Drupal 7 (rc1).

Comments

eric_a’s picture

StatusFileSize
new8.12 KB

This patch brings down the number of calls from about 16 to 0 or 16 to 4 depending on where the images come from.

eric_a’s picture

Status: Active » Needs review

Status: Needs review » Needs work
Issue tags: -Performance

The last submitted patch, chessboard-io-995762-1.patch, failed testing.

eric_a’s picture

Status: Needs work » Needs review

#1: chessboard-io-995762-1.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, chessboard-io-995762-1.patch, failed testing.

eric_a’s picture

Status: Needs work » Needs review

#1: chessboard-io-995762-1.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, chessboard-io-995762-1.patch, failed testing.

eric_a’s picture

Status: Needs work » Needs review

#1: chessboard-io-995762-1.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +Performance

The last submitted patch, chessboard-io-995762-1.patch, failed testing.

eric_a’s picture

eric_a’s picture

Status: Needs work » Fixed
StatusFileSize
new8.81 KB

Committed #11.

#1 passes tests locally. Even so there is a problem:

+    // Append class atttribute.
+    foreach ($group_attributes as $group => $attributes) {
+      $group_attributes[$group]['class'] = 'chessboard-' . $group;
+    }

This will assign the class attrribute if width/ height attributes were assigned, which is not always the case if for some reason the size cannot be determined. I'm postponing writing a test for such cases until we remove old API functions and introduce an up-to-date API (and UI) for changing the image directory.

New patch is written to fix this and be more readable. Passes test locally.

eric_a’s picture

Status: Fixed » Needs review
StatusFileSize
new1.03 KB

These two comments are just... not good.
Automated testing should be back now... Let's see.

eric_a’s picture

Issue tags: -Performance

#1: chessboard-io-995762-1.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +Performance

The last submitted patch, chessboard-io-995762-12.patch, failed testing.

eric_a’s picture

Status: Needs work » Fixed

Yes! Drupal 6 install code is fixed and patch testing is back again *after* committing to the branch. Of course the patches do not apply anymore after having been committed. (Tested locally. And the branch is fine according to qa.)

Status: Fixed » Closed (fixed)
Issue tags: -Performance

Automatically closed -- issue fixed for 2 weeks with no activity.