Page MenuHomePhabricator

revlog-native: introduced ABI version in capsule
ClosedPublic

Authored by gracinet on Tue, Jan 14, 6:05 AM.

Details

Summary

Concerns that an inconsistency could arise between the actual contents
of the capsule in revlog.c and the Rust consumer have been raised after
the switch to the array of data and function pointers in f384d68d8ea8.

It has been suggested that the version from parsers.c could be use for
this. In this change, we introduce instead a separate ABI version number,
which should have the following advantages:

  • no need to change the consuming Rust code for changes that have nothing to do with the contents of the capsule
  • the version number in parsers.c is not explicitely flagged as ABI. It's not obvious to me whether an ABI change that would be invisible to Python would warrant an increment

The drawback is that developers now have to consider two version numbers.

We expect the added cost of the check to be negligible because it occurs
at instantiation of CIndex only, which in turn is tied to instantiation
of Python objects such as LazyAncestors and MixedIndex. Frequent calls
to Cindex::new should also probably hit the CPU branch predictor.

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

gracinet created this revision.Tue, Jan 14, 6:05 AM
gracinet added a subscriber: yuja.Tue, Jan 14, 6:10 AM

@yuja I believe this could address the concerns you expressed on D7543

If you prefer to use the version from parsers.c, that's your call

This revision was not accepted when it landed; it landed in state Needs Review.
This revision was automatically updated to reflect the committed changes.