This is an archive of the discontinued Mercurial Phabricator instance.

parsers: use an attr-derived class for revlog index entries
Needs RevisionPublic

Authored by indygreg on Dec 26 2017, 7:35 PM.

Details

Reviewers
yuja
durin42
baymax
Group Reviewers
hg-reviewers
Summary

Currently, revlog index entries are tuples.

This commit introduces a new type to represent revlog v1 index
entries. The pure Python parser has been converted to use it.

To provide backwards compatibility with existing code (including the
C extension), the type has a getitem to facilitate index
lookups. The intent is to remove this getitem once all consumers
are using named attributes.

Since the pure Python index parser is now using our new type,
test-parseindex2.py had to be updated to deal with the type mismatch.
Once all index parsers are converted and the new type/interface is
ubiquitous, we can restore the simplicity of test-parseindex2.py. We
also had to (temporarily) add "# no-check-code" to test-parseindex2.py
to allow the import of the pure module. This will be removed once the C
extension is ported.

Because consumers are going through an extra Python function call to
getitem to resolve attributes by index, this change regresses
performance on a simple DAG walking revset for the Firefox repository:

$ HGMODULEPOLICY=py hg perfrevset '::tip'
! wall 1.366281 comb 1.360000 user 1.360000 sys 0.000000 (best of 7)
! wall 1.860068 comb 1.860000 user 1.860000 sys 0.000000 (best of 5)

Using PyPy, a major regression is not apparent:

! wall 0.163141 comb 0.160000 user 0.160000 sys 0.000000 (best of 56)
! wall 0.154325 comb 0.160000 user 0.160000 sys 0.000000 (best of 54)

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

indygreg created this revision.Dec 26 2017, 7:35 PM
yuja requested changes to this revision.Jan 4 2018, 10:50 PM
yuja added a subscriber: yuja.
yuja added inline comments.
tests/test-parseindex2.py
182

Perhaps you mean list(map(...)) instead of a list of one element?

This revision now requires changes to proceed.Jan 4 2018, 10:50 PM
indygreg edited the summary of this revision. (Show Details)Jan 6 2018, 3:26 AM
indygreg updated this revision to Diff 4735.
durin42 accepted this revision.Jan 10 2018, 5:54 PM
baymax requested changes to this revision.Jan 24 2020, 12:33 PM

There seems to have been no activities on this Diff for the past 3 Months.

By policy, we are automatically moving it out of the need-review state.

Please, move it back to need-review without hesitation if this diff should still be discussed.

:baymax:need-review-idle:

This revision now requires changes to proceed.Jan 24 2020, 12:33 PM