More changes and cleanup to reduce the startup time (#8396)

Major changes are:
- Rename `s_wasSystemPolicyDebugPolicy` to `s_allowDebugOverridePolicy` to make it less confusing. Also slightly refactor `HelperSecurity.psm1` and `ConstrainedLanguageDebugger.Tests.ps1` to remove unneeded code. There is no functional change in this commit.
- Remove the unneeded static property `IsInbox`, as PowerShell Core won't be shipped in-box with Windows in the foreseeable feature. Even if we do in future, we won't be needing it because Windows PowerShell will probably be gone by that time.
- Update 'BindRunspace' to avoid getting all commands and unneeded method calls.
- Avoid creating a `IsSafeValueVisitor` every time when `IsScriptBlockInFactASafeHashtable` runs.

src/System.Management.Automation/CoreCLR/CorePsPlatform.cs

@@ -121,32 +121,6 @@ namespace System.Management.Automation
             }
         }
 
-        /// <summary>
-        /// True if it is the inbox powershell for NanoServer or IoT.
-        /// </summary>
-        internal static bool IsInbox
-        {
-            get
-            {
-#if UNIX
-                return false;
-#else
-                if (_isInbox.HasValue) { return _isInbox.Value; }
-
-                _isInbox = false;
-                if (IsNanoServer || IsIoT)
-                {
-                    _isInbox = string.Equals(
-                        Utils.DefaultPowerShellAppBase,
-                        Utils.GetApplicationBaseFromRegistry(Utils.DefaultPowerShellShellID),
-                        StringComparison.OrdinalIgnoreCase);
-                }
-
-                return _isInbox.Value;
-#endif
-            }
-        }
-
         /// <summary>
         /// True if underlying system is Windows Desktop.
         /// </summary>
@@ -168,7 +142,6 @@ namespace System.Management.Automation
 #if !UNIX
         private static bool? _isNanoServer = null;
         private static bool? _isIoT = null;
-        private static bool? _isInbox = null;
         private static bool? _isWindowsDesktop = null;
 #endif
 

src/System.Management.Automation/engine/InitialSessionState.cs

@@ -2360,67 +2360,106 @@ namespace System.Management.Automation.Runspaces
 
         internal Exception BindRunspace(Runspace initializedRunspace, PSTraceSource runspaceInitTracer)
         {
-            // Get initial list of public commands in session.
-            HashSet<CommandInfo> publicCommands = new HashSet<CommandInfo>();
-            foreach (CommandInfo sessionCommand in initializedRunspace.ExecutionContext.SessionState.InvokeCommand.GetCommands(
-                        "*", CommandTypes.Alias | CommandTypes.Function | CommandTypes.Filter | CommandTypes.Cmdlet, true))
+            // Get the initial list of public commands from session in a lazy way, so that we can defer
+            // the work until it's actually needed.
+            //
+            // We could use Lazy<> with an initializer for the same purpose, but we can save allocations
+            // by using the local function. It avoids allocating the delegate, and it's more efficient on
+            // capturing variables from the enclosing scope by using a struct.
+            HashSet<CommandInfo> publicCommands = null;
+            HashSet<CommandInfo> GetPublicCommands()
             {
-                if (sessionCommand.Visibility == SessionStateEntryVisibility.Public)
+                if (publicCommands != null)
                 {
-                    publicCommands.Add(sessionCommand);
+                    return publicCommands;
                 }
+
+                publicCommands = new HashSet<CommandInfo>();
+                foreach (CommandInfo sessionCommand in initializedRunspace.ExecutionContext.SessionState.InvokeCommand.GetCommands(
+                            "*", CommandTypes.Alias | CommandTypes.Function | CommandTypes.Filter | CommandTypes.Cmdlet, nameIsPattern: true))
+                {
+                    if (sessionCommand.Visibility == SessionStateEntryVisibility.Public)
+                    {
+                        publicCommands.Add(sessionCommand);
+                    }
+                }
+                return publicCommands;
             }
 
-            // If a user has any module with the same name as that of the core module( or nested module inside the core module)
-            // in his module path, then that will get loaded instead of the actual nested module (from the GAC - in our case)
-            // Hence, searching only from the system module path while loading the core modules
             var unresolvedCmdsToExpose = new HashSet<string>(this.UnresolvedCommandsToExpose, StringComparer.OrdinalIgnoreCase);
-            ProcessImportModule(initializedRunspace, CoreModulesToImport, ModuleIntrinsics.GetPSHomeModulePath(), publicCommands, unresolvedCmdsToExpose);
+            if (CoreModulesToImport.Count > 0 || unresolvedCmdsToExpose.Count > 0)
+            {
+                // If a user has any module with the same name as that of the core module( or nested module inside the core module)
+                // in his module path, then that will get loaded instead of the actual nested module (from the GAC - in our case)
+                // Hence, searching only from the system module path while loading the core modules
+                ProcessModulesToImport(initializedRunspace, CoreModulesToImport, ModuleIntrinsics.GetPSHomeModulePath(), GetPublicCommands(), unresolvedCmdsToExpose);
+            }
 
             // Win8:328748 - functions defined in global scope end up in a module
             // Since we import the core modules, EngineSessionState's module is set to the last imported module. So, if a function is defined in global scope, it ends up in that module.
             // Setting the module to null fixes that.
             initializedRunspace.ExecutionContext.EngineSessionState.Module = null;
 
-            Exception moduleImportException = ProcessImportModule(initializedRunspace, ModuleSpecificationsToImport, string.Empty, publicCommands, unresolvedCmdsToExpose);
-            if (moduleImportException != null)
+            if (ModuleSpecificationsToImport.Count > 0 || unresolvedCmdsToExpose.Count > 0)
             {
-                runspaceInitTracer.WriteLine(
-                    "Runspace open failed while loading module: First error {1}", moduleImportException);
-                return moduleImportException;
+                Exception moduleImportException = ProcessModulesToImport(initializedRunspace, ModuleSpecificationsToImport, string.Empty, GetPublicCommands(), unresolvedCmdsToExpose);
+                if (moduleImportException != null)
+                {
+                    runspaceInitTracer.WriteLine(
+                        "Runspace open failed while loading module: First error {1}", moduleImportException);
+                    return moduleImportException;
+                }
             }
 
             // If we still have unresolved commands after importing specified modules, then try finding associated module for
             // each unresolved command and import that module.
-            string[] foundModuleList = GetModulesForUnResolvedCommands(unresolvedCmdsToExpose, initializedRunspace.ExecutionContext);
-            if (foundModuleList.Length > 0)
+            if (unresolvedCmdsToExpose.Count > 0)
             {
-                ProcessImportModule(initializedRunspace, foundModuleList, string.Empty, publicCommands, unresolvedCmdsToExpose);
+                string[] foundModuleList = GetModulesForUnResolvedCommands(unresolvedCmdsToExpose, initializedRunspace.ExecutionContext);
+                if (foundModuleList.Length > 0)
+                {
+                    ProcessModulesToImport(initializedRunspace, foundModuleList, string.Empty, GetPublicCommands(), unresolvedCmdsToExpose);
+                }
             }
 
-            ProcessDynamicVariables(initializedRunspace);
-            ProcessCommandModifications(initializedRunspace);
-
-            // Process User: drive
-            Exception userDriveException = ProcessUserDrive(initializedRunspace);
-            if (userDriveException != null)
+            // Process dynamic variables if any are defined.
+            if (DynamicVariablesToDefine.Count > 0)
             {
-                runspaceInitTracer.WriteLine(
-                                    "Runspace open failed while processing user drive with error {1}", userDriveException);
+                ProcessDynamicVariables(initializedRunspace);
+            }
 
-                Exception result = PSTraceSource.NewInvalidOperationException(userDriveException, RemotingErrorIdStrings.UserDriveProcessingThrewTerminatingError, userDriveException.Message);
-                return result;
+            // Process command modifications if any are defined.
+            if (CommandModifications.Count > 0)
+            {
+                ProcessCommandModifications(initializedRunspace);
+            }
+
+            // Process the 'User:' drive if 'UserDriveEnabled' is set.
+            if (UserDriveEnabled)
+            {
+                Exception userDriveException = ProcessUserDrive(initializedRunspace);
+                if (userDriveException != null)
+                {
+                    runspaceInitTracer.WriteLine(
+                                        "Runspace open failed while processing user drive with error {1}", userDriveException);
+
+                    Exception result = PSTraceSource.NewInvalidOperationException(userDriveException, RemotingErrorIdStrings.UserDriveProcessingThrewTerminatingError, userDriveException.Message);
+                    return result;
+                }
             }
 
             // Process startup scripts
-            Exception startupScriptException = ProcessStartupScripts(initializedRunspace);
-            if (startupScriptException != null)
+            if (StartupScripts.Count > 0)
             {
-                runspaceInitTracer.WriteLine(
-                    "Runspace open failed while running startup script: First error {1}", startupScriptException);
+                Exception startupScriptException = ProcessStartupScripts(initializedRunspace);
+                if (startupScriptException != null)
+                {
+                    runspaceInitTracer.WriteLine(
+                        "Runspace open failed while running startup script: First error {1}", startupScriptException);
 
-                Exception result = PSTraceSource.NewInvalidOperationException(startupScriptException, RemotingErrorIdStrings.StartupScriptThrewTerminatingError, startupScriptException.Message);
-                return result;
+                    Exception result = PSTraceSource.NewInvalidOperationException(startupScriptException, RemotingErrorIdStrings.StartupScriptThrewTerminatingError, startupScriptException.Message);
+                    return result;
+                }
             }
 
             // Start transcribing
@@ -2639,8 +2678,6 @@ namespace System.Management.Automation.Runspaces
 
         private Exception ProcessUserDrive(Runspace initializedRunspace)
         {
-            if (!UserDriveEnabled) { return null; }
-
             Exception ex = null;
             try
             {
@@ -2805,7 +2842,7 @@ namespace System.Management.Automation.Runspaces
             return null;
         }
 
-        private RunspaceOpenModuleLoadException ProcessImportModule(
+        private RunspaceOpenModuleLoadException ProcessModulesToImport(
             Runspace initializedRunspace,
             IEnumerable moduleList,
             string path,
@@ -2819,7 +2856,7 @@ namespace System.Management.Automation.Runspaces
                 string moduleName = module as string;
                 if (moduleName != null)
                 {
-                    exceptionToReturn = ProcessImportModule(initializedRunspace, moduleName, null, path, publicCommands);
+                    exceptionToReturn = ProcessOneModule(initializedRunspace, moduleName, null, path, publicCommands);
                 }
                 else
                 {
@@ -2830,7 +2867,7 @@ namespace System.Management.Automation.Runspaces
                         {
                             // if only name is specified in the module spec, just try import the module
                             // ie., don't take the performance overhead of calling GetModule.
-                            exceptionToReturn = ProcessImportModule(initializedRunspace, moduleSpecification.Name, null, path, publicCommands);
+                            exceptionToReturn = ProcessOneModule(initializedRunspace, moduleSpecification.Name, null, path, publicCommands);
                         }
                         else
                         {
@@ -2838,7 +2875,7 @@ namespace System.Management.Automation.Runspaces
 
                             if (moduleInfos != null && moduleInfos.Count > 0)
                             {
-                                exceptionToReturn = ProcessImportModule(initializedRunspace, moduleSpecification.Name, moduleInfos[0], path, publicCommands);
+                                exceptionToReturn = ProcessOneModule(initializedRunspace, moduleSpecification.Name, moduleInfos[0], path, publicCommands);
                             }
                             else
                             {
@@ -2979,7 +3016,7 @@ namespace System.Management.Automation.Runspaces
         /// if <paramref name="moduleInfoToLoad"/> is null, import module using <paramref name="name"/>. Otherwise,
         /// import module using <paramref name="moduleInfoToLoad"/>
         /// </summary>
-        private RunspaceOpenModuleLoadException ProcessImportModule(Runspace initializedRunspace, string name, PSModuleInfo moduleInfoToLoad, string path, HashSet<CommandInfo> publicCommands)
+        private RunspaceOpenModuleLoadException ProcessOneModule(Runspace initializedRunspace, string name, PSModuleInfo moduleInfoToLoad, string path, HashSet<CommandInfo> publicCommands)
         {
             using (PowerShell pse = PowerShell.Create())
             {

src/System.Management.Automation/engine/Utils.cs

@@ -480,7 +480,7 @@ namespace System.Management.Automation
         /// <summary>
         /// This is used to construct the profile path.
         /// </summary>
-        internal static string ProductNameForDirectory = Platform.IsInbox ? "WindowsPowerShell" : "PowerShell";
+        internal const string ProductNameForDirectory = "PowerShell";
 
         /// <summary>
         /// The subdirectory of module paths

src/System.Management.Automation/engine/parser/SafeValues.cs

@@ -41,11 +41,7 @@ namespace System.Management.Automation.Language
         public static bool IsAstSafe(Ast ast, GetSafeValueVisitor.SafeValueContext safeValueContext)
         {
             IsSafeValueVisitor visitor = new IsSafeValueVisitor(safeValueContext);
-            if ((bool)ast.Accept(visitor) && visitor._visitCount < MaxVisitCount)
-            {
-                return true;
-            }
-            return false;
+            return visitor.IsAstSafe(ast);
         }
 
         internal IsSafeValueVisitor(GetSafeValueVisitor.SafeValueContext safeValueContext)
@@ -53,15 +49,27 @@ namespace System.Management.Automation.Language
             _safeValueContext = safeValueContext;
         }
 
+        internal bool IsAstSafe(Ast ast)
+        {
+            if ((bool)ast.Accept(this) && _visitCount < MaxVisitCount)
+            {
+                return true;
+            }
+            return false;
+        }
+
+        // A readonly singleton with the default SafeValueContext.
+        internal readonly static IsSafeValueVisitor Default = new IsSafeValueVisitor(GetSafeValueVisitor.SafeValueContext.Default);
+
         // This is a check of the number of visits
-        private int _visitCount = 0;
-        private const int MaxVisitCount = 5000;
+        private uint _visitCount = 0;
+        private const uint MaxVisitCount = 5000;
         private const int MaxHashtableKeyCount = 500;
 
         // Used to determine if we are being called within a GetPowerShell() context,
         // which does some additional security verification outside of the scope of
         // what we can verify.
-        private GetSafeValueVisitor.SafeValueContext _safeValueContext;
+        private readonly GetSafeValueVisitor.SafeValueContext _safeValueContext;
 
         public object VisitErrorStatement(ErrorStatementAst errorStatementAst) { return false; }
         public object VisitErrorExpression(ErrorExpressionAst errorExpressionAst) { return false; }

src/System.Management.Automation/engine/runtime/CompiledScriptBlock.cs

@@ -263,7 +263,7 @@ namespace System.Management.Automation
 
                 // After the above steps, we know the ScriptBlockAst is in fact just a HashtableAst,
                 // now we need to check if the HashtableAst is safe.
-                return IsSafeValueVisitor.IsAstSafe(hashtableAst, GetSafeValueVisitor.SafeValueContext.Default);
+                return IsSafeValueVisitor.Default.IsAstSafe(hashtableAst);
             }
         }
 

src/System.Management.Automation/security/wldpNativeMethods.cs

@@ -48,11 +48,11 @@ namespace System.Management.Automation.Security
         /// <returns>An EnforcementMode that describes the system policy</returns>
         public static SystemEnforcementMode GetSystemLockdownPolicy()
         {
-            if (s_wasSystemPolicyDebugPolicy || (s_systemLockdownPolicy == null))
+            if (s_allowDebugOverridePolicy || (s_systemLockdownPolicy == null))
             {
                 lock (s_systemLockdownPolicyLock)
                 {
-                    if (s_wasSystemPolicyDebugPolicy || (s_systemLockdownPolicy == null))
+                    if (s_allowDebugOverridePolicy || (s_systemLockdownPolicy == null))
                     {
                         s_systemLockdownPolicy = GetLockdownPolicy(null, null);
                     }
@@ -64,7 +64,7 @@ namespace System.Management.Automation.Security
 
         private static object s_systemLockdownPolicyLock = new Object();
         private static Nullable<SystemEnforcementMode> s_systemLockdownPolicy = null;
-        private static bool s_wasSystemPolicyDebugPolicy = false;
+        private static bool s_allowDebugOverridePolicy = false;
 
         /// <summary>
         /// Gets lockdown policy as applied to a file
@@ -340,7 +340,7 @@ namespace System.Management.Automation.Security
 
         private static SystemEnforcementMode GetDebugLockdownPolicy(string path)
         {
-            s_wasSystemPolicyDebugPolicy = true;
+            s_allowDebugOverridePolicy = true;
 
             // Support fall-back debug hook for path exclusions on non-WOA platforms
             if (path != null)
@@ -419,7 +419,7 @@ namespace System.Management.Automation.Security
                         // Hook for testability. If we've got an environmental override, say that ADODB.Parameter
                         // is not allowed.
                         // 0000050b-0000-0010-8000-00aa006d2ea4 = ADODB.Parameter
-                        if (s_wasSystemPolicyDebugPolicy)
+                        if (s_allowDebugOverridePolicy)
                         {
                             if (String.Equals(clsid.ToString(), "0000050b-0000-0010-8000-00aa006d2ea4", StringComparison.OrdinalIgnoreCase))
                             {

test/powershell/Modules/Microsoft.PowerShell.Security/ConstrainedLanguageDebugger.Tests.ps1

@@ -141,13 +141,12 @@ try
             # script debugger command processor.
             $script = @'
                 Import-Module -Name HelpersSecurity
-                Import-Module -Name {0} -Force
                 Invoke-LanguageModeTestingSupportCmdlet -SetLockdownMode
                 $ExecutionContext.SessionState.LanguageMode = "ConstrainedLanguage"
-                Import-Module -Name {1} -Force
+                Import-Module -Name {0} -Force
                 Set-PSBreakpoint -Command PublicFn
                 PublicFn
-'@ -f "$languageModuleDirectory\TestCmdletForConstrainedLanguage.dll", $trustedManifestFilePath
+'@ -f $trustedManifestFilePath
 
             [powershell] $ps = [powershell]::Create()
             $ps.Runspace = $runspace

test/tools/Modules/HelpersSecurity/HelpersSecurity.psm1

@@ -10,14 +10,6 @@ if ($IsWindows)
     #region Using directives
 
     using System;
-    using System.Globalization;
-    using System.Reflection;
-    using System.Collections;
-    using System.Collections.Generic;
-    using System.IO;
-    using System.Security;
-    using System.Runtime.InteropServices;
-    using System.Threading;
     using System.Management.Automation;
 
     #endregion
@@ -27,42 +19,27 @@ if ($IsWindows)
     public sealed class InvokeLanguageModeTestingSupportCmdlet : PSCmdlet
     {
         [Parameter()]
-        public SwitchParameter EnableFullLanguageMode
-        {
-            get { return enableFullLanguageMode; }
-            set { enableFullLanguageMode = value; }
-        }
-        private SwitchParameter enableFullLanguageMode;
+        public SwitchParameter EnableFullLanguageMode { get; set; }
 
         [Parameter()]
-        public SwitchParameter SetLockdownMode
-        {
-            get { return setLockdownMode; }
-            set { setLockdownMode = value; }
-        }
-        private SwitchParameter setLockdownMode;
+        public SwitchParameter SetLockdownMode { get; set; }
 
         [Parameter()]
-        public SwitchParameter RevertLockdownMode
-        {
-            get { return revertLockdownMode; }
-            set { revertLockdownMode = value; }
-        }
-        private SwitchParameter revertLockdownMode;
+        public SwitchParameter RevertLockdownMode { get; set; }
 
         protected override void BeginProcessing()
         {
-            if (enableFullLanguageMode)
+            if (EnableFullLanguageMode)
             {
                 SessionState.LanguageMode = PSLanguageMode.FullLanguage;
             }
 
-            if (setLockdownMode)
+            if (SetLockdownMode)
             {
                 Environment.SetEnvironmentVariable("__PSLockdownPolicy", "0x80000007", EnvironmentVariableTarget.Machine);
             }
 
-            if (revertLockdownMode)
+            if (RevertLockdownMode)
             {
                 Environment.SetEnvironmentVariable("__PSLockdownPolicy", null, EnvironmentVariableTarget.Machine);
             }