As a relative newcomer, I'm just starting to get into Drupal code. I'm no PHP stud and not not savvy with MySQL nuances either. I'd like to use this avatar_selection module as an ongoing opportunity to try to improve performance for selecting and rendering the image list in the Manage page.

I just took a look at the code and here are some general thoughts.
- I see that you're doing some processing on each image as it's rendered. It might be best to do that sort of processing in a separate loop, or perhaps offer a checkbox to allow the user to decide whether the extra processing will be done during their session.
- When generating $avatars_list, the foreach loop copies the entire array, in memory and into the new $avatars_indexed array. There has to be a ton of overhead there with the 7500 images I'm sending into there (total memory footprint = 22500 images consuming about 22MB of RAM every time a page is rendered!). I tried changing $image to &$image but there wasn't any obvious effect.
- Note that the foreach statement seems to have the arguments backwards:
foreach ($avatars as $image => $name) should be
foreach ($avatars as $name => $image) no ? (PHP doc on foreach)
- I also changed the foreach to a for-loop with no perceived improvement.
- There are two indexes on the avatar_selection table, PRIMARY, and 'avatar'. Since one is redundant I removed PRIMARY. Any objection?
- We are planning to use OG and I appreciate the code in there, but we're not using weights and not assigning a unique name to avatars, so as a test I changed the selection to Order By avatar (which is indexed where 'weight' and 'name' are not), and I commented out anything related to those fields and replaced them with 'avatar'. There was no noticable improvement in performance.

Remember that these are just local experiments, not change proposals. Considering all of the apps in the world that operate on images I'm sure there is a way to optimize the selection of large sets of images and rendering of subsets one page at a time.

Comments welcome here about anything related to performance. Sound like a plan?

Comments

stella’s picture

Taking each of your points individually:

  1. Processing on each image - I'm not totally sure what you mean here, can you be more specific. All the code in _avatar_selection_image_list() is needed to determine whether it should be in the list returned or not.
  2. Generating $avatar_list - this code is actually obsolete as of the last release, so I've removed this bit - see the latest dev release (available later today).
  3. Backward foreach arguments - no the arguments are the correct way around, $image is the key and $name is the value - as set on online 393. Your change here could cause the module to function incorrectly if you have assigned any names to the avatars.
  4. "foreach" loops are actually faster than "for" loops when reading from a hash array. We aren't modifying it in this case, just creating new ones. This code has been removed in any case, see point 2.
  5. There's a primary key and a unique key on the table. It probably doesn't matter if you remove one of them, but removing indexes from a table will generally not improve performance, instead the opposite could happen.
  6. Sure, removing functionality could speed things up, but it's not something I can do to the module. It's fine if you do it to your local copy, though it will make it harder to upgrade.

Cheers,
Stella

stella’s picture

Status: Active » Fixed

Try the latest dev release (available later today). I've changed the function which fetches the list of avatars. It may help.

Cheers,
Stella

Anonymous’s picture

Status: Fixed » Closed (fixed)

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

Starbuck’s picture

Status: Closed (fixed) » Active

Re-opening: Tried latest Dev cut from Aug 05. I see the avatar_selection table itself is updated with all image files, but table avatar_selection_roles does not exist so invoking the code after update results in errors. Please advise.

Thanks for your efforts.

stella’s picture

Status: Active » Fixed

Please run update.php, which you should be doing every time you upgrade any module.

Starbuck’s picture

I disabled, updated, re-enabled, updated, but I didn't do a disable/uninstall before updating the module, so hook_install wasn't run. I'll take another shot at this tomorrow.

It looks like all files get deleted from the directory during uninstall. My image directory is a soft link to a path shared by my forum, so deleting all of the images isn't desirable, and the folder isn't going to be deleted. I guess I'll need to make a site-specific mod to avoid that part of the code - agreed?

Thanks again.

stella’s picture

You shouldn't need to uninstall the module in order to get the new tables. If update.php is not adding the two new tables (avatar_selection_roles and avatar_selection_og), then tell me the schema_version number from the system table for the avatar selection module. If it is set to 6002, you may need to grab the latest dev release from today.

Cheers,
Stella

Starbuck’s picture

Yes, it is 6002. I'm just getting familiar with Drupal internals but I wouldn't expect hook_install to execute on an update. Whatever the case, I will wait another day just in case the dev code is in a precarious state and then load the latest build. If there are issues with this specific installation, I don't want to bother you with them. I will get the latest dev fully installed and we'll work forward from there, focusing on the module.

Regards and thanks as always.

stella’s picture

hook_install isn't run on an update, but there is a hook_update_N function (avatar_selection_update_6003()) which should be run when you run update.php in the latest dev release. Please let me know if the latest dev release fixes the issue for you.

stella’s picture

Released in Avatar Selection 6.x-1.5 and 5.x-2.7.

Cheers,
Stella

Anonymous’s picture

Status: Fixed » Closed (fixed)

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