WIP: Add PSConfig.json field to override threshold for slow profile loading msg #17330

rkeithhill · 2022-05-13T06:12:03Z

PR Summary

This PR adds a field to the powershell.config.json file to allow the end user to set the time threshold for showing the "slow profile loading" message. Fixes #16929.

This is WIP because I haven't looked into the test impact yet (and have not written any new tests). Before I do that I'd like to get a nod that the PR would be accepted.

I also expect some debate on what the JSON field should be named. The proposed name is descriptive but perhaps a bit too long - SlowProfileLoadingMessageThreshold.

PR Context

To address and fix issue #16929. See the issue for the complete rationale for this PR.

PR Checklist

iSazonov · 2022-05-13T06:33:47Z

src/Microsoft.PowerShell.ConsoleHost/host/msh/ConsoleHost.cs

+                        int showMessageThresholdMs = 500;
+
+                        int thresholdMs = PowerShellConfig.Instance.GetSlowProfileLoadingMessageThreshold(ConfigScope.AllUsers);
+                        if (thresholdMs > 0)


What if the config contains 0 or -1 value?

The current impl would ignore <= 0 so you'd get the default 500 milliseconds. I suppose I could change that to threshold >= 0 so if the user set it to 0, you'd always see the slow profile loading message. That's a bit odd because then it is no longer about slow profile loading but is instead "always show my profile load time". Then again a threshold of 1 ms is more or less the same.

Part of the problem with this approach to current user setting overriding the global setting is that I'm relying on a default value of 0 to indicate the setting is not set in that config file. I should probably just make the PowerShellConfig property return int? and use that instead to see if the value was set in the corresponding config file. Then 0 could literally mean the threshold is 0 ms and any negative number could mean the threshold is infinite.

TravisEz13 · 2022-05-13T17:11:25Z

PR to fix container failure: #17338

TravisEz13 · 2022-05-13T17:36:22Z

/rebase

pull-request-quantifier · 2022-05-19T01:40:55Z

This PR has 12 quantified lines of changes. In general, a change size of upto 200 lines is ideal for the best PR experience!

Quantification details

Label      : Extra Small
Size       : +10 -2
Percentile : 4.8%

Total files changed: 2

Change summary by file extension:
.cs : +10 -2

Change counts above are quantified counts, based on the PullRequestQuantifier customizations.

Why proper sizing of changes matters

Optimal pull request sizes drive a better predictable PR flow as they strike a
balance between between PR complexity and PR review overhead. PRs within the
optimal size (typical small, or medium sized PRs) mean:

Fast and predictable releases to production:
- Optimal size changes are more likely to be reviewed faster with fewer
  iterations.
- Similarity in low PR complexity drives similar review times.
Review quality is likely higher as complexity is lower:
- Bugs are more likely to be detected.
- Code inconsistencies are more likely to be detetcted.
Knowledge sharing is improved within the participants:
- Small portions can be assimilated better.
Better engineering practices are exercised:
- Solving big problems by dividing them in well contained, smaller problems.
- Exercising separation of concerns within the code changes.

What can I do to optimize my changes

Use the PullRequestQuantifier to quantify your PR accurately
- Create a context profile for your repo using the context generator
- Exclude files that are not necessary to be reviewed or do not increase the review complexity. Example: Autogenerated code, docs, project IDE setting files, binaries, etc. Check out the Excluded section from your prquantifier.yaml context profile.
- Understand your typical change complexity, drive towards the desired complexity by adjusting the label mapping in your prquantifier.yaml context profile.
- Only use the labels that matter to you, see context specification to customize your prquantifier.yaml context profile.
Change your engineering behaviors
- For PRs that fall outside of the desired spectrum, review the details and check if:
  - Your PR could be split in smaller, self-contained PRs instead
  - Your PR only solves one particular issue. (For example, don't refactor and code new features in the same PR).

How to interpret the change counts in git diff output

One line was added: +1 -0
One line was deleted: +0 -1
One line was modified: +1 -1 (git diff doesn't know about modified, it will
interpret that line like one addition plus one deletion)
Change percentiles: Change characteristics (addition, deletion, modification)
of this PR in relation to all other PRs within the repository.

Was this comment helpful? 👍 👌 👎 (Email)
Customize PullRequestQuantifier for this repository.

msftbot · 2022-05-26T02:00:55Z

This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days.
Maintainer, please provide feedback and/or mark it as Waiting on Author

rkeithhill · 2022-06-03T01:08:02Z

@daxian-dbw Any thoughts on whether this PR is something the team would be interested in taking?

daxian-dbw · 2022-06-03T16:57:18Z

The WG-Interactive-Console working group needs to triage the issue. From a quick look of the change, it's not clear to me how this will work with pwsh -settingsfile flag. But you may want to wait on addressing that until after the WG agrees to accept this change.

daxian-dbw · 2022-06-09T18:41:17Z

@rkeithhill Given the comment #16929 (comment), you may want to close this PR.

rkeithhill requested review from daxian-dbw and anmenaga as code owners May 13, 2022 06:12

msftbot bot assigned PaulHigin May 13, 2022

pull-request-quantifier bot added the Extra Small label May 13, 2022

iSazonov reviewed May 13, 2022

View reviewed changes

Add PConfig file option to set threshold for slow profile loading msg

cdf0234

github-actions bot force-pushed the rkeithhill/slow-profile-loading-msg-threshold branch from 6098e5e to cdf0234 Compare May 13, 2022 17:37

Return Nullable<int> from setting in order to distinguish from "not set"

3f0c401

msftbot bot added the Review - Needed The PR is being reviewed label May 26, 2022

msftbot bot removed the Review - Needed The PR is being reviewed label Jun 3, 2022

rkeithhill closed this Jun 9, 2022

Jul	AUG	Sep
	15
2022	2023	2024

WIP: Add PSConfig.json field to override threshold for slow profile loading msg #17330

WIP: Add PSConfig.json field to override threshold for slow profile loading msg #17330

rkeithhill commented May 13, 2022

iSazonov May 13, 2022

rkeithhill May 13, 2022 •

edited

TravisEz13 commented May 13, 2022

TravisEz13 commented May 13, 2022

pull-request-quantifier bot commented May 19, 2022

What can I do to optimize my changes

How to interpret the change counts in git diff output

msftbot bot commented May 26, 2022

rkeithhill commented Jun 3, 2022

daxian-dbw commented Jun 3, 2022

daxian-dbw commented Jun 9, 2022

WIP: Add PSConfig.json field to override threshold for slow profile loading msg #17330

WIP: Add PSConfig.json field to override threshold for slow profile loading msg #17330

Conversation

rkeithhill commented May 13, 2022

PR Summary

PR Context

PR Checklist

iSazonov May 13, 2022

Choose a reason for hiding this comment

rkeithhill May 13, 2022 • edited

Choose a reason for hiding this comment

TravisEz13 commented May 13, 2022

TravisEz13 commented May 13, 2022

pull-request-quantifier bot commented May 19, 2022

What can I do to optimize my changes

How to interpret the change counts in git diff output

msftbot bot commented May 26, 2022

rkeithhill commented Jun 3, 2022

daxian-dbw commented Jun 3, 2022

daxian-dbw commented Jun 9, 2022

rkeithhill May 13, 2022 •

edited