Page MenuHomePhabricator

Denormalize Diff PHIDs onto Revisions
ClosedPublic

Authored by epriestley on Nov 1 2017, 5:50 PM.
Tags
None
Referenced Files
F13225598: D18756.id45017.diff
Sun, May 19, 4:10 PM
F13224095: D18756.id45022.diff
Sun, May 19, 6:21 AM
F13196384: D18756.diff
Sun, May 12, 11:13 PM
Unknown Object (File)
Fri, May 3, 3:56 AM
Unknown Object (File)
Mon, Apr 29, 4:05 PM
Unknown Object (File)
Wed, Apr 24, 11:54 PM
Unknown Object (File)
Apr 19 2024, 6:57 PM
Unknown Object (File)
Apr 11 2024, 8:10 AM
Subscribers
None

Details

Summary

Ref T12539. See PHI190. Currently, each Diff has a revisionID, but Revisions do not point at the current active diff. To find the active diff for a given revision, we need to issue a separate query.

Furthermore, this query is inefficient for bulk loads: if we have a lot of revisions, we end up querying for all diff IDs for all those revisions first, then selecting the largest ones and querying again to get the actual diff objects. This strategy could likely be optimized but the query is a mess in any case.

In several cases, it's useful to have the active diff PHID without needing to do a second query -- sometimes for convenience, and sometimes for performance.

T12539 is an example of such a case: it would be nice to refine the bucketing logic (which only depends on active diff PHIDs), but it feels bad to make the page heavier to do it.

For now, this is unused. I'll start using it to fix the bucketing issue, and then we can expand it gradually to address other performance/convenience issues.

Test Plan
  • Ran migrations, inspected database, saw sensible values.
  • Created a new revision, saw a sensible database value.
  • Updated an existing revision, saw database update properly.

Diff Detail

Repository
rP Phabricator
Branch
adiff1
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 18795
Build 25331: Run Core Tests
Build 25330: arc lint + arc unit