( )⚙ D8417 nodemap: gate the feature behind a new requirement

This is an archive of the discontinued Mercurial Phabricator instance.

nodemap: gate the feature behind a new requirement
ClosedPublic

Authored by marmoute on Apr 14 2020, 11:48 AM.

Details

Summary

Now that the feature is working smoothly, a question was still open, should we
gate the feature behind a new requirement or just treat it as a cache to be
warmed by those who can and ignored by other.

The advantage of using the cache approach is a transparent upgrade/downgrade
story, making the feature easier to move to. However having out of date cache
can come with a significant performance hit for process who expect an up to
date cache but found none. In this case the file needs to be stored under
.hg/cache.

The "requirement" approach guarantee that the persistent nodemap is up to date.
However, it comes with a less flexible activation story since an explicite
upgrade is required. In this case the file can be stored in .hg/store.

This wiki page is relevant to this questions:

https://www.mercurial-scm.org/wiki/ComputedIndexPlan

So which one should we take? Another element came into plan, the persistent
nodemap use the add method of the transaction, it is used to keep track of a
file content before a transaction in case we need to rollback it back. It turns
out that the transaction.add API does not support file stored anywhere than
.hg/store. Making it support file stored elsewhere is possible, require a
change in on disk transaction format. Updating on disk file requires…
introducing a new requirements.

As a result, we pick the second option "gating the persistent nodemap behind a
new requirements".

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

marmoute created this revision.Apr 14 2020, 11:48 AM
marmoute updated this revision to Diff 21228.Apr 27 2020, 1:12 PM
Alphare accepted this revision.May 7 2020, 9:09 AM
This revision was not accepted when it landed; it landed in state Needs Review.
This revision was automatically updated to reflect the committed changes.