[Mono-dev] [PATCH] Optimizations for FileSystemWatcher and MulticastDelegate

[Juraj Skripsky]

Hello Miguel,

On Thu, 2007-07-19 at 12:04 -0400, Miguel de Icaza wrote:
> Hello Juraj,
> 
>     can you explain the rationale for the changes in MulticastDelegate,
> for example, why not return this.Clone () instead of the new construct
> new Delegate [1] { this }?
> 
>     Also, should we not be checking prev/next in the method?
> 

The new code distinguishes between two cases (see the diff below):

a) We don't have a chain, the only delegate to return is 'this'
   (or rather an array only containing one element: 'this').

   The old code did far too much work in this case:
   - clone this
   - build forward linked list (0 iterations as d.prev == null)
   - enter the if-block (as d.kpm_next was set to null)
   - clone the clone
   - set other.prev and other.kpm_next to null (they already are!)
   - return "new Delegate[1] { other }" where other.Equals(this)

   So it cloned the delegate twice and finally returned an
   array containing the second clone (which equals 'this').
   
   The new code does the same without all the unnecessary steps.

b) This stays the same.


     public sealed override Delegate[] GetInvocationList ()
     {
-        MulticastDelegate d;
-        d = (MulticastDelegate) this.Clone ();
+        // does the list only contain ourself?
+        if (prev == null)
+            return new Delegate [1] { this };
+
+        // walk the list backwards, connecting the links the other way 
+        MulticastDelegate d = this;
         for (d.kpm_next = null; d.prev != null; d = d.prev)
             d.prev.kpm_next = d;
 
-        if (d.kpm_next == null) {
-            MulticastDelegate other = (MulticastDelegate) d.Clone ();
-            other.prev = null;
-            other.kpm_next = null;
-            return new Delegate [1] { other };
-        }
-
+        // walk the list, building the invocation list
         ArrayList list = new ArrayList ();
         for (; d != null; d = d.kpm_next) {
             MulticastDelegate other = (MulticastDelegate) d.Clone ();


- Juraj