Move conditionals which control whether a task is placed in the JobEngine inside Watchdog.RunJob() (renamed from RunWhenPossible) and generalize them.

2 files changed, 62 insertions, 45 deletions

diff --git a/OpenSim/Framework/Monitoring/Watchdog.cs b/OpenSim/Framework/Monitoring/Watchdog.cs
index ebbf095..0feec7c 100644
--- a/OpenSim/Framework/Monitoring/Watchdog.cs
+++ b/OpenSim/Framework/Monitoring/Watchdog.cs
@@ -454,7 +454,38 @@ namespace OpenSim.Framework.Monitoring
             m_watchdogTimer.Start();
         }
 
-        public static void RunWhenPossible(string jobType, WaitCallback callback, string name, object obj, bool log = false)
+        /// <summary>
+        /// Run a job.
+        /// </summary>
+        /// <remarks>
+        /// This differs from direct scheduling (e.g. Util.FireAndForget) in that a job can be run in the job
+        /// engine if it is running, where all jobs are currently performed in sequence on a single thread.  This is
+        /// to prevent observed overload and server freeze problems when there are hundreds of connections which all attempt to 
+        /// perform work at once (e.g. in conference situations).  With lower numbers of connections, the small
+        /// delay in performing jobs in sequence rather than concurrently has not been notiecable in testing, though a future more 
+        /// sophisticated implementation could perform jobs concurrently when the server is under low load.
+        /// 
+        /// However, be advised that some callers of this function rely on all jobs being performed in sequence if any
+        /// jobs are performed in sequence (i.e. if jobengine is active or not).  Therefore, expanding the jobengine
+        /// beyond a single thread will require considerable thought.
+        /// 
+        /// Also, any jobs submitted must be guaranteed to complete within a reasonable timeframe (e.g. they cannot
+        /// incorporate a network delay with a long timeout).  At the moment, work that could suffer such issues 
+        /// should still be run directly with RunInThread(), Util.FireAndForget(), etc.  This is another area where
+        /// the job engine could be improved and so CPU utilization improved by better management of concurrency within
+        /// OpenSimulator.
+        /// </remarks>
+        /// <param name="jobType">General classification for the job (e.g. "RezAttachments").</param>
+        /// <param name="callback">Callback for job.</param>
+        /// <param name="name">Specific name of job (e.g. "RezAttachments for Joe Bloggs"</param>
+        /// <param name="obj">Object to pass to callback when run</param>
+        /// <param name="canRunInThisThread">If set to true then the job may be run in ths calling thread.</param>
+        /// <param name="mustNotTimeout">If the true then the job must never timeout.</param>
+        /// <param name="log">If set to true then extra logging is performed.</param>
+        public static void RunJob(
+            string jobType, WaitCallback callback, string name, object obj, 
+            bool canRunInThisThread = false, bool mustNotTimeout = false, 
+            bool log = false)
         {
             if (Util.FireAndForgetMethod == FireAndForgetMethod.RegressionTest)           
             {
@@ -465,8 +496,12 @@ namespace OpenSim.Framework.Monitoring
 
             if (JobEngine.IsRunning)
                 JobEngine.QueueRequest(name, callback, obj);
-            else
+            else if (canRunInThisThread)
+                callback(obj);
+            else if (mustNotTimeout)
                 RunInThread(callback, name, obj, log);
+            else
+                Util.FireAndForget(callback, obj, name);
         }
     }
 }
\ No newline at end of file
diff --git a/OpenSim/Region/Framework/Scenes/ScenePresence.cs b/OpenSim/Region/Framework/Scenes/ScenePresence.cs
index 1d234e2..757ec43 100644
--- a/OpenSim/Region/Framework/Scenes/ScenePresence.cs
+++ b/OpenSim/Region/Framework/Scenes/ScenePresence.cs
@@ -1228,15 +1228,11 @@ namespace OpenSim.Region.Framework.Scenes
                 // viewers without (e.g. v1 viewers) will not, so we still need to make this call.
                 if (Scene.AttachmentsModule != null)
                 {
-                    if (Watchdog.JobEngine.IsRunning)
-                        Watchdog.RunWhenPossible(
-                            "RezAttachments", 
-                            o => Scene.AttachmentsModule.RezAttachments(this), 
-                            string.Format("Rez attachments for {0} in {1}", Name, Scene.Name), 
-                            null);
-                    else
-                        Util.FireAndForget(
-                            o => Scene.AttachmentsModule.RezAttachments(this), null, "ScenePresence.RezAttachmentsOnLogin");
+                    Watchdog.RunJob(
+                        "RezAttachments", 
+                        o => Scene.AttachmentsModule.RezAttachments(this), 
+                        string.Format("Rez attachments for {0} in {1}", Name, Scene.Name), 
+                        null);
                 }
             }
             else
@@ -1258,18 +1254,12 @@ namespace OpenSim.Region.Framework.Scenes
 
                 if (attachments.Count > 0)
                 {
-                    if (Watchdog.JobEngine.IsRunning)
-                    {
-                        Watchdog.RunWhenPossible(
-                            "StartAttachmentScripts", 
-                            o => RestartAttachmentScripts(attachments),
-                            string.Format("Start attachment scripts for {0} in {1}", Name, Scene.Name), 
-                            null);
-                    }
-                    else
-                    {
-                        RestartAttachmentScripts(attachments);
-                    }
+                    Watchdog.RunJob(
+                        "StartAttachmentScripts", 
+                        o => RestartAttachmentScripts(attachments),
+                        string.Format("Start attachment scripts for {0} in {1}", Name, Scene.Name),
+                        null,
+                        true);
                 }
             }
 
@@ -1825,18 +1815,12 @@ namespace OpenSim.Region.Framework.Scenes
                 // XXX: If we force an update after activity has completed, then multiple attachments do appear correctly on a destination region
                 // If we do it a little bit earlier (e.g. when converting the child to a root agent) then this does not work.
                 // This may be due to viewer code or it may be something we're not doing properly simulator side.
-                if (Watchdog.JobEngine.IsRunning)
-                {
-                    Watchdog.RunWhenPossible(
-                        "ScheduleAttachmentsForFullUpdate", 
-                        o => ScheduleAttachmentsForFullUpdate(),
-                        string.Format("Schedule attachments for full update for {0} in {1}", Name, Scene.Name), 
-                        null);
-                }
-                else
-                {
-                    ScheduleAttachmentsForFullUpdate();
-                }
+                Watchdog.RunJob(
+                    "ScheduleAttachmentsForFullUpdate", 
+                    o => ScheduleAttachmentsForFullUpdate(),
+                    string.Format("Schedule attachments for full update for {0} in {1}", Name, Scene.Name),
+                    null,
+                    true);
 
     //            m_log.DebugFormat(
     //                "[SCENE PRESENCE]: Completing movement of {0} into region {1} took {2}ms", 
@@ -3391,7 +3375,7 @@ namespace OpenSim.Region.Framework.Scenes
             SentInitialDataToClient = true;
 
             // Send all scene object to the new client
-            Watchdog.RunWhenPossible("SendInitialDataToClient", delegate
+            Watchdog.RunJob("SendInitialDataToClient", delegate
             {
 //                m_log.DebugFormat(
 //                    "[SCENE PRESENCE]: Sending initial data to {0} agent {1} in {2}, tp flags {3}", 
@@ -3409,7 +3393,7 @@ namespace OpenSim.Region.Framework.Scenes
                     if (e != null && e is SceneObjectGroup)
                         ((SceneObjectGroup)e).SendFullUpdateToClient(ControllingClient);
                 }
-            }, string.Format("SendInitialDataToClient ({0} in {1})", Name, Scene.Name), null);
+            }, string.Format("SendInitialDataToClient ({0} in {1})", Name, Scene.Name),null,  false, true);
         }
 
         /// <summary>
@@ -4073,14 +4057,12 @@ namespace OpenSim.Region.Framework.Scenes
                 // We don't need to worry about a race condition as the job to later start the scripts is also 
                 // JobEngine scheduled and so will always occur after this task.
                 // XXX: This will not be true if JobEngine ever gets more than one thread.
-                if (Watchdog.JobEngine.IsRunning)
-                    Watchdog.RunWhenPossible(
-                        "CopyAttachments", 
-                        o => Scene.AttachmentsModule.CopyAttachments(cAgent, this), 
-                        string.Format("Copy attachments for {0} entering {1}", Name, Scene.Name), 
-                        null);
-                else
-                    Scene.AttachmentsModule.CopyAttachments(cAgent, this);
+                Watchdog.RunJob(
+                    "CopyAttachments", 
+                    o => Scene.AttachmentsModule.CopyAttachments(cAgent, this), 
+                    string.Format("Copy attachments for {0} entering {1}", Name, Scene.Name),
+                    null,
+                    true);
             }
 
             // This must occur after attachments are copied or scheduled to be copied, as it releases the CompleteMovement() calling thread