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
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

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
591–592

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.

621–623

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
591–592

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

621–623

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
604

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
604

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