Skip to content

Commit

Permalink
Performance fix in argparse interface
Browse files Browse the repository at this point in the history
Instead of adding all global parameters to all subparsers (leading to a O(n^2) complexity where n is the number of tasks), just figure out the subparser that's needed and add it to that one

Running nosetests test/parameter_test.py:

Before: Ran 62 tests in 5.248s
After:  Ran 62 tests in 1.381s
  • Loading branch information
Erik Bernhardsson committed Feb 2, 2015
1 parent 99eb230 commit f279df3
Showing 1 changed file with 12 additions and 9 deletions.
21 changes: 12 additions & 9 deletions luigi/interface.py
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,8 @@ def parse_task(self, cmdline_args=None, main_task_cls=None):

add_global_parameters(parser)

subparsers_by_name = {}

if main_task_cls:
add_task_parameters(parser, main_task_cls)

Expand All @@ -258,23 +260,24 @@ def parse_task(self, cmdline_args=None, main_task_cls=None):
subparsers = parser.add_subparsers(dest='command', metavar=orderedtasks)

for name, cls in Register.get_reg().iteritems():
subparser = subparsers.add_parser(name)
if cls == Register.AMBIGUOUS_CLASS:
continue
add_task_parameters(subparser, cls)

# Add global params here as well so that we can support both:
# test.py --global-param xyz Test --n 42
# test.py Test --n 42 --global-param xyz
add_global_parameters(subparser)

args = parser.parse_args(args=cmdline_args)
subparsers_by_name[name] = subparsers.add_parser(name)
add_task_parameters(subparsers_by_name[name], cls)

if main_task_cls:
args = parser.parse_args(args=cmdline_args)
task_cls = main_task_cls
else:
args, unknown = parser.parse_known_args(args=cmdline_args)
task_cls = Register.get_task_cls(args.command)

# Add global params here as well so that we can support both:
# test.py --global-param xyz Test --n 42
# test.py Test --n 42 --global-param xyz
add_global_parameters(subparsers_by_name[args.command])
args = parser.parse_args(args=cmdline_args)

# Notice that this is not side effect free because it might set global params
set_global_parameters(args)
task_params = get_task_parameters(task_cls, args)
Expand Down

0 comments on commit f279df3

Please sign in to comment.