Details
- Reviewers
fabe chad - Group Reviewers
Blessed Reviewers - Maniphest Tasks
- T2453: Populate the blame cache when parsing commits
T2450: Build a blame cache - Commits
- Restricted Diffusion Commit
rPe8d3071452f0: Implement a git blame cache
View files in Diffusion with enabled blame
Diff Detail
- Repository
- rP Phabricator
- Branch
- arcpatch-D10600_1
- Lint
Lint Passed - Unit
Test Failures - Build Status
Buildable 3311 Build 3318: [Placeholder Plan] Wait for 30 Seconds
| Time | Test | |
|---|---|---|
| 0 ms | testCelerityMaps | |
| 0 ms | testApplicationsInstalled | |
| 0 ms | testAuthorName | |
| 0 ms | testBranchFilter | |
| 0 ms | testConduitMethods | |
| View Full Test Results (1 Failed · 17 Passed) |
Event Timeline
When i look at the blame xhr request in the dark console i can still see a query selecting all the commits by their identifiers instead of the id's (which take a long time).
I suspect this is for showing the connected revisions to the commits. I couldn't quite find where that is executed. Maybe someone could push me to the right file?
However changing this behavior might need an conduit api change (adding an extra field with the revision information)
blame for phutil_map now takes ~6s instead of ~20s
There's still some weird stuff going on with $handlers / $author_phids in the controller but that won't affect performance
- add new db schema config
- populate blame cache on reparse
- replaced some code with an already existing conduit call
- rebasing and fix db validity issues
- add new db schema config
- populate blame cache on reparse
- replaced some code with an already existing conduit call
- rebasing and fix db validity issues
- reabsing
I'm finally doing some Diffusion work, let me see if I can bring this the last mile. Thanks for the patch!
Sure :)
I remember that the biggest performance gain was coming from using PHIDs when selecting all the commit info in the FileController instead of the commit identifiers.
Showing the blame info on the library_map file was a good benchmark.
You were right, MySQL was running the old query really inefficiently. I think D14962 should fix it.
- Use lastmodifiedquery (which is a little more tailored/efficient) instead of historyquery.
- Use standard cache infrastructure (this is slightly less efficient but much simpler and feels like a good place to start).
- Don't try to do the reparse/prefill stuff yet -- we could pursue that later.
With other optimizations, this drops __phutil_library_map__.php in blame + no color mode from about 3.5 seconds to about 800ms (when we hit cache) for me locally.