Skip to:

Opened 7 years ago

Closed 5 years ago

#7435 closed enhancement (fixed)

XProfile caching: The Next Generation

Reported by: boonebgorges's profile boonebgorges Owned by: boonebgorges's profile boonebgorges
Milestone: 5.0.0 Priority: normal
Severity: normal Version:
Component: Extended Profile Keywords: has-unit-tests has-patch
Cc: dcavins


I'm currently building a member directory that uses a tabular format, populated by xprofile data. Pulling up this data is painful, for a couple of reasons:

  • BP_XProfile_Field::get_id_from_name() is uncached, which means that I have to build my own name/id map to avoid all the extra lookups. This makes it unfeasible to use xprofile_get_field_data()
  • BP_XProfile_Group::get() has a few uncached queries, which makes building the map fairly expensive
  • My alternative strategy of calling BP_XProfile_ProfileData::get_all_for_user() in the loop ran into some of the same problems

Let's think about ways to unify the caching strategy (or lack thereof) in these areas with what's happening through the rest of XProfile.

Attachments (1)

7435.diff (17.4 KB) - added by boonebgorges 6 years ago.

Download all attachments as: .zip

Change History (14)

#1 @dcavins
7 years ago

  • Cc dcavins added

#2 @r-a-y
7 years ago

  • Milestone changed from 2.9 to 3.0

Moving to 3.0, but bump to FR if needed.

#3 @DJPaul
6 years ago

  • Milestone changed from 3.0 to Awaiting Contributions

6 years ago

#4 @boonebgorges
6 years ago

  • Milestone changed from Awaiting Contributions to 4.0

I've started work on this and would like to squeeze at least part of it into 4.0, if possible.

7435.diff breaks up BP_XProfile_Group::get() somewhat, so that the group ID query and the field ID queries take place in different methods: get_group_ids() and get_group_field_ids(). They're meant for internal use, and are public only so that it's easier to test them. Each of them has its own caching layer, both of which are tied to the bp_xprofile_groups incrementor. I'm using the same incrementor for each, because any changes to *either* groups or fields can result in the queries being thrown off (because of hide_empty_groups etc). I'm then using the SQL statement to generate a cache key.

I've made an attempt to cover the invalidation cases. I've erred on the side of too-much-invalidation: we bust the incrementor anytime a group or a field is saved or deleted. Updating field/group order also needs its own invalidation, because it doesn't run through save().

This thought process - especially invalidation - could use another set of eyes. Maybe @dcavins ? I can work on more caching, but what's suggested here would be a good start.

#5 @dcavins
6 years ago

I'm happy to help, Boone. I will test your patch next week for starters, then we can go from there. Thanks for improving the caching in BP everywhere.

This ticket was mentioned in Slack in #buddypress by boone. View the logs.

6 years ago

This ticket was mentioned in Slack in #buddypress by dcavins. View the logs.

6 years ago

#8 @boonebgorges
6 years ago

  • Milestone changed from 4.0 to Up Next

#9 @boonebgorges
5 years ago

  • Milestone changed from Up Next to 5.0.0

#10 @boonebgorges
5 years ago

I've reviewed the patch and I think it's ready to go. I'd like to get it in so that we can work on some additional improvements in this area.

#11 @boonebgorges
5 years ago

In 12317:

XProfile: Add caching to group and field queries.

See #7435.

#12 @espellcaste
5 years ago

  • Keywords has-unit-tests has-patch added
  • Owner set to boonebgorges
  • Status changed from new to reviewing

@boonebgorges Are you done done here? Since you committed code...

#13 @boonebgorges
5 years ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

There's more I want to do, but I'll do it in a future release.

Note: See TracTickets for help on using tickets.