Opened 7 years ago
Closed 9 months ago
#7666 closed enhancement (maybelater)
Load template loop functions only when the template loop is initialized
Reported by: |
|
Owned by: | |
---|---|---|---|
Milestone: | Priority: | normal | |
Severity: | normal | Version: | |
Component: | Core | Keywords: | has-patch dev-feedback 2nd-opinion |
Cc: |
Description
A lot of our codebase is for template functions.
We do not need to load this code if a template loop isn't initialized.
For example, you can't use the bp_activities()
or bp_get_activity_item_id()
functions without initializing the activity template loop, right? So it would make sense to only load this if the bp_has_activities()
function was called.
Attached patch is just an example of offshoring template functions used for the bp_has_activities()
template loop; it's a 56KB file savings and doesn't break unit tests.
Mutiply this technique for all our template loops across all components (especially for the Groups component) and we can definitely trim more fat.
This would be noticeable on multisite installs when BP isn't on the root blog.
Attachments (1)
Change History (8)
This ticket was mentioned in Slack in #buddypress by r-a-y. View the logs.
7 years ago
#3
@
7 years ago
My technique doesn't require new APIs. It just uses a require_once
line near the end of the loop.
There shouldn't be a backward-compatibility issue unless developers use functions that are not meant to be used outside the loop. We can try to be conservative with what is offshored though.
It's hard to do benchmarks without writing a patch for the whole codebase, but it would save loading thousands of lines of code. The example patch already saves more than 2000+ lines of code alone and that is just for the bp_has_activities()
loop.
#4
@
7 years ago
We have recent tickets where people have tried to use template loop functions outside of a template loop, and have asked for us to add optional arguments to those functions (for IDs, etc), so that they can be used outside a loop. This suggests we have API architecture problems somewhere for those things (I can dig out the ticket references later) but also suggests that the change proposed here is definitely high-risk.
#5
@
7 years ago
- Keywords 2nd-opinion added
I think the risk of this change is too high; #7652 is a discussion ticket about the opposite (having template loop functions used everywhere) because feedback we have is that people already do it. It wouldn't surprise me if people were manually setting a global to use a specific template function.
Recording what @johnjamesjacoby said on Slack: