[merge] use lazy imports
Martin Pool
mbp at canonical.com
Tue Oct 10 08:05:36 BST 2006
On 6 Oct 2006, John Arbash Meinel <john at arbash-meinel.com> wrote:
> The attached patch starts making use of the lazy_import functionality
> that got merged into bzr-0.11.
>
> This requires a bit of changes to the code base, since it starts
> changing things to import modules, rather than importing members of
> modules (which is generally recommended anyway).
>
> So while I haven't change the whole code base, I would like to merge the
> little bit that I have already completed, before it gets to be too large
> of a change. (It already is getting borderline).
>
> There is still a few places that can be tweaked, like I need to evaluate
> ElementTree and ConfigObj, which are our 2 largest startup times right
> now (approx 30ms each). I have managed to avoid ElementTree, but
> ConfigObj is needed if we want command aliases.
>
>
> This is the specific impact for the benchmarks:
> bench_commit.CommitBenchmark.test_cmd_commit_subprocess OK 383ms/ 509ms
> bench_log.LogBenchmark.test_cmd_log_subprocess OK 372ms/ 391ms
> bench_rocks.RocksBenchmark.test_rocks OK 283ms/ 285ms
>
> bench_commit.CommitBenchmark.test_cmd_commit_subprocess OK 350ms/ 478ms
> bench_log.LogBenchmark.test_cmd_log_subprocess OK 322ms/ 340ms
> bench_rocks.RocksBenchmark.test_rocks OK 157ms/ 159ms
>
> So it has a small effect on commit (30ms == 10%), a slightly large one
> on log (50ms == 15%), and a rather huge one on 'rocks' (120ms == 50%).
>
> Also the test suite isn't testing with '--no-plugins' so these times are
> influenced by what plugins are doing. (And at present, most plugins
> aren't very lazy about what they import).
I'd like to change the benchmarks to pass --no-plugins to make things
more reproducible.
> There also is something to warn about, which is that the lazy objects
> are namespace replacers, and not proxy objects. So it isn't allowed to
> pass them to other variables, and this includes importing them into
> another namespace.
>
> So there are a few places in the code which were doing:
>
> from bzrlib.osutils import rmtree
>
> But now 'rmtree' is lazily imported from 'shutil'. In general, I think
> we should avoid directly importing members of modules, and work harder
> to just import the modules.
>
> If someone is interested, the lazy import code could also be updated so
> that if the module already exists in sys.modules, it just returns it,
> rather than creating a replacer object.
OK, these things ought to go into the HACKING file.
> I apologize in advance for this being a 3K line patch, and I'll try hard
> to submit smaller changes in the future.
It's OK, as you say it doesn't need to be read in detail.
So is _mod_foo meant to be a new general pattern for importing
submodules which clash with commonly used names? It seems like a
reasonable tradeoff to me, but again if it's to be a standard could be
documented.
One downside with this is that tools like pyflakes don't understand it,
so will not be able to give useful errors on missing imports. On the
whole that's something I'm prepared to give up: they don't catch many
things that aren't caught by our tests, and they do generate a lot of
spurious warnings.
The main thing I found was that they'd report on whether errors were
correctly imported, because it can be hard to test all those cases.
That might be partially alleviated by always specifying 'errors.Foo'
(though that still won't catch typos.)
pyflakes also tells of things which are defined or imported but
not used; imports are not such a big deal but it occasionally catches
more important things.
--
Martin
More information about the bazaar
mailing list