Page MenuHomePhabricator

transactions: convert changes['phases'] to list of ranges
ClosedPublic

Authored by joerg.sonnenberger on Feb 15 2020, 4:31 PM.

Details

Summary

Consecutive revisions are often in the same phase, especially public
revisions. This means that a dictionary keyed by the revision for the
phase transitions is highly redundant. Build a list of (range, (old,
new)) entries instead and aggressively merge ranges with the same
transition. For the test case in issue5691, this reduces memory use by
~20MB.

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

I've a couple of high level question:

  • This reduce the memory usage of 20MB compare to what total usage ?
  • Did you asses the performance impact of this? and if so, what is it?

I haven't measured run-time impact. The sorting should ensure that the lists are normally kept small, but when many updates apply to very fragmented repositories, it could be worse. 20MB is relative to the 600MB peak RSS from the referenced issue.

durin42 requested changes to this revision.Mar 3 2020, 1:30 PM
durin42 added a subscriber: durin42.

Overall seems fine, some structural nits on the code.

mercurial/phases.py
275

It doesn't look like there's a good reason for this function to be nested like this. Can we move it to module-level with an underscore prefix?

295

here too

This revision now requires changes to proceed.Mar 3 2020, 1:30 PM
This revision was not accepted when it landed; it landed in state Needs Review.
This revision was automatically updated to reflect the committed changes.