cros: speed up basic commandline parsing
Our current design imports all subcommand modules before doing anything.
As we add more modules, and the modules themselves get larger, this does
not scale well at all. As it stands now, doing nothing or just passing
in --help takes over 500msec.
The reason for this design was to enable flexibility: the name of the
module implementing a subcommand did not have to match the subcommand,
and we could put more than one subcommand in a module using decorators.
In practice, we've never used this functionality -- we've always had a
one-to-one matching between the module name & the subcommand it held.
Lets make this a hard requirement to regain performance. We no longer
have to import every single subcommand every time (and all their unique
but unused modules), we only have to import & execute the specific one
the user has requested.
This cuts the no-op time by half for `cros`, and shaves 300msec off all
`cros` command users run.
BUG=chromium:868820, chromium:1022507
TEST=`cros --help` is faster
TEST=`cros lint ...` still works
Change-Id: Icc1cc9493c12dae94f5459a027fca37cecfa0aef
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/chromite/+/1903449
Reviewed-by: Achuith Bhandarkar <achuith@chromium.org>
Reviewed-by: Chris McDonald <cjmcdonald@chromium.org>
Tested-by: Mike Frysinger <vapier@chromium.org>
diff --git a/cli/command_unittest.py b/cli/command_unittest.py
index 1c1abf1..10f8774 100644
--- a/cli/command_unittest.py
+++ b/cli/command_unittest.py
@@ -8,10 +8,8 @@
from __future__ import print_function
import argparse
-import glob
import os
-from chromite.lib import constants
from chromite.cli import command
from chromite.lib import commandline
from chromite.lib import cros_import
@@ -97,30 +95,27 @@
class CommandTest(cros_test_lib.MockTestCase):
"""This test class tests that we can load modules correctly."""
- # pylint: disable=protected-access
-
def testFindModules(self):
"""Tests that we can return modules correctly when mocking out glob."""
fake_command_file = 'cros_command_test.py'
filtered_file = 'cros_command_unittest.py'
- mydir = 'mydir'
- self.PatchObject(glob, 'glob',
+ self.PatchObject(os, 'listdir',
return_value=[fake_command_file, filtered_file])
- self.assertEqual(command._FindModules(mydir), [fake_command_file])
+ self.assertEqual(command.ListCommands(), {'command-test'})
def testLoadCommands(self):
"""Tests import commands correctly."""
fake_module = 'cros_command_test'
- fake_command_file = os.path.join(constants.CHROMITE_DIR, 'foo', fake_module)
- module_path = ['chromite', 'foo', fake_module]
+ module_path = ['chromite', 'cli', 'cros', fake_module]
- self.PatchObject(command, '_FindModules', return_value=[fake_command_file])
# The code doesn't use the return value, so stub it out lazy-like.
load_mock = self.PatchObject(cros_import, 'ImportModule', return_value=None)
- command._ImportCommands()
+ command._commands['command-test'] = 123
+ self.assertEqual(command.ImportCommand('command-test'), 123)
+ command._commands.pop('command-test')
load_mock.assert_called_with(module_path)