Page MenuHomePhabricator

perftweaks: also wrap branchmap.updatecache
AbandonedPublic

Authored by quark on Nov 18 2017, 12:56 AM.

Details

Reviewers
durham
Group Reviewers
Restricted Project
Summary

branchmap.branchcache.update is not the only code path triggering branchmap
update. branchmap.updatecache also matters. Let's also change it too.

Diff Detail

Repository
rFBHGX Facebook Mercurial Extensions
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

quark created this revision.Nov 18 2017, 12:56 AM
Herald added a reviewer: Restricted Project. · View Herald TranscriptNov 18 2017, 12:56 AM
durham requested changes to this revision.Nov 21 2017, 10:15 AM
durham added a subscriber: durham.

Can you provide a more detailed description of why this is safe and correct? The branch cache is a funky bit of logic. I know we only use it for heads these days, but I want to be sure we aren't breaking that functionality.

What kind of testing did we do here?

This revision now requires changes to proceed.Nov 21 2017, 10:15 AM
quark abandoned this revision.Nov 21 2017, 10:18 PM

I thought this is similar to _branchmapupdate but that might be wrong. With D1473, this might be unnecessary.