Page MenuHomePhabricator

Reduce the cost of loading large numbers of macros
ClosedPublic

Authored by epriestley on Sep 6 2014, 12:21 AM.
Tags
None
Referenced Files
F14007074: D10428.diff
Mon, Oct 28, 11:04 PM
F14002712: D10428.id25087.diff
Fri, Oct 25, 9:33 PM
F14001316: D10428.id.diff
Fri, Oct 25, 6:21 AM
F13981960: D10428.id25088.diff
Sat, Oct 19, 8:15 PM
F13974148: D10428.diff
Fri, Oct 18, 4:18 AM
Unknown Object (File)
Tue, Oct 8, 11:59 PM
Unknown Object (File)
Oct 2 2024, 12:14 PM
Unknown Object (File)
Sep 24 2024, 4:06 PM

Details

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
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

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.
csilvers edited edge metadata.

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
epriestley updated this revision to Diff 25088.

Closed by commit rPb772a2b92af5 (authored by @epriestley).