Reduce the cost of loading large numbers of macros
ClosedPublic
Actions

Authored by epriestley on Sep 6 2014, 12:21 AM.

Details

Reviewers

btrahan
csilvers

Maniphest Tasks

T6013: Image Thumbnails not visible when logged out

Commits

Restricted Diffusion Commit
rPb772a2b92af5: Reduce the cost of loading large numbers of macros

Summary

Ref T6013. I accidentally made this cost explosviely huge when fixing macros for logged out users in D10411.

Specifically, we'd load all the macros, which would load all the files, which would load all the macros (to do policy checks), which would fill out of cache I think (but maybe only some of the time?). Anyway, bad news.

Instead, only load the files if we need them.

Test Plan

Viewed macro main page, macro detail, used a macro, used a meme, edited a macro, edited audio.

Diff Detail

Repository

rP Phabricator

Branch

macrocost

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 2446
Build 2450: [Placeholder Plan] Wait for 30 Seconds

Event Timeline

epriestley updated this revision to Diff 25087.Sep 6 2014, 12:21 AM

epriestley retitled this revision from to Reduce the cost of loading large numbers of macros.

epriestley updated this object.

epriestley edited the test plan for this revision. (Show Details)

epriestley added reviewers: btrahan, csilvers.

epriestley added a task: T6013: Image Thumbnails not visible when logged out.

Herald added subscribers: sophiebits, epriestley. · View Herald TranscriptSep 6 2014, 12:21 AM

Harbormaster completed remote builds in B2446: Diff 25087.Sep 6 2014, 12:22 AM

I'm happy to approve this, in the interests of fixing our install!

I actually understand the diff; the only thing I can't really review competently is whether there are any places you needed to add needsFiles(true) that are missed currently.

This revision is now accepted and ready to land.Sep 6 2014, 12:27 AM