make parallel rewriting optional

docs/release-notes.md

@@ -9,7 +9,7 @@
 
 ## Upcoming release
 * For players:
-  * Reduced startup time when loading mod DLLs (thanks to ZaneYork!).
+  * Added experimental option to reduce startup time when loading mod DLLs (thanks to ZaneYork!). Enable `RewriteInParallel` in the `smapi-internal/config.json` to try it.
   * Reduced processing time when a mod loads many unpacked images (thanks to Entoarox!).
   * Mod warnings are now listed alphabetically.
   * MacOS files starting with `._` are now ignored and can no longer cause skipped mods.

src/SMAPI/Constants.cs

@@ -52,6 +52,14 @@ namespace StardewModdingAPI
         /****
         ** Internal
         ****/
+        /// <summary>Whether SMAPI was compiled in debug mode.</summary>
+        internal const bool IsDebugBuild =
+#if DEBUG
+            true;
+#else
+            false;
+#endif
+
         /// <summary>The URL of the SMAPI home page.</summary>
         internal const string HomePageUrl = "https://smapi.io";
 

src/SMAPI/Framework/ModLoading/AssemblyLoader.cs

@@ -76,9 +76,10 @@ namespace StardewModdingAPI.Framework.ModLoading
         /// <param name="mod">The mod for which the assembly is being loaded.</param>
         /// <param name="assemblyPath">The assembly file path.</param>
         /// <param name="assumeCompatible">Assume the mod is compatible, even if incompatible code is detected.</param>
+        /// <param name="rewriteInParallel">Whether to enable experimental parallel rewriting.</param>
         /// <returns>Returns the rewrite metadata for the preprocessed assembly.</returns>
         /// <exception cref="IncompatibleInstructionException">An incompatible CIL instruction was found while rewriting the assembly.</exception>
-        public Assembly Load(IModMetadata mod, string assemblyPath, bool assumeCompatible)
+        public Assembly Load(IModMetadata mod, string assemblyPath, bool assumeCompatible, bool rewriteInParallel)
         {
             // get referenced local assemblies
             AssemblyParseResult[] assemblies;
@@ -108,7 +109,7 @@ namespace StardewModdingAPI.Framework.ModLoading
                     continue;
 
                 // rewrite assembly
-                bool changed = this.RewriteAssembly(mod, assembly.Definition, loggedMessages, logPrefix: "      ");
+                bool changed = this.RewriteAssembly(mod, assembly.Definition, loggedMessages, logPrefix: "      ", rewriteInParallel);
 
                 // detect broken assembly reference
                 foreach (AssemblyNameReference reference in assembly.Definition.MainModule.AssemblyReferences)
@@ -262,9 +263,10 @@ namespace StardewModdingAPI.Framework.ModLoading
         /// <param name="assembly">The assembly to rewrite.</param>
         /// <param name="loggedMessages">The messages that have already been logged for this mod.</param>
         /// <param name="logPrefix">A string to prefix to log messages.</param>
+        /// <param name="rewriteInParallel">Whether to enable experimental parallel rewriting.</param>
         /// <returns>Returns whether the assembly was modified.</returns>
         /// <exception cref="IncompatibleInstructionException">An incompatible CIL instruction was found while rewriting the assembly.</exception>
-        private bool RewriteAssembly(IModMetadata mod, AssemblyDefinition assembly, HashSet<string> loggedMessages, string logPrefix)
+        private bool RewriteAssembly(IModMetadata mod, AssemblyDefinition assembly, HashSet<string> loggedMessages, string logPrefix, bool rewriteInParallel)
         {
             ModuleDefinition module = assembly.MainModule;
             string filename = $"{assembly.Name.Name}.dll";
@@ -313,7 +315,7 @@ namespace StardewModdingAPI.Framework.ModLoading
                     return rewritten;
                 }
             );
-            bool anyRewritten = rewriter.RewriteModule();
+            bool anyRewritten = rewriter.RewriteModule(rewriteInParallel);
 
             // handle rewrite flags
             foreach (IInstructionHandler handler in handlers)

src/SMAPI/Framework/ModLoading/Framework/RecursiveRewriter.cs

@@ -1,4 +1,5 @@
 using System;
+using System.Collections.Generic;
 using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
@@ -56,15 +57,20 @@ namespace StardewModdingAPI.Framework.ModLoading.Framework
         }
 
         /// <summary>Rewrite the loaded module code.</summary>
+        /// <param name="rewriteInParallel">Whether to enable experimental parallel rewriting.</param>
         /// <returns>Returns whether the module was modified.</returns>
-        public bool RewriteModule()
+        public bool RewriteModule(bool rewriteInParallel)
         {
-            int typesChanged = 0;
+            IEnumerable<TypeDefinition> types = this.Module.GetTypes().Where(type => type.BaseType != null); // skip special types like <Module>
-            Exception exception = null;
 
-            Parallel.ForEach(
+            // experimental parallel rewriting
-                source: this.Module.GetTypes().Where(type => type.BaseType != null), // skip special types like <Module>
+            // This may cause intermittent startup errors and is disabled by default: https://github.com/Pathoschild/SMAPI/issues/721
-                body: type =>
+            if (rewriteInParallel)
+            {
+                int typesChanged = 0;
+                Exception exception = null;
+
+                Parallel.ForEach(types, type =>
                 {
                     if (exception != null)
                         return;
@@ -72,50 +78,7 @@ namespace StardewModdingAPI.Framework.ModLoading.Framework
                     bool changed = false;
                     try
                     {
-                        changed |= this.RewriteCustomAttributes(type.CustomAttributes);
+                        changed = this.RewriteTypeDefinition(type);
-                        changed |= this.RewriteGenericParameters(type.GenericParameters);
-
-                        foreach (InterfaceImplementation @interface in type.Interfaces)
-                            changed |= this.RewriteTypeReference(@interface.InterfaceType, newType => @interface.InterfaceType = newType);
-
-                        if (type.BaseType.FullName != "System.Object")
-                            changed |= this.RewriteTypeReference(type.BaseType, newType => type.BaseType = newType);
-
-                        foreach (MethodDefinition method in type.Methods)
-                        {
-                            changed |= this.RewriteTypeReference(method.ReturnType, newType => method.ReturnType = newType);
-                            changed |= this.RewriteGenericParameters(method.GenericParameters);
-                            changed |= this.RewriteCustomAttributes(method.CustomAttributes);
-
-                            foreach (ParameterDefinition parameter in method.Parameters)
-                                changed |= this.RewriteTypeReference(parameter.ParameterType, newType => parameter.ParameterType = newType);
-
-                            foreach (var methodOverride in method.Overrides)
-                                changed |= this.RewriteMethodReference(methodOverride);
-
-                            if (method.HasBody)
-                            {
-                                foreach (VariableDefinition variable in method.Body.Variables)
-                                    changed |= this.RewriteTypeReference(variable.VariableType, newType => variable.VariableType = newType);
-
-                                // check CIL instructions
-                                ILProcessor cil = method.Body.GetILProcessor();
-                                Collection<Instruction> instructions = cil.Body.Instructions;
-                                for (int i = 0; i < instructions.Count; i++)
-                                {
-                                    var instruction = instructions[i];
-                                    if (instruction.OpCode.Code == Code.Nop)
-                                        continue;
-
-                                    changed |= this.RewriteInstruction(instruction, cil, newInstruction =>
-                                    {
-                                        changed = true;
-                                        cil.Replace(instruction, newInstruction);
-                                        instruction = newInstruction;
-                                    });
-                                }
-                            }
-                        }
                     }
                     catch (Exception ex)
                     {
@@ -124,18 +87,90 @@ namespace StardewModdingAPI.Framework.ModLoading.Framework
 
                     if (changed)
                         Interlocked.Increment(ref typesChanged);
-                }
+                });
-            );
 
-            return exception == null
+                return exception == null
-                 ? typesChanged > 0
+                    ? typesChanged > 0
-                 : throw new Exception($"Rewriting {this.Module.Name} failed.", exception);
+                    : throw new Exception($"Rewriting {this.Module.Name} failed.", exception);
+            }
+
+            // non-parallel rewriting
+            {
+                bool changed = false;
+
+                try
+                {
+                    foreach (var type in types)
+                        changed |= this.RewriteTypeDefinition(type);
+                }
+                catch (Exception ex)
+                {
+                    throw new Exception($"Rewriting {this.Module.Name} failed.", ex);
+                }
+
+                return changed;
+            }
         }
 
 
         /*********
         ** Private methods
         *********/
+        /// <summary>Rewrite a loaded type definition.</summary>
+        /// <param name="type">The type definition to rewrite.</param>
+        /// <returns>Returns whether the type was modified.</returns>
+        private bool RewriteTypeDefinition(TypeDefinition type)
+        {
+            bool changed = false;
+
+            changed |= this.RewriteCustomAttributes(type.CustomAttributes);
+            changed |= this.RewriteGenericParameters(type.GenericParameters);
+
+            foreach (InterfaceImplementation @interface in type.Interfaces)
+                changed |= this.RewriteTypeReference(@interface.InterfaceType, newType => @interface.InterfaceType = newType);
+
+            if (type.BaseType.FullName != "System.Object")
+                changed |= this.RewriteTypeReference(type.BaseType, newType => type.BaseType = newType);
+
+            foreach (MethodDefinition method in type.Methods)
+            {
+                changed |= this.RewriteTypeReference(method.ReturnType, newType => method.ReturnType = newType);
+                changed |= this.RewriteGenericParameters(method.GenericParameters);
+                changed |= this.RewriteCustomAttributes(method.CustomAttributes);
+
+                foreach (ParameterDefinition parameter in method.Parameters)
+                    changed |= this.RewriteTypeReference(parameter.ParameterType, newType => parameter.ParameterType = newType);
+
+                foreach (var methodOverride in method.Overrides)
+                    changed |= this.RewriteMethodReference(methodOverride);
+
+                if (method.HasBody)
+                {
+                    foreach (VariableDefinition variable in method.Body.Variables)
+                        changed |= this.RewriteTypeReference(variable.VariableType, newType => variable.VariableType = newType);
+
+                    // check CIL instructions
+                    ILProcessor cil = method.Body.GetILProcessor();
+                    Collection<Instruction> instructions = cil.Body.Instructions;
+                    for (int i = 0; i < instructions.Count; i++)
+                    {
+                        var instruction = instructions[i];
+                        if (instruction.OpCode.Code == Code.Nop)
+                            continue;
+
+                        changed |= this.RewriteInstruction(instruction, cil, newInstruction =>
+                        {
+                            changed = true;
+                            cil.Replace(instruction, newInstruction);
+                            instruction = newInstruction;
+                        });
+                    }
+                }
+            }
+
+            return changed;
+        }
+
         /// <summary>Rewrite a CIL instruction if needed.</summary>
         /// <param name="instruction">The current CIL instruction.</param>
         /// <param name="cil">The CIL instruction processor.</param>

src/SMAPI/Framework/Models/SConfig.cs

@@ -15,12 +15,8 @@ namespace StardewModdingAPI.Framework.Models
         private static readonly IDictionary<string, object> DefaultValues = new Dictionary<string, object>
         {
             [nameof(CheckForUpdates)] = true,
-            [nameof(ParanoidWarnings)] =
+            [nameof(ParanoidWarnings)] = Constants.IsDebugBuild,
-#if DEBUG
+            [nameof(RewriteInParallel)] = Constants.IsDebugBuild,
-                true,
-#else
-                false,
-#endif
             [nameof(UseBetaChannel)] = Constants.ApiVersion.IsPrerelease(),
             [nameof(GitHubProjectName)] = "Pathoschild/SMAPI",
             [nameof(WebApiBaseUrl)] = "https://smapi.io/api/",
@@ -45,6 +41,9 @@ namespace StardewModdingAPI.Framework.Models
         /// <summary>Whether to check for newer versions of SMAPI and mods on startup.</summary>
         public bool CheckForUpdates { get; set; }
 
+        /// <summary>Whether to enable experimental parallel rewriting.</summary>
+        public bool RewriteInParallel { get; set; } = (bool)SConfig.DefaultValues[nameof(SConfig.RewriteInParallel)];
+
         /// <summary>Whether to add a section to the 'mod issues' list for mods which which directly use potentially sensitive .NET APIs like file or shell access.</summary>
         public bool ParanoidWarnings { get; set; } = (bool)SConfig.DefaultValues[nameof(SConfig.ParanoidWarnings)];
 

src/SMAPI/Framework/SCore.cs

@@ -337,6 +337,8 @@ namespace StardewModdingAPI.Framework
             // add headers
             if (this.Settings.DeveloperMode)
                 this.Monitor.Log($"You have SMAPI for developers, so the console will be much more verbose. You can disable developer mode by installing the non-developer version of SMAPI, or by editing {Constants.ApiConfigPath}.", LogLevel.Info);
+            if (this.Settings.RewriteInParallel)
+                this.Monitor.Log($"You enabled experimental parallel rewriting. This may result in faster startup times, but intermittent startup errors. You can disable it by reinstalling SMAPI or editing {Constants.ApiConfigPath}.", LogLevel.Info);
             if (!this.Settings.CheckForUpdates)
                 this.Monitor.Log($"You configured SMAPI to not check for updates. Running an old version of SMAPI is not recommended. You can enable update checks by reinstalling SMAPI or editing {Constants.ApiConfigPath}.", LogLevel.Warn);
             if (!this.Monitor.WriteToConsole)
@@ -981,7 +983,7 @@ namespace StardewModdingAPI.Framework
                 Assembly modAssembly;
                 try
                 {
-                    modAssembly = assemblyLoader.Load(mod, assemblyPath, assumeCompatible: mod.DataRecord?.Status == ModStatus.AssumeCompatible);
+                    modAssembly = assemblyLoader.Load(mod, assemblyPath, assumeCompatible: mod.DataRecord?.Status == ModStatus.AssumeCompatible, rewriteInParallel: this.Settings.RewriteInParallel);
                     this.ModRegistry.TrackAssemblies(mod, modAssembly);
                 }
                 catch (IncompatibleInstructionException) // details already in trace logs

src/SMAPI/SMAPI.config.json

@@ -33,18 +33,30 @@ copy all the settings, or you may cause bugs due to overridden changes in future
      */
     "DeveloperMode": true,
 
+    /**
+     * Whether to enable experimental parallel rewriting when SMAPI is loading mods. This can
+     * reduce startup time when you have many mods installed, but is experimental and may cause
+     * intermittent startup errors.
+     *
+     * When this is commented out, it'll be true for local debug builds and false otherwise.
+     */
+    //"RewriteInParallel": false,
+
     /**
      * Whether to add a section to the 'mod issues' list for mods which directly use potentially
      * sensitive .NET APIs like file or shell access. Note that many mods do this legitimately as
      * part of their normal functionality, so these warnings are meaningless without further
-     * investigation. When this is commented out, it'll be true for local debug builds and false
+     * investigation.
-     * otherwise.
+     *
+     * When this is commented out, it'll be true for local debug builds and false otherwise.
      */
     //"ParanoidWarnings": true,
 
     /**
-     * Whether SMAPI should show newer beta versions as an available update. When this is commented
+     * Whether SMAPI should show newer beta versions as an available update.
-     * out, it'll be true if the current SMAPI version is beta, and false otherwise.
+     *
+     * When this is commented out, it'll be true if the current SMAPI version is beta, and false
+     * otherwise.
      */
     //"UseBetaChannel": true,