Version 3 slower than Version 2

m4ss1m0g commented 3 years ago

Prerequisites

[X] I have read and understand the CONTRIBUTING guide
[X] I looked for duplicate issues before submitting this one

Description

Version 3 is slower than version 2

Environment

Oh my Posh version: 3.132.0
Theme: slim
Operating System: Windows 10
Shell: pwsh 7.1.3
Terminal: Windows Terminal 1.6.10571.0

Steps to Reproduce

Install oh-my-posh through scoop
Disable Antivirus
Open Windows Terminal
execute pwsh -noprofile to be sure no other module interfer
execute Invoke-Expression (oh-my-posh --init --shell pwsh --config "$(scoop prefix oh-my-posh3)/themes/slim.omp.json")

Expected behavior: Same execution time of version 2

Actual behavior: Slower that version 2

Notes

I disabled antivirus (Kaspersky)
I launched pwsh with -noprofile flag
I attached two small video
I have also tried to install oh-my-posh 3 through Install-Module with no luck

I used Measure-Command to oh-my-posh command, below the result

massimo@JUPITER ❯ Measure-Command { oh-my-posh }

Days              : 0
Hours             : 0
Minutes           : 0
Seconds           : 0
Milliseconds      : 133
Ticks             : 1331365
TotalDays         : 1,54093171296296E-06
TotalHours        : 3,69823611111111E-05
TotalMinutes      : 0,00221894166666667
TotalSeconds      : 0,1331365
TotalMilliseconds : 133,1365

Version 2

https://user-images.githubusercontent.com/4323707/113514502-541c9580-956f-11eb-83db-b6fda5c955ee.mp4

Version 3

https://user-images.githubusercontent.com/4323707/113514530-71e9fa80-956f-11eb-8428-22b1bcc4be37.mp4

JanDeDobbeleer commented 3 years ago

@m4ss1m0g what does oh-my-posh --config "$(scoop prefix oh-my-posh3)/themes/slim.omp.json") --shell uni --debug say?

m4ss1m0g commented 3 years ago

oh-my-posh --config "$(scoop prefix oh-my-posh3)/themes/slim.omp.json") --shell uni --debug

massimo@JUPITER ❯ oh-my-posh --config "$(scoop prefix oh-my-posh3)/themes/slim.omp.json" --shell uni --debug

Here are the timings of segments in your prompt:

ConsoleTitle(true)   -   0 ms - uni :: D:
os(true)             -   0 ms - ▓  
session(true)        -   0 ms -  massimoJUPITER 
root(false)          -   0 ms -
path(true)           -   0 ms -   D:\
git(false)           -  22 ms -
text(true)           -   0 ms - 
text(true)           -   0 ms - 
node(false)          -   0 ms -
python(true)         -  62 ms -    
dotnet(false)        -   1 ms -
time(true)           -   1 ms -  19:36:19  ▓
text(true)           -   0 ms -  ~#@❯
text(true)           -   0 ms -  ❮
executiontime(false) -   0 ms -
exit(true)           -   0 ms -  

I have also tried with a minimum config with no luck

{
    "$schema": "https://raw.githubusercontent.com/JanDeDobbeleer/oh-my-posh/main/themes/schema.json",
    "blocks": [
      {
        "type": "prompt",
        "alignment": "left",
        "segments": [
          {
            "type": "path",
            "style": "plain",
            "foreground": "lightYellow",
            "properties": {
              "prefix": "<darkGray> - </>",
              "style": "full"
            }
          }
        ]
      },
      {
        "type": "newline"
      },
      {
        "type": "prompt",
        "alignment": "left",
        "segments": [
          {
            "type": "text",
            "style": "plain",
            "foreground": "lightRed",
            "properties": {
              "prefix": "",
              "text": "$",
              "postfix": ""
            }
          }
        ]
      }
    ],
    "final_space": true
  }

JanDeDobbeleer commented 3 years ago

@m4ss1m0g this makes no sense, the timings in OMP are so low it's got to be your filesystem that's somehow slowing down the execution of oh-my-posh. Are you running this on a heavily regulated company laptop? Because we've seen Defender acting like a blocking factor in that context.

JanDeDobbeleer commented 3 years ago

@m4ss1m0g in your post you mentioned running OMP is 133ms, that's super fast. Is it fast when your run oh-my-posh directly? Because then something inside the init script is slowing things down maybe.

m4ss1m0g commented 3 years ago

You're right I found the issue on omp.ps1 I have the Az module and two Azure Subscriptions, every time the init tries to set the $env variables, I added the if before and now run smoothly.

I don't know where you use the Az $env variables, the below code fixes the problem and rise another: if I change the Azure Subscription? (I have two...)


    # $env:AZ_SUBSCRIPTION_NAME = $null
    # $env:AZ_SUBSCRIPTION_ID = $null

    if ($null -eq $env:AZ_SUBSCRIPTION_NAME -and $null -eq $env:AZ_SUBSCRIPTION_ID) {

        if (Get-Module -ListAvailable -Name "Az.Accounts") {
            try {
                $subscription = Get-AzContext | Select-Object -ExpandProperty "Subscription" | Select-Object "Name", "Id"
                if ($null -ne $subscription) {
                    $env:AZ_SUBSCRIPTION_NAME = $subscription.Name
                    $env:AZ_SUBSCRIPTION_ID = $subscription.Id
                }
            }
            catch {}
        }

    }

JanDeDobbeleer commented 3 years ago

@TravisTX you might want to have a look at this 😉

tradiff commented 3 years ago

yeah, this is the sort of thing I was worried about. This is why I wanted to let the user opt-in via Set-PoshContext instead of forcing it in the global init script.

@m4ss1m0g Can you help identify which command is slow for you?

Measure-Command {Get-Module -ListAvailable -Name "Az.Accounts"} (31ms for me)
Measure-Command {Get-AzContext} (2ms for me)

I've created PR #614 but I would like to hear m4ss1m0g's results before we move forward with that PR.

I don't know where you use the Az $env variables,

It's used in the az segment.

m4ss1m0g commented 3 years ago

Below my measurements

Measure-Command {Get-Module -ListAvailable -Name "Az.Accounts"} (42ms) Measure-Command {Get-AzContext} (845ms)

I have two Azure subscriptions.

JanDeDobbeleer commented 3 years ago

Holy forkballs that command is slow.

tradiff commented 3 years ago

I have 4 Azure subscriptions, so I don't think that's the issue.

@JanDeDobbeleer I don't see a way to make it faster, while still providing reliable data. We can't cache Get-AzContext because that would ruin the entire point of the feature. The only option I can see at this point is to let people opt out of the feature if it's not usable by them.

I wish there was a way to know at this point in the prompt whether the user is even using the az segment, but I don't see a way of doing that either without PowerShell reading the theme file and parsing the json, which sounds really gross.

So the methods of opting out are:

Opt-Out: Have the user add $env:AZ_ENABLED = $false in their $profile (this is how the PR currently works)
Opt-In: Have the user opt-in by adding $env:AZ_ENABLED = $true to their $profile (I could change the PR a bit to honor this and do nothing if the environment variable isn't set)
Opt-In: Scrap the whole init script, and have the user opt-in via Set-PoshContext setting the environment variables.

@JanDeDobbeleer do you have a preference (or maybe you see other options that I don't see)?

JanDeDobbeleer commented 3 years ago

I see opt-in as a viable path, same for posh-git btw. But via the global promptsettings in that case.

m4ss1m0g commented 3 years ago

I have updated the Az Module and now the Measures are:

Measure-Command {Get-Module -ListAvailable -Name "Az.Accounts"} (47ms) Measure-Command {Get-AzContext} (7ms)

nickwilliams-codynamic commented 3 years ago

I'm seeing a similar problem here too. Seems like Az CLI is indeed the issue for me:

Measure-Command {Get-Module -ListAvailable -Name "Az.Accounts"} 149ms Measure-Command {Get-AzContext} 1091 ms

JanDeDobbeleer commented 3 years ago

@nickwilliams-codynamic also after updating the AZ module?

nickwilliams-codynamic commented 3 years ago

Ah, sorry @JanDeDobbeleer, I misread the comment above. Upgrading to Az.Accounts 2.2.7 works: Measure-Command {Get-AzContext} (2ms)

JanDeDobbeleer commented 3 years ago

@TravisTX in this case, we need to inform people they need to update. Can we add a timeout to the commands and inform people something's wrong and they might need to update/check connectivity?

m4ss1m0g commented 3 years ago

@TravisTX Do you think merging the PR anyway? I think is a good option.

github-actions[bot] commented 5 months ago

This issue has been automatically locked since there has not been any recent activity (i.e. last half year) after it was closed. It helps our maintainers focus on the active issues. If you have found a problem that seems similar, please open a discussion first, complete the body with all the details necessary to reproduce, and mention this issue as reference.

JanDeDobbeleer / oh-my-posh