Skip to:

Opened 7 years ago

Closed 7 years ago

#7404 closed enhancement (fixed)

Caching for `xprofile_get_field_from_name()`

Reported by: boonebgorges's profile boonebgorges Owned by: boonebgorges's profile boonebgorges
Milestone: 2.8 Priority: normal
Severity: normal Version:
Component: Extended Profile Keywords: has-patch 2nd-opinion


xprofile_get_field_from_name() can sometimes be called many times on a page load - especially when you're doing a lot of fetching of the Name field. Let's add some caching.

Attachments (1)

7404.diff (3.7 KB) - added by boonebgorges 7 years ago.

Download all attachments as: .zip

Change History (5)

7 years ago

#1 @boonebgorges
7 years ago

  • Keywords has-patch 2nd-opinion added

I chose to go with an incrementor cache, so that all field-by-name caches are invalidated whenever a field is updated. I went this route for the following reason. If you have a field 'Foo', it'll be cached with a key 'Foo'. If you then change the name to 'Bar', you have to know that the previous name was 'Foo' in order to invalidate the cache. The way we do field updates - by directly setting the name property on BP_XProfile_Field objects - makes this difficult or impossible to do.

Feedback on the approach would be appreciated. See 7404.diff

#2 @dcavins
7 years ago

This patch works well for me. I was wondering why you went this route instead of adding caching at a lower query level, like the recent improvements to BP_Groups_Group::get(). It looks like this class could use some updating, generally.

This seems like a nice performance improvement until we get a chance to rethink the Xprofile classes. Thanks!

#3 @boonebgorges
7 years ago

@dcavins Thanks for having a look. I guess I was trying to keep logic out of the query class, but you're right that we keep caching logic there in other places, so let's do that.

#4 @boonebgorges
7 years ago

  • Owner set to boonebgorges
  • Resolution set to fixed
  • Status changed from new to closed

In 11352:

XProfile: Add caching for fetching a field ID by name.

Fixes #7404.

Note: See TracTickets for help on using tickets.