This is a minor bug which can be the first link of a chain which can affect performance significantly.

user module tries to load the file object of a user picture even if it's set to 0, which means no picture.

In general this doesn't cause any issue. If a website also has entitycache module caching file entities, and memcache module as the cache backend, then entitycache will try to get a non-existent file from the cache, and with high loads this could lead to memcache stampede protection kicking in too easily, leading to long waiting times for semaphore release.

A workaround is to disable user pictures.

This should be fixed in memcache too (and possibly handled somehow in entitycache), but fixing it here does not hurt.

CommentFileSizeAuthor
#2 3006007-2.patch688 bytesmarco

Comments

marco created an issue. See original summary.

marco’s picture

marco commented
Status: Active » Needs review
StatusFileSize
new 3006007-2.patch688 bytes
poker10’s picture

poker10 commented

Seems like a good catch, thanks! In addition to memcache, this could hurt the performance especially if the site has lot of users, pictures are enabled, but most of users are without picture. And if the page is loading more users by user_load_multiple, then the generated entity query will be something like this: 

SELECT base.fid AS fid, base.uid AS uid, base.filename AS filename, base.uri AS uri, base.filemime AS filemime, base.filesize AS filesize, base.status AS status, base.timestamp AS timestamp FROM file_managed base WHERE (base.fid IN ('0', '0', '0', '0', '0', '0', '0', '0', '0', '0', '0', '0', '0', '254', '148'));

Which is not optimal. Without these zeroes the performance of this query will be way way better. Proposed array_filter() should solve the problem and use only FIDs which are not zero. +1 to this patch.

mcdruid’s picture

mcdruid commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +RTBM

This looks like a nice simple fix / optimisation.

Adding a test might be nice, but as we're not fixing an actual error in vanilla core ("In general this doesn't cause any issue") it might not be trivial to do so.

I'm happy for this to be committed as it is.

  • poker10 committed 047ef12 on 7.x 
    Issue #3006007 by marco: Don't try to load the user picture if it's not...
poker10’s picture

poker10 commented
Status: Reviewed & tested by the community » Fixed
Issue tags: -RTBM

Thanks all!

Status: Fixed » Closed (fixed)

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