Skip to:
Content

BuddyPress.org

Opened 3 years ago

Closed 13 months ago

#7435 closed enhancement (fixed)

XProfile caching: The Next Generation

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

Description

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 19 months ago.

Download all attachments as: .zip

Change History (14)

#1 @dcavins
3 years ago

  • Cc dcavins added

#2 @r-a-y
3 years ago

  • Milestone changed from 2.9 to 3.0

Moving to 3.0, but bump to FR if needed.

#3 @DJPaul
2 years ago

  • Milestone changed from 3.0 to Awaiting Contributions

@boonebgorges
19 months ago

#4 @boonebgorges
19 months 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
19 months 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.


18 months ago

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


18 months ago

#8 @boonebgorges
18 months ago

  • Milestone changed from 4.0 to Up Next

#9 @boonebgorges
17 months ago

  • Milestone changed from Up Next to 5.0.0

#10 @boonebgorges
16 months 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
16 months ago

In 12317:

XProfile: Add caching to group and field queries.

See #7435.

#12 @espellcaste
13 months 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
13 months 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.