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

CLI - reduce the startup time #16320

Merged
merged 1 commit into from
Apr 8, 2021
Merged

CLI - reduce the startup time #16320

merged 1 commit into from
Apr 8, 2021

Conversation

mkouba
Copy link
Contributor

@mkouba mkouba commented Apr 7, 2021

  • do not set CommandLine.setUsageHelpLongOptionsMaxWidth(); a side
    effect of this option is that picocli attempts to detect the terminal
    size which is quite expensive
  • do not detect unused removed beans false posititives at runtime

@mkouba mkouba added the area/cli Related to quarkus cli (not maven/gradle/etc.) label Apr 7, 2021
@mkouba mkouba requested a review from maxandersen April 7, 2021 12:32
@quarkus-bot quarkus-bot bot added the area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins label Apr 7, 2021
@mkouba
Copy link
Contributor Author

mkouba commented Apr 7, 2021

FTR this is the qs -h output diff. Actually, the behavior seems to be a bit "unpredictable" - I can't see the diff anymore when running the uber-jar manually.

Before:

Usage: qs [-ehV] [--verbose] [COMMAND]
  -e, --errors    Produce execution error messages.
  -h, --help      Show this help message and exit.
  -V, --version   Print version information and exit.
      --verbose   Verbose mode.
Commands:
  build             Build your quarkus project
  clean             Clean current project
  create            Create a new quarkus project.
  create-jbang      Create a new quarkus jbang project.
  list              List installed (default) or installable extensions.
  platforms         List imported (default) or all available Quarkus platforms.
  add               Add extension(s) to current project.
  remove, rm        Remove an extension from this project.
  dev               Execute project in live coding dev mode
  create-extension  Creates the base of a Quarkus extension in different layout
                      depending of the options and environment.

After:

Usage: qs [-ehV] [--verbose] [COMMAND]
  -e, --errors    Produce execution error messages.
  -h, --help      Show this help message and exit.
  -V, --version   Print version information and exit.
      --verbose   Verbose mode.
Commands:
  build             Build your quarkus project
  clean             Clean current project
  create            Create a new quarkus project.
  create-jbang      Create a new quarkus jbang project.
  list              List installed (default) or installable extensions.
  platforms         List imported (default) or all available Quarkus platforms.
  add               Add extension(s) to current project.
  remove, rm        Remove an extension from this project.
  dev               Execute project in live coding dev mode
  create-extension  Creates the base of a Quarkus extension in different layout depending of the options and environment.

@gsmet
Copy link
Member

gsmet commented Apr 7, 2021

do not detect unused removed beans false posititives at runtime

how is it a problem for the CLI and not for your typical Quarkus applications?

@mkouba
Copy link
Contributor Author

mkouba commented Apr 7, 2021

do not detect unused removed beans false posititives at runtime

how is it a problem for the CLI and not for your typical Quarkus applications?

Well, it's not such a big problem but the generated io.quarkus.arc.ComponentsProvider contains info about every removed bean and we want to reduce the size of the CLI uber-jar (which is currently huge) and attempt to reduce the number of loaded classes.

We could probably initialize the set of removed beans lazily but I'm not sure it's worth it.

@mkouba
Copy link
Contributor Author

mkouba commented Apr 7, 2021

Actually, it seems that all CLI commands use @CommandLine.Command(usageHelpAutoWidth = true). So this PR will only help to reduce the startup time of qs -h :-(.

I've removed @CommandLine.Command(usageHelpAutoWidth = true) from all commands as I see no difference in the output. Of course, this will need more eyes...

@quarkus-bot quarkus-bot bot added the area/jbang Issues related to when using jbang.dev with Quarkus label Apr 7, 2021
- do not set CommandLine.setUsageHelpLongOptionsMaxWidth(); a side
effect of this option is that picocli attempts to detect the terminal
size which is quite expensive
- removed @CommandLine.Command(usageHelpAutoWidth = true) from all
commands
- do not detect unused removed beans false posititives at runtime
Copy link
Member

@maxandersen maxandersen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes sense to me - i dont spot formatting differences either.

LGTM!

@mkouba mkouba added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Apr 8, 2021
@maxandersen
Copy link
Member

@patriot1burke might remember why he put the formatting widths in place - but in any case I think we should do everything we can do avoid relying on the expensive width call.

@mkouba mkouba merged commit 9ec5162 into quarkusio:main Apr 8, 2021
@quarkus-bot quarkus-bot bot added this to the 2.0 - main milestone Apr 8, 2021
@quarkus-bot quarkus-bot bot removed the triage/waiting-for-ci Ready to merge when CI successfully finishes label Apr 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/cli Related to quarkus cli (not maven/gradle/etc.) area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/jbang Issues related to when using jbang.dev with Quarkus
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants