Skip to:
Content

BuddyPress.org

Opened 3 years ago

Closed 2 years ago

Last modified 23 months ago

#8590 closed defect (bug) (fixed)

Various object cache groups are called upon, but were never actually created

Reported by: nickchomey's profile nickchomey Owned by: dcavins's profile dcavins
Milestone: 11.0.0 Priority: normal
Severity: normal Version: 9.1.1
Component: Performance Keywords: has-patch
Cc:

Description

I just submitted a Pull Request for the BuddyBoss Platform, but many of the bugs date back 5-7 years to BuddyPress. I don't have the time to figure out how to navigate the BuddyPress ticket/code submission process, but figured I'd notify you here.

https://github.com/buddyboss/buddyboss-platform/pull/3091/

Attachments (1)

8590.patch (5.0 KB) - added by imath 2 years ago.

Download all attachments as: .zip

Change History (16)

#1 @dcavins
3 years ago

  • Owner set to dcavins
  • Severity changed from major to normal
  • Status changed from new to assigned

Thanks for your ticket. I'll be happy to look at your patch for BuddyBoss and apply the parts that are pertinent to BP.

By the way, you have successfully navigated the ticket submission process. To submit code, you'd just attach a diff or patch to this ticket, created using git diff --no-prefix ... , so you're most of the way there, ha ha! Anyway, it's not GitHub, but it's not too hard.

Thanks for your suggestions!

#2 @espellcaste
3 years ago

  • Component changed from (not sure) to Performance
  • Keywords needs-patch needs-unit-tests added
  • Milestone changed from Awaiting Review to Up Next
  • Owner changed from dcavins to espellcaste
  • Status changed from assigned to accepted

I'm interested in looking at this further.

#3 @espellcaste
3 years ago

  • Milestone changed from Up Next to 11.0.0

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


2 years ago

#5 @nickchomey
2 years ago

p.s. My PR to BuddyBoss ended up being incorporated into a far larger Performance-focused overhaul of the entire BuddyBoss platform. They made immense changes to the object and static caching, which resulted in considerable performance improvements.

While my initial PR was simple, I'm not sure how you'd incorporate it all of this into BuddyPress. Re-forking BuddyBoss seems to be the right call here. Or just focusing on creating a high-quality, open-source theme for BuddyBoss.

@imath
2 years ago

#6 @imath
2 years ago

  • Keywords has-patch added; needs-patch needs-unit-tests removed

@nickchomey is right about cache groups. I believe we should commit the patch asap.

#7 @dcavins
2 years ago

This patch looks good to me. I agree with @nickchomey that we should also consider rolling in some of the cache improvements committed as part of the BuddyBoss ticket he linked to.

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


2 years ago

#9 @imath
2 years ago

In 13336:

Add missing global cache groups

Props nickchomey, dcavins

See #8590

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


2 years ago

#11 @espellcaste
2 years ago

@imath Could you catch me up on what's pending here?

#12 @imath
2 years ago

  • Owner changed from espellcaste to dcavins
  • Status changed from accepted to assigned

Hi @espellcaste

During last dev-chat, @dcavins volunteered to have the other cache improvements made by the buddyboss team member included into BuddyPress.

#13 @dcavins
2 years ago

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

I looked through the cache changes that were made in the BuddyBoss PR and didn't find any changes that I could backport. In several cases, cache mechanisms already existed in BP and hadn't been updated in the related code in BuddyBoss. In other cases, we already are caching objects at a higher level, and adding a redundant cache at the query occurrence wouldn't add anything.

It does look like BP_Blogs_Blog::get() could use some improved caching, like adding a bp_core_get_incremented_cache() call after the SQL statement is built. So maybe a new ticket should be added for just that component.

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


2 years ago

#15 @espellcaste
23 months ago

I'm currently in the process of changing the BP_Messages_Thread::get_messages method cache to use bp_core_get_incremented_cache.

Note: See TracTickets for help on using tickets.