Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"Lazy" loading of commands? #320

Closed
wtyneb opened this issue Dec 18, 2019 · 7 comments · Fixed by #332
Closed

"Lazy" loading of commands? #320

wtyneb opened this issue Dec 18, 2019 · 7 comments · Fixed by #332
Assignees
Milestone

Comments

@wtyneb
Copy link

wtyneb commented Dec 18, 2019

Is there a recommended way of loading things like sub-commands lazily?
My thinking is that if I'm frequently invoking a CLI tool with many commands, each with many subcommands, it would be preferable to only "load" the sub-commands of the parent command?

I suppose it could be done with some custom parsing or otherwise not linking the commands and sub-commands together with app.Command<T>(...), but I think that kind-of defeats the purpose of the command/sub-command structure.

Is there another way that I'm not seeing?

I've spent a bit of time with this library, and so far I really like the builder syntax and the lack of excessive "magic"! Makes it easier to approach. Thank you!

@natemcmaster
Copy link
Owner

Currently, there is no support for lazy-loading subcommands. Why do you ask? Have you noticed performance issues with large apps?

@wtyneb
Copy link
Author

wtyneb commented Jan 6, 2020

Hi Nate. I finally got around to organizing my response, I wanted to provide an example of what I'm trying to achieve before going into more detail.

Here's a single file ConsoleApp that I've been using as a playground for learning about your library (see earlier revisions), it is a WIP, but I think it demonstrates what I'm getting at.
CustomSubcommand.cs gist
The earlier revisions contain more complicated examples that I was using to test my first idea of an ICliCommand interface, and for testing IOC/DI.

Bottom line, I am not seeing a negative performance impact yet, and I do would not expect to see one for "simple" subcommands (but I haven't tested that). Instead my concern is about how/when more complicated subcommands of type CommandLineApplication<TModel> have their TModel initialized.

In my case, I'm looking to build a CLI that interacts with my team's systems, and therefore would depend on packages, clients, and service classes that we have already written. Not all of these dependencies are lightweight :), so as this CLI tool grows in scope, it would be nice to defer instantiating these dependencies until they are needed.

Of course, it might be possible to defer resolving/constructing these dependencies ourselves until a particular subcommand's CommandLineApplication is invoked. However, this seems redundant when CommandLineApplication is already wrapping and providing a similar layer of indirection.

I've attempted to do this in the CustomSubcommand.cs gist, with the ICommandWrapper<TModel> interface and the LazySubcommand<TWrapper, TModel, TBase> extension method. I'm not in love with this solution, but it did get the job done. In my first attempt, before I wrote ICommandWrapper<TModel>, there was another custom interface ICliCommand. The idea of ICliCommand is to have all commands/subcommands implement a standard set of functionality, which allows them to be registered without the registrar (the root CommandLineApplication knowing anything about them) via the Subcommand extension method I created. However, in the lazy scenario with ICommandWrapper<TModel this is now divided, with the wrapper performing the "glue setup", and ICliCommand2 (forgive the name!) requiring only two methods: Handle for doing real work, and InjectOptions for getting data from the wrapper.

For background:

This is how I understood the CommandLineApplication:
CommandLineApplication<TModel> uses an instance of TModel to do all the "heavy lifting" while the wrapping CommandLineApplication<TModel> provides the "glue", the general idea being that the OnExecute delegate would call methods defined by TModel. The construction of TModel is done eagerly by DI when the app is invoked, because the tree of CommandLineApplication<TModel> need be created for command parsing/options. But, I think it could easily be deferred.

My reasoning is that by using CommandLineApplication<TModel>'s apparent intent is separation of concerns. The framework with its CommandLineApplication is good at organizing the commands, aliases, help, etc., and managing arguments/options and the hierarchy of commands. So, CommandLineApplication<TModel> could crate TModel lazilly since the library is unconcerned about TModel.

It seems then the preferred (ideal?) behaviour is and only construct the chain of TModels needed for a chain of commands/subcommands.

Example:

Take a CLI too called my_cli with command and subcommand foo and bar, invoked as my_cli foo bar. This is implemented a chain of CommandLineApplication<TModel> : CommandLineApplication<MyCliModel> , CommandLineApplication<FooModel>, CommandLineApplication<BarModel>. If we were able to defer construction of TModel, then the library would only need to construct instances of BaseModel , FooModel, and BarModel. Even though, in this scenario, the base app my_cli with a root of CommandLineApplication<BaseModel> supports several command "trees":

base
  foo
     bar
     baz
  zar
    doz
  bleep
    bloop
  fizz
    buzz

The idea is, at runtime the library would never need to construct a FizzModel unless that branch of the command tree was invoked.

Possible ways forward:

I'm relatively new with this framework, but my first thought is some variant of CommandLineApplication<TModel>.Command that allows the user to pass a constructor delegate for TModel, and then have a variant of OnExecute where the delegate accepts an instance of TModel, which I think would mean CommandLineApplication<TModel>.Model would now be allowed to return null, OR a new variant of CommandLineApplication where .Model returns a promise/deferred type.

I'm happy to discuss this further, take feedback, and help with implementing a solution to this if it's something you/we think is worthwhile.

@natemcmaster
Copy link
Owner

Thanks for the details, @wtyneb. I got a little lost reading the sample code, but the points you were making about deferring TModel let me to investigate closer the actual behavior. Here is a definite mistake:


This can trigger initializations during a .Dispose() call, which is just wrong.
The other possible mistake is here:
(this as IModelAccessor).GetModel();
This can create unnecessary initializations, depending on which conventions you applied. This was added in d9cb6d3 by myself for unclear reasons. Removing it doesn't seem to be a problem.

I've corrected both of these in #332. Can you take a look at let me know if this is addressing the main problem you've described?

@wtyneb
Copy link
Author

wtyneb commented Jan 7, 2020

Those changes look good, it definitely makes sense to avoid activating Lazy<T>s when they're not actually needed!
Unfortunately though they don't really help with what I was trying to do; but there were some mistakes in my approach.

It seems like I'm trying to get you library to do something that it wasn't designed to do. While TModel can be lazy, and can be delegated the actual command execution, it was not intended as a container for defining command/subcommand structure. I should've realized this earlier, but naturally TModel can't be lazy and also provide structure configuration originating from instance members.

My ideal would to have the root CommandLineApplication<TModel> rely on the subcommands to tell it (the root) how to "load" them, while also having the subcommands know very little about the root application. The subcommands would simply have to implement an interface that provides access to configuration details (name, aliases, arguments, options, description, help) and an interface (possibly the same one) that provides access to command execution implementation (aka "heavy lifting")

Basically, I want the CLI structure to be more dynamic and driven by the subcommands, instead of having the root application contain everything.

Earlier on I did attempt to initialize child CommandLineApplications only when their parent is invoked, but that didn't work. Maybe this is an alternative approach, but I was just doing it incorrectly?

I want to continue using this framework, because I really like the way the framework handles option/argument parsing and allowing nesting of subcommands via a relation of objects.

Maybe this discussion needs its own thread, since I think this is about more than just "lazy" loading of subcommands.

@natemcmaster
Copy link
Owner

naturally TModel can't be lazy and also provide structure configuration originating from instance members.

Correct. Using attributes to defining a command line API means it is completely declarative. The builder API is a little more flexible, but it still does not allow you to change the definition of the command line app while parsing is happening.

Splitting subcommands into separate classes which are decoupled from the root application should be possible. The subcommands samples doesn't do a good job off this. Maybe I should right more samples. In the meantime, here are two real-world apps with a structure that might be sort of what you're looking for.

@natemcmaster
Copy link
Owner

By the way, I'll leave this open until I release the 2.5.1 patch, but if you want to start a new thread for more discussion about structuring apps with many subcommands, feel free to do so 😄

@natemcmaster
Copy link
Owner

2.5.1 is released.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants