Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More aggressive entity caching (Trac #4543) #4543

Closed
elgg-gitbot opened this issue Feb 16, 2013 · 6 comments
Closed

More aggressive entity caching (Trac #4543) #4543

elgg-gitbot opened this issue Feb 16, 2013 · 6 comments

Comments

@elgg-gitbot
Copy link

@elgg-gitbot elgg-gitbot commented Feb 16, 2013

Original ticket http://trac.elgg.org/ticket/4543 on 42396522-09-29 by ewinslow, assigned to ewinslow.

Elgg version: 1.7

$entities = elgg_get_entities(array(...)); // complicated db call

get_entity($entities[0]->guid); //hits db again!

get_entity($entities[0]->guid); //hits db again!

The above situation is common and pointless. We already loaded the entity, we should just be able to hit a local cache (i.e. it should not require a shared memory cache like memcache).

Once we have this optimization in place for elgg_get_entities, we can (ab)use it as the basis for other optimizations. E.g. currently when you are displaying the river, it's 1 query to get all the relevant river entries, and then 1 more query per owner, and object, plus another for each set of comments, plus another for each comment's owner, etc.

Instead, in elgg_get_river, we can preload all the entities referenced from the results by looping over them, pulling out owner_guid and object_guid into an array, and then doing elgg_get_entities(array('guids' => $guids)); Now $item->getOwnerEntity() and $item->getObjectEntity() will just hit the cache.

Since this is invisible to the API, we can include it in a bugfix release, so I'm slating for 1.8.x.

@elgg-gitbot
Copy link
Author

@elgg-gitbot elgg-gitbot commented Feb 17, 2013

ewinslow wrote on 42403183-10-13

#241

@elgg-gitbot
Copy link
Author

@elgg-gitbot elgg-gitbot commented Feb 17, 2013

trac user mrclay wrote on 42404337-05-18

Yes! I had an idea like the but was worried about the possibilities of having multiple instances of the same entity pointing to different objects. E.g. Using the constructor will always produce a distinct object. The answer to this is, that's already possible. It might be worth considering having all entity construction done via a public static method (making constructor private), which would mean the cache can always be checked and identical GUIDs always point to the same object (unless low on memory). I wonder if the patch could be a bit more clever instead of a static limit on the cache size. Even as is, this is awesome.

@elgg-gitbot
Copy link
Author

@elgg-gitbot elgg-gitbot commented Feb 17, 2013

trac user mrclay wrote on 42408932-05-15

Just sent Evan a PR with some fixes to the patch: ewinslow#1

@elgg-gitbot
Copy link
Author

@elgg-gitbot elgg-gitbot commented Feb 17, 2013

trac user mrclay wrote on 42452875-01-30

Latest PR for Evan's branch ewinslow#2
Adds missing invalidation points and caches during elgg_get_entities. Can get in 1.8.6?

@elgg-gitbot
Copy link
Author

@elgg-gitbot elgg-gitbot commented Feb 17, 2013

trac user Evan Winslow wrote on 42643932-12-02

Fixes #4543: Cache entities fetched with get_entity and elgg_get_entities
Changeset: 100d718

@elgg-gitbot
Copy link
Author

@elgg-gitbot elgg-gitbot commented Feb 17, 2013

Milestone changed to Elgg 1.8.9 by trac user mrclay on 42643936-02-14

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant
You can’t perform that action at this time.