This is an archive of the discontinued Mercurial Phabricator instance.

demandimport: disable if chg is being used
ClosedPublic

Authored by quark on Aug 11 2017, 2:26 PM.

Details

Summary

In chg's case, making modules lazily loaded could actually slow down things
since chg pre-imports them. Therefore disable demandimport if chg is being
used.

This is not done by setting HGDEMANDIMPORT chg client-side because that
has side-effects on child processes (hooks, etc).

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

quark created this revision.Aug 11 2017, 2:26 PM
yuja added a subscriber: yuja.Aug 15 2017, 3:02 AM

Perhaps the chg executable can simply set HGDEMANDIMPORT=disable
if it isn't specified by user.

quark added a comment.EditedAug 15 2017, 11:15 AM

I tried that approach. If we only set HGDEMANDIMPORT for the forked process to execute hg server, it will be an infinite loop - environment config hash will mismatch. Setting HGDEMANDIMPORT for both the forked and non-forked chg client processes seems less cleaner.

sid0 added a subscriber: sid0.Aug 15 2017, 2:05 PM

I tried that approach. If we only set HGDEMANDIMPORT for the forked process to execute hg server, it will be an infinite loop - environment config hash will mismatch. Setting HGDEMANDIMPORT for both the forked and non-forked chg client processes seems less cleaner.

This should be a comment in this patch :)

phillco accepted this revision.Aug 15 2017, 2:39 PM
phillco added a subscriber: phillco.

lgtm, pending sid's comment.

yuja added a comment.Aug 15 2017, 10:52 PM

Setting HGDEMANDIMPORT for both the forked and non-forked chg client processes seems less cleaner.

That seems less bad since the hg script doesn't have to know the chg stuff.

That seems less bad since the hg script doesn't have to know the chg stuff.

Having HGDEMANDIMPORT set may affect hg ran by child process (like hooks).

How about moving the demandimport stuff to dispatch.py? I think eventually dispatch.py will have some special handling about chg.

quark edited the summary of this revision. (Show Details)Aug 16 2017, 1:45 PM
quark retitled this revision from demandimport: disable by default if chg is being used to demandimport: disable if chg is being used.
quark updated this revision to Diff 1003.
quark planned changes to this revision.Aug 16 2017, 2:55 PM
quark updated this revision to Diff 1011.Aug 16 2017, 3:13 PM
yuja added a comment.Aug 17 2017, 9:23 AM

Can you update the comment in chgserver.py in new series? It says
"CHGINTERNALMARK is temporarily set by chg client to detect if chg will start another chg."

How about using sys.argv[0] to test if chg is being used?

Instead of running hg serve --cmdserver chgunix ..., chg client executes chgserve .... And the hg script could notice that by checking sys.argv[0] and does some special handling (expand it to hg serve --cmdserver chgunix for now, but we might want to use a different entry point in the future).

yuja added a comment.Aug 18 2017, 9:11 AM

Instead of running hg serve --cmdserver chgunix ..., chg client executes chgserve .... And the hg script could notice that by checking sys.argv[0] and does some special handling

At this point, I don't think it's simpler than using CHGINTERNALMARK or HGDEMANDIMPORT.

We might need another entry point for chg in future for some reason, but that is the last option IMHO.

quark updated this revision to Diff 1076.Aug 18 2017, 1:58 PM
quark updated this revision to Diff 1078.Aug 18 2017, 2:12 PM
yuja accepted this revision.Aug 19 2017, 2:46 AM

Queued this, thanks.

This revision is now accepted and ready to land.Aug 19 2017, 2:46 AM
This revision was automatically updated to reflect the committed changes.