1176642 - Cleanup module importing in mach and elsewhere

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Gregory Szorc [:gps]]

The performance of `mach` has slowly degraded over the years partly due to a lot more modules being imported. While I investigate lazy module importing and possibly caching the command dispatch table, there is some low-hanging fruit to speed things up.

First, we can defer a bunch of imports from module level to function level. 

Second, we can use "from __future__ absolute_import." This has the benefit of reducing the number of stats() Python performs to look for modules since the full package name is unambiguous. It also ensures code is forward compatible with Python 3.

Comment 1

[Gregory Szorc [:gps]]

Attachment: MozReview Request: Bug 1176642 - Remove unused import; r?glandium

Bug 1176642 - Remove unused import; r?glandium

Comment 2

[Gregory Szorc [:gps]]

Attachment: MozReview Request: Bug 1176642 - Use deferred imports for taskcluster mach commands; r?glandium

Bug 1176642 - Use deferred imports for taskcluster mach commands; r?glandium

Tracing imports reveals that this file accounts for a non-trivial amount
of extra imports when mach starts. Deferring imports makes many of them
go away.

Comment 3

[Gregory Szorc [:gps]]

Attachment: MozReview Request: Bug 1176642 - Use absolute_import in mach_commands.py files; r?glandium

Bug 1176642 - Use absolute_import in mach_commands.py files; r?glandium

This removes ambiguity as to which modules are being imported, making
import slightly faster as Python doesn't need to test so many
directories for file presence.

All files should already be using absolute imports because mach command
modules aren't imported to the package they belong to: they instead
belong to the "mach" package. So relative imports shouldn't have been
used.

As part of touching these files, my static analysis commit hook detected
some unused imports. They have been removed.

Comment 4

[Gregory Szorc [:gps]]

Attachment: MozReview Request: Bug 1176642 - Defer import Eclipse backend modules; r?glandium

Bug 1176642 - Defer import Eclipse backend modules; r?glandium

This import brought in significant parts of the mozbuild package. Moving
it to a deferred import reduces the total number of Python modules
imported during mach dispatch by 43.

Comment 5

[Gregory Szorc [:gps]]

Attachment: MozReview Request: Bug 1176642 - Defer import of autotry and pprint; r?chmanchester

Bug 1176642 - Defer import of autotry and pprint; r?chmanchester

This was the only mach_commands.py file importing these modules. Defer
import so mach doesn't work so hard during startup.

Comment 6

[Gregory Szorc [:gps]]

Attachment: MozReview Request: Bug 1176642 - Defer import of glob; r?glandium

Bug 1176642 - Defer import of glob; r?glandium

This was the only import of glob from all mach_commands.py files. Kill
it.

With this commit, there are no modules imported by a single
mach_commands.py outside of testing/web-platform/mach_commands.py.

Comment 7

[Gregory Szorc [:gps]]

Attachment: MozReview Request: Bug 1176642 - Defer import of urllib2; r?chmanchester

Bug 1176642 - Defer import of urllib2; r?chmanchester

This prevents 8 module imports from occurring at mach startup time.

As part of this, I discovered a redundant import of "sys" and eliminated
it.

Comment 8

[Gregory Szorc [:gps]]

Attachment: MozReview Request: Bug 1176642 - Use absolute_import in mozinfo; r?chmanchester

Bug 1176642 - Use absolute_import in mozinfo; r?chmanchester

absolute_import was introduced in Python 2.6. There should be no
backwards compatibility concern here.

Comment 9

[Gregory Szorc [:gps]]

Attachment: MozReview Request: Bug 1176642 - Use absolute_import in mozfile; r?chmanchester

Bug 1176642 - Use absolute_import in mozfile; r?chmanchester

Comment 10

[Gregory Szorc [:gps]]

Attachment: MozReview Request: Bug 1176642 - Use absolute_import, remove unused imports from mozprocess; r?chmanchester

Bug 1176642 - Use absolute_import, remove unused imports from mozprocess; r?chmanchester

Comment 11

[Gregory Szorc [:gps]]

Attachment: MozReview Request: Bug 1176642 - Use absolute_import in mozpack; r?glandium

Bug 1176642 - Use absolute_import in mozpack; r?glandium

Comment 12

[Gregory Szorc [:gps]]

Attachment: MozReview Request: Bug 1176642 - Use absolute_import, remove unused imports in mozbuild; r?glandium

Bug 1176642 - Use absolute_import, remove unused imports in mozbuild; r?glandium

Comment 13

[Gregory Szorc [:gps]]

https://reviewboard.mozilla.org/r/11809/#review10265

On my MBP, this series appears to reduce the wall time of `mach help` by an average of 80ms. ~770s -> ~690ms. Although timings are kinda all over the map. For comparison, `mach help` with no mach_commands.py files takes ~130ms. We have a long way to go.

Comment 14

[Gregory Szorc [:gps]]

Just for kicks, I hooked up Mercurial's lazy module importer and it didn't have much impact on mach startup time.

What does have an impact is removing all the mach_commands.py files under testing/. That yields 310ms. Significantly better, but not great. I think we may need to cache mapping of command to mach_commands.py file to avoid loading all these files.

Comment 15

[Mike Hommey [:glandium]]

Comment on attachment 8625218 [details]
MozReview Request: Bug 1176642 - Remove unused import; r?glandium

https://reviewboard.mozilla.org/r/11811/#review10355

Ship It!

Comment 16

[Mike Hommey [:glandium]]

Comment on attachment 8625219 [details]
MozReview Request: Bug 1176642 - Use deferred imports for taskcluster mach commands; r?glandium

https://reviewboard.mozilla.org/r/11813/#review10357

Ship It!

::: testing/taskcluster/mach_commands.py
(Diff revision 1)
> -import datetime

might as well move this in the "remove unused import" patch.

Comment 17

[Mike Hommey [:glandium]]

Comment on attachment 8625220 [details]
MozReview Request: Bug 1176642 - Use absolute_import in mach_commands.py files; r?glandium

https://reviewboard.mozilla.org/r/11815/#review10359

Ship It!

::: addon-sdk/mach_commands.py
(Diff revision 1)
> -import sys

might as well remove them in the "remove unused import" patch.

Comment 18

[Mike Hommey [:glandium]]

Comment on attachment 8625221 [details]
MozReview Request: Bug 1176642 - Defer import Eclipse backend modules; r?glandium

https://reviewboard.mozilla.org/r/11817/#review10361

Ship It!

Comment 19

[Mike Hommey [:glandium]]

Comment on attachment 8625223 [details]
MozReview Request: Bug 1176642 - Defer import of glob; r?glandium

https://reviewboard.mozilla.org/r/11821/#review10363

Ship It!

Comment 20

[Mike Hommey [:glandium]]

Comment on attachment 8625228 [details]
MozReview Request: Bug 1176642 - Use absolute_import in mozpack; r?glandium

https://reviewboard.mozilla.org/r/11831/#review10365

Ship It!

Comment 21

[Mike Hommey [:glandium]]

Comment on attachment 8625229 [details]
MozReview Request: Bug 1176642 - Use absolute_import, remove unused imports in mozbuild; r?glandium

https://reviewboard.mozilla.org/r/11833/#review10367

Ship It!

::: python/mozbuild/mozbuild/dotproperties.py
(Diff revision 1)
> -import os

Same comment as other patches, might as well move this to the "remove unused import patch".

Comment 22

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8625222 [details]
MozReview Request: Bug 1176642 - Defer import of autotry and pprint; r?chmanchester

https://reviewboard.mozilla.org/r/11819/#review10375

Ship It!

Comment 23

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8625224 [details]
MozReview Request: Bug 1176642 - Defer import of urllib2; r?chmanchester

https://reviewboard.mozilla.org/r/11823/#review10377

Ship It!

Comment 24

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8625225 [details]
MozReview Request: Bug 1176642 - Use absolute_import in mozinfo; r?chmanchester

https://reviewboard.mozilla.org/r/11825/#review10379

Ship It!

Comment 25

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8625226 [details]
MozReview Request: Bug 1176642 - Use absolute_import in mozfile; r?chmanchester

https://reviewboard.mozilla.org/r/11827/#review10381

Ship It!

Comment 26

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8625227 [details]
MozReview Request: Bug 1176642 - Use absolute_import, remove unused imports from mozprocess; r?chmanchester

https://reviewboard.mozilla.org/r/11829/#review10383

Ship It!

Comment 27

[Gregory Szorc [:gps]]

url:        https://hg.mozilla.org/integration/fx-team/rev/c62732d440bd8cf6e2d5f326fbc01eac9cdfd5eb
changeset:  c62732d440bd8cf6e2d5f326fbc01eac9cdfd5eb
user:       Gregory Szorc <[e-mail]>
date:       Thu Jun 25 12:16:38 2015 -0700
description:
Bug 1176642 - Remove unused imports; r=glandium

url:        https://hg.mozilla.org/integration/fx-team/rev/b6ab3b5ac90c920282b8eb13abb0180a9a66a667
changeset:  b6ab3b5ac90c920282b8eb13abb0180a9a66a667
user:       Gregory Szorc <[e-mail]>
date:       Sun Jun 21 17:09:58 2015 -0700
description:
Bug 1176642 - Use deferred imports for taskcluster mach commands; r=glandium

Tracing imports reveals that this file accounts for a non-trivial amount
of extra imports when mach starts. Deferring imports makes many of them
go away.

url:        https://hg.mozilla.org/integration/fx-team/rev/c8cedeb555cd8b3b084a1c3dad3a5e450e52f6e1
changeset:  c8cedeb555cd8b3b084a1c3dad3a5e450e52f6e1
user:       Gregory Szorc <[e-mail]>
date:       Sun Jun 21 17:39:09 2015 -0700
description:
Bug 1176642 - Use absolute_import in mach_commands.py files; r=glandium

This removes ambiguity as to which modules are being imported, making
import slightly faster as Python doesn't need to test so many
directories for file presence.

All files should already be using absolute imports because mach command
modules aren't imported to the package they belong to: they instead
belong to the "mach" package. So relative imports shouldn't have been
used.

url:        https://hg.mozilla.org/integration/fx-team/rev/c2e532dc8fd097c948968470ca1bb3e6ae1d2c31
changeset:  c2e532dc8fd097c948968470ca1bb3e6ae1d2c31
user:       Gregory Szorc <[e-mail]>
date:       Thu Jun 25 12:11:12 2015 -0700
description:
Bug 1176642 - Defer import Eclipse backend modules; r=glandium

This import brought in significant parts of the mozbuild package. Moving
it to a deferred import reduces the total number of Python modules
imported during mach dispatch by 43.

url:        https://hg.mozilla.org/integration/fx-team/rev/b1d8d8d0589e7971d734884e04dcd3ca1b6ebbf2
changeset:  b1d8d8d0589e7971d734884e04dcd3ca1b6ebbf2
user:       Gregory Szorc <[e-mail]>
date:       Thu Jun 25 12:11:34 2015 -0700
description:
Bug 1176642 - Defer import of autotry and pprint; r=chmanchester

This was the only mach_commands.py file importing these modules. Defer
import so mach doesn't work so hard during startup.

url:        https://hg.mozilla.org/integration/fx-team/rev/70512d77f24bd38e838828d773e63279665d8418
changeset:  70512d77f24bd38e838828d773e63279665d8418
user:       Gregory Szorc <[e-mail]>
date:       Thu Jun 25 12:11:55 2015 -0700
description:
Bug 1176642 - Defer import of glob; r=glandium

This was the only import of glob from all mach_commands.py files. Kill
it.

With this commit, there are no modules imported by a single
mach_commands.py outside of testing/web-platform/mach_commands.py.

url:        https://hg.mozilla.org/integration/fx-team/rev/4edfcd2f0a9c07173de644e7fb71062128b3452e
changeset:  4edfcd2f0a9c07173de644e7fb71062128b3452e
user:       Gregory Szorc <[e-mail]>
date:       Thu Jun 25 12:12:15 2015 -0700
description:
Bug 1176642 - Defer import of urllib2; r=chmanchester

This prevents 8 module imports from occurring at mach startup time.

As part of this, I discovered a redundant import of "sys" and eliminated
it.

url:        https://hg.mozilla.org/integration/fx-team/rev/9d5ca5c11b73348f512743e7dccf45a474ef2681
changeset:  9d5ca5c11b73348f512743e7dccf45a474ef2681
user:       Gregory Szorc <[e-mail]>
date:       Thu Jun 25 12:12:39 2015 -0700
description:
Bug 1176642 - Use absolute_import in mozinfo; r=chmanchester

absolute_import was introduced in Python 2.6. There should be no
backwards compatibility concern here.

url:        https://hg.mozilla.org/integration/fx-team/rev/99b9307a60f1628be6ec5cabd2933366d2925452
changeset:  99b9307a60f1628be6ec5cabd2933366d2925452
user:       Gregory Szorc <[e-mail]>
date:       Thu Jun 25 12:13:08 2015 -0700
description:
Bug 1176642 - Use absolute_import in mozfile; r=chmanchester

url:        https://hg.mozilla.org/integration/fx-team/rev/101600812e66deb86c9624438ccd0e32fb6ea4fa
changeset:  101600812e66deb86c9624438ccd0e32fb6ea4fa
user:       Gregory Szorc <[e-mail]>
date:       Thu Jun 25 12:13:27 2015 -0700
description:
Bug 1176642 - Use absolute_import, remove unused imports from mozprocess; r=chmanchester

url:        https://hg.mozilla.org/integration/fx-team/rev/f06c02f9ffbebf30eca846cb6b3a9cbd73e1db0b
changeset:  f06c02f9ffbebf30eca846cb6b3a9cbd73e1db0b
user:       Gregory Szorc <[e-mail]>
date:       Thu Jun 25 12:13:55 2015 -0700
description:
Bug 1176642 - Use absolute_import in mozpack; r=glandium

url:        https://hg.mozilla.org/integration/fx-team/rev/bf34d16b6ab2e48ddae93dc745a72cbddab35fa9
changeset:  bf34d16b6ab2e48ddae93dc745a72cbddab35fa9
user:       Gregory Szorc <[e-mail]>
date:       Sun Jun 21 18:37:18 2015 -0700
description:
Bug 1176642 - Use absolute_import in mozbuild; r=glandium

Comment 28

[Gregory Szorc [:gps]]

url:        https://hg.mozilla.org/integration/fx-team/rev/4ab5f2bbb9941fc155e2dd7515f59d866d69a206
changeset:  4ab5f2bbb9941fc155e2dd7515f59d866d69a206
user:       Gregory Szorc <[e-mail]>
date:       Thu Jun 25 12:55:48 2015 -0700
description:
Bug 1176642 - Use relative imports in mozprocess; r=smacleod

This regressed in 101600812e66.

Comment 29

[Gregory Szorc [:gps]]

url:        https://hg.mozilla.org/integration/fx-team/rev/1a7b9cd270eb098df4b6b19abe4d12fc9f2480cf
changeset:  1a7b9cd270eb098df4b6b19abe4d12fc9f2480cf
user:       Gregory Szorc <[e-mail]>
date:       Thu Jun 25 13:06:25 2015 -0700
description:
Bug 1176642 - Fix typo in import name; r=smacleod

This regressed in 101600812e66.

Comment 30

[Gregory Szorc [:gps]]

url:        https://hg.mozilla.org/integration/fx-team/rev/b57706580b6ad0be4217d93874ca4839b377ba12
changeset:  b57706580b6ad0be4217d93874ca4839b377ba12
user:       Gregory Szorc <[e-mail]>
date:       Thu Jun 25 16:11:22 2015 -0700
description:
Bug 1176642 - Import proper mozinfo package; r=me

bf34d16b6ab2 added absolute_import to this file. When changed, "import
mozinfo" stopped picking up mozbuild.mozinfo and started importing
mozinfo instead.

Use relative imports to force mozbuild.mozinfo to be picked up.

Comment 31

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/c62732d440bd
https://hg.mozilla.org/mozilla-central/rev/b6ab3b5ac90c
https://hg.mozilla.org/mozilla-central/rev/c8cedeb555cd
https://hg.mozilla.org/mozilla-central/rev/c2e532dc8fd0
https://hg.mozilla.org/mozilla-central/rev/b1d8d8d0589e
https://hg.mozilla.org/mozilla-central/rev/70512d77f24b
https://hg.mozilla.org/mozilla-central/rev/4edfcd2f0a9c
https://hg.mozilla.org/mozilla-central/rev/9d5ca5c11b73
https://hg.mozilla.org/mozilla-central/rev/99b9307a60f1
https://hg.mozilla.org/mozilla-central/rev/101600812e66
https://hg.mozilla.org/mozilla-central/rev/f06c02f9ffbe
https://hg.mozilla.org/mozilla-central/rev/bf34d16b6ab2
https://hg.mozilla.org/mozilla-central/rev/4ab5f2bbb994
https://hg.mozilla.org/mozilla-central/rev/1a7b9cd270eb
https://hg.mozilla.org/mozilla-central/rev/b57706580b6a