( )⚙ D8414 nodemap: add a new mode option, with an optional "warn" value

This is an archive of the discontinued Mercurial Phabricator instance.

Differential D8414

nodemap: add a new mode option, with an optional "warn" value
ClosedPublic

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

Details

Reviewers
Alphare
Group Reviewers
hg-reviewers
Commits
rHG6493f0a567c2: nodemap: add a new mode option, with an optional "warn" value
Summary

When "warn" is set, user will get notified when the slow code, used for
compatibility is used. This can help people to detect situation were using that
feature will give them a slowdown instead of a speedup.

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
pulkit added a subscriber: pulkit.Apr 14 2020, 2:45 PM
pulkit added inline comments.
mercurial/localrepo.py
941

nit: we can directly assign the value instead of temp variable

mercurial/revlogutils/nodemap.py
148

needs a b'' prefix

pulkit added inline comments.Apr 14 2020, 2:46 PM
mercurial/configitems.py
684

another minor nit: since the config section is experimental, we can skip the exp- prefix.

marmoute added inline comments.Apr 15 2020, 7:19 AM
mercurial/configitems.py
684

This matcht he pattern used above. In addition the exp- prefix make sense here` the feature is (was) in a state were any actual usage is out of question. Using a specific prefix for this prevent people using older version to step into a non-working experimental feature when t hey try to use an experimental feature that works fine in a later version.

mercurial/localrepo.py
941

Then the line is too long an need wrapping. I would rather have the small tmp variable than complex line wrapping here.

mercurial/revlogutils/nodemap.py
148

good catch, thanks

marmoute updated this revision to Diff 21092.Apr 15 2020, 7:59 AM
marmoute updated this revision to Diff 21227.Apr 27 2020, 1:12 PM
Alphare accepted this revision.May 7 2020, 8:58 AM
marmoute added a commit: rHG6493f0a567c2: nodemap: add a new mode option, with an optional "warn" value.May 7 2020, 11:33 AM
This revision was not accepted when it landed; it landed in state Needs Review.
Closed by commit rHG6493f0a567c2: nodemap: add a new mode option, with an optional "warn" value (authored by marmoute). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 21290

mercurial/configitems.py

) )
coreconfigitem( coreconfigitem(
b'experimental', b'exp-persistent-nodemap', default=False, b'experimental', b'exp-persistent-nodemap', default=False,
) )
coreconfigitem( coreconfigitem(
b'experimental', b'exp-persistent-nodemap.mmap', default=True, b'experimental', b'exp-persistent-nodemap.mmap', default=True,
) )
coreconfigitem( coreconfigitem(
b'experimental', b'exp-persistent-nodemap.mode', default=b'compat',
pulkitUnsubmitted
Not Done

another minor nit: since the config section is experimental, we can skip the exp- prefix.

pulkit: another minor nit: since the config section is `experimental`, we can skip the `exp-` prefix.
marmouteAuthorUnsubmitted
Done

This matcht he pattern used above. In addition the exp- prefix make sense here` the feature is (was) in a state were any actual usage is out of question. Using a specific prefix for this prevent people using older version to step into a non-working experimental feature when t hey try to use an experimental feature that works fine in a later version.

marmoute: This matcht he pattern used above. In addition the `exp-` prefix make sense here` the feature…
)
coreconfigitem(
b'experimental', b'server.filesdata.recommended-batch-size', default=50000, b'experimental', b'server.filesdata.recommended-batch-size', default=50000,
) )
coreconfigitem( coreconfigitem(
b'experimental', b'experimental',
b'server.manifestdata.recommended-batch-size', b'server.manifestdata.recommended-batch-size',
default=100000, default=100000,
) )
coreconfigitem( coreconfigitem(

mercurial/localrepo.py

options[b'enableellipsis'] = True options[b'enableellipsis'] = True
if ui.configbool(b'experimental', b'rust.index'): if ui.configbool(b'experimental', b'rust.index'):
options[b'rust.index'] = True options[b'rust.index'] = True
if ui.configbool(b'experimental', b'exp-persistent-nodemap'): if ui.configbool(b'experimental', b'exp-persistent-nodemap'):
options[b'exp-persistent-nodemap'] = True options[b'exp-persistent-nodemap'] = True
if ui.configbool(b'experimental', b'exp-persistent-nodemap.mmap'): if ui.configbool(b'experimental', b'exp-persistent-nodemap.mmap'):
options[b'exp-persistent-nodemap.mmap'] = True options[b'exp-persistent-nodemap.mmap'] = True
epnm = ui.config(b'experimental', b'exp-persistent-nodemap.mode')
options[b'exp-persistent-nodemap.mode'] = epnm
pulkitUnsubmitted
Not Done

nit: we can directly assign the value instead of temp variable

pulkit: nit: we can directly assign the value instead of temp variable
marmouteAuthorUnsubmitted
Done

Then the line is too long an need wrapping. I would rather have the small tmp variable than complex line wrapping here.

marmoute: Then the line is too long an need wrapping. I would rather have the small tmp variable than…
if ui.configbool(b'devel', b'persistent-nodemap'): if ui.configbool(b'devel', b'persistent-nodemap'):
options[b'devel-force-nodemap'] = True options[b'devel-force-nodemap'] = True
return options return options
def makemain(**kwargs): def makemain(**kwargs):
"""Produce a type conforming to ``ilocalrepositorymain``.""" """Produce a type conforming to ``ilocalrepositorymain``."""

mercurial/revlogutils/nodemap.py

# nodemap.py - nodemap related code and utilities # nodemap.py - nodemap related code and utilities
# #
# Copyright 2019 Pierre-Yves David <pierre-yves.david@octobus.net> # Copyright 2019 Pierre-Yves David <pierre-yves.david@octobus.net>
# Copyright 2019 George Racinet <georges.racinet@octobus.net> # Copyright 2019 George Racinet <georges.racinet@octobus.net>
# #
# This software may be used and distributed according to the terms of the # This software may be used and distributed according to the terms of the
# GNU General Public License version 2 or any later version. # GNU General Public License version 2 or any later version.
from __future__ import absolute_import from __future__ import absolute_import
import errno import errno
import os import os
import re import re
import struct import struct
from ..i18n import _
from .. import ( from .. import (
error, error,
node as nodemod, node as nodemod,
util, util,
) )
class NodeMap(dict): class NodeMap(dict):
pass pass
def add(self, *args, **kwargs): def add(self, *args, **kwargs):
pass pass
def addabort(self, *args, **kwargs): def addabort(self, *args, **kwargs):
pass pass
def _report(self, *args):
pass
def update_persistent_nodemap(revlog): def update_persistent_nodemap(revlog):
"""update the persistent nodemap right now """update the persistent nodemap right now
To be used for updating the nodemap on disk outside of a normal transaction To be used for updating the nodemap on disk outside of a normal transaction
setup (eg, `debugupdatecache`). setup (eg, `debugupdatecache`).
""" """
if revlog._inline: if revlog._inline:
if revlog.nodemap_file is None: if revlog.nodemap_file is None:
msg = "calling persist nodemap on a revlog without the feature enableb" msg = "calling persist nodemap on a revlog without the feature enableb"
raise error.ProgrammingError(msg) raise error.ProgrammingError(msg)
can_incremental = util.safehasattr(revlog.index, "nodemap_data_incremental") can_incremental = util.safehasattr(revlog.index, "nodemap_data_incremental")
ondisk_docket = revlog._nodemap_docket ondisk_docket = revlog._nodemap_docket
feed_data = util.safehasattr(revlog.index, "update_nodemap_data") feed_data = util.safehasattr(revlog.index, "update_nodemap_data")
use_mmap = revlog.opener.options.get(b"exp-persistent-nodemap.mmap") use_mmap = revlog.opener.options.get(b"exp-persistent-nodemap.mmap")
mode = revlog.opener.options.get(b"exp-persistent-nodemap.mode")
if not can_incremental:
msg = _(b"persistent nodemap in strict mode without efficient method")
pulkitUnsubmitted
Not Done

needs a b'' prefix

pulkit: needs a `b''` prefix
marmouteAuthorUnsubmitted
Done

good catch, thanks

marmoute: good catch, thanks
if mode == b'warn':
tr._report(b"%s\n" % msg)
data = None data = None
# first attemp an incremental update of the data # first attemp an incremental update of the data
if can_incremental and ondisk_docket is not None: if can_incremental and ondisk_docket is not None:
target_docket = revlog._nodemap_docket.copy() target_docket = revlog._nodemap_docket.copy()
( (
src_docket, src_docket,
data_changed_count, data_changed_count,

tests/test-persistent-nodemap.t

=================================== ===================================
Test the persistent on-disk nodemap Test the persistent on-disk nodemap
=================================== ===================================
$ hg init test-repo $ hg init test-repo
$ cd test-repo $ cd test-repo
$ cat << EOF >> .hg/hgrc $ cat << EOF >> .hg/hgrc
> [experimental] > [experimental]
> exp-persistent-nodemap=yes > exp-persistent-nodemap=yes
> [devel] > [devel]
> persistent-nodemap=yes > persistent-nodemap=yes
> EOF > EOF
$ hg debugbuilddag .+5000 --new-file $ hg debugbuilddag .+5000 --new-file --config "experimental.exp-persistent-nodemap.mode=warn"
persistent nodemap in strict mode without efficient method (no-rust no-pure !)
persistent nodemap in strict mode without efficient method (no-rust no-pure !)
$ hg debugnodemap --metadata $ hg debugnodemap --metadata
uid: ???????????????? (glob) uid: ???????????????? (glob)
tip-rev: 5000 tip-rev: 5000
tip-node: 6b02b8c7b96654c25e86ba69eda198d7e6ad8b3c tip-node: 6b02b8c7b96654c25e86ba69eda198d7e6ad8b3c
data-length: 121088 data-length: 121088
data-unused: 0 data-unused: 0
data-unused: 0.000% data-unused: 0.000%
$ f --size .hg/store/00changelog.n $ f --size .hg/store/00changelog.n