Page MenuHomePhabricator

repair: improve performance of detection of revisions affected by issue6528
ClosedPublic

Authored by Alphare on Aug 6 2021, 6:12 AM.

Details

Summary

Explanations inside the patch. I've tested this on Mozilla-Central and it's
5 times faster than the naive approach on my laptop.

Diff Detail

Repository
rHG Mercurial
Branch
stable
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

Alphare created this revision.Aug 6 2021, 6:12 AM
marmoute requested changes to this revision.Aug 6 2021, 6:57 AM
marmoute added a subscriber: marmoute.

The overall logic seems fine, but the affected_chain cache and the expected_metadata_len seems wrong,

mercurial/revlogutils/rewrite.py
589–590

Should we check if they are affected ? or if they have metadata ? because if we have no diff compared to a revision that has metadata, we do have metadat and might have our parent swapped, unlike the delta base.

619–621

I am confused about this check, why are we using filename here ?

This revision now requires changes to proceed.Aug 6 2021, 6:57 AM
Alphare added inline comments.Aug 6 2021, 8:37 AM
mercurial/revlogutils/rewrite.py
589–590

I'm not sure I understand what you're asking

619–621

Right, we should simply use a single placeholder byte (for the shortest possible copy source), and not the filename, I forgot about that.

Alphare updated this revision to Diff 29836.Aug 6 2021, 1:50 PM

Looks overall good, but for the comment I made were we seems to do extra, unnecessary, revision restoration. I'll have a look tomorrow to check if we can safely remove this (and if not, why).

mercurial/revlogutils/rewrite.py
602

This seems to trigger the slow path for a non linear revision and might slow things down. Why do we needs it ?

marmoute added inline comments.Aug 7 2021, 6:12 AM
mercurial/revlogutils/rewrite.py
602

Dropping this line does not affect the correctness of the result and results in a significant speed up : 2m53 → 1m43

I am sending an updating this line dropped.

marmoute updated this revision to Diff 29841.Aug 7 2021, 6:13 AM
marmoute accepted this revision.Aug 7 2021, 6:27 AM
baymax updated this revision to Diff 29844.Aug 7 2021, 9:47 AM

✅ refresh by Heptapod after a successful CI run (🐙 💚)
⚠ This patch is intended for stable ⚠

baymax updated this revision to Diff 29856.Aug 7 2021, 6:09 PM

✅ refresh by Heptapod after a successful CI run (🐙 💚)
⚠ This patch is intended for stable ⚠

pulkit accepted this revision.Aug 10 2021, 8:16 AM
This revision is now accepted and ready to land.Aug 10 2021, 8:16 AM