Skip to content

Commit

Permalink
[delegate] Replace multicast delegate implementation
Browse files Browse the repository at this point in the history 
Replace the reversed linked implementation by an array based implementation. This will improve Combine performance, as well as ease delegate to virtual function optimization.
  • Loading branch information
@luhenry
luhenry committed May 5, 2015
1 parent 690aa51 commit 611a43e
Show file tree
Hide file tree
Showing 17 changed files with 412 additions and 238 deletions.
16 changes: 10 additions & 6 deletions mcs/class/corlib/System/Delegate.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,9 +123,6 @@ public object Target {
[MethodImplAttribute (MethodImplOptions.InternalCall)]
internal static extern Delegate CreateDelegate_internal (Type type, object target, MethodInfo info, bool throwOnBindFailure);

[MethodImplAttribute (MethodImplOptions.InternalCall)]
internal extern void SetMulticastInvoke ();

private static bool arg_type_match (Type delArgType, Type argType) {
bool match = delArgType == argType;

Expand Down Expand Up @@ -186,11 +183,12 @@ static Delegate CreateDelegate (Type type, object firstArgument, MethodInfo meth

MethodInfo invoke = type.GetMethod ("Invoke");

if (!return_type_match (invoke.ReturnType, method.ReturnType))
if (!return_type_match (invoke.ReturnType, method.ReturnType)) {
if (throwOnBindFailure)
throw new ArgumentException ("method return type is incompatible");
else
return null;
}

ParameterInfo[] delargs = invoke.GetParametersInternal ();
ParameterInfo[] args = method.GetParametersInternal ();
Expand Down Expand Up @@ -224,11 +222,12 @@ static Delegate CreateDelegate (Type type, object firstArgument, MethodInfo meth
argLengthMatch = args.Length == delargs.Length + 1;
}
}
if (!argLengthMatch)
if (!argLengthMatch) {
if (throwOnBindFailure)
throw new ArgumentException ("method argument length mismatch");
else
return null;
}

bool argsMatch;
DelegateData delegate_data = new DelegateData ();
Expand Down Expand Up @@ -274,11 +273,12 @@ static Delegate CreateDelegate (Type type, object firstArgument, MethodInfo meth
}
}

if (!argsMatch)
if (!argsMatch) {
if (throwOnBindFailure)
throw new ArgumentException ("method arguments are incompatible");
else
return null;
}

Delegate d = CreateDelegate_internal (type, target, method, throwOnBindFailure);
if (d != null)
Expand Down Expand Up @@ -614,5 +614,9 @@ internal static Delegate CreateDelegateNoSecurityCheck (RuntimeType type, Object
{
return CreateDelegate_internal (type, firstArgument, method, true);
}

/* Internal call largely inspired from MS Delegate.InternalAllocLike */
[MethodImplAttribute(MethodImplOptions.InternalCall)]
internal extern static MulticastDelegate AllocDelegateLike_internal (Delegate d);
}
}
2 changes: 1 addition & 1 deletion mcs/class/corlib/System/Environment.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public static partial class Environment {
* of icalls, do not require an increment.
*/
#pragma warning disable 169
private const int mono_corlib_version = 130;
private const int mono_corlib_version = 131;
#pragma warning restore 169

[ComVisible (true)]
Expand Down
258 changes: 116 additions & 142 deletions mcs/class/corlib/System/MulticastDelegate.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,7 @@ namespace System
[StructLayout (LayoutKind.Sequential)]
public abstract class MulticastDelegate : Delegate
{
MulticastDelegate prev;
MulticastDelegate kpm_next;
Delegate[] delegates;

protected MulticastDelegate (object target, string method)
: base (target, method)
Expand All @@ -61,13 +60,18 @@ public override void GetObjectData (SerializationInfo info, StreamingContext con
base.GetObjectData (info, context);
}


protected sealed override object DynamicInvokeImpl (object[] args)
{
if (prev != null)
prev.DynamicInvokeImpl (args);

return base.DynamicInvokeImpl (args);
if (delegates == null) {
return base.DynamicInvokeImpl (args);
} else {
object r;
int i = 0, len = delegates.Length;
do {
r = delegates [i].DynamicInvoke (args);
} while (++i < len);
return r;
}
}

// <remarks>
Expand All @@ -83,19 +87,21 @@ public sealed override bool Equals (object obj)
if (d == null)
return false;

MulticastDelegate this_prev = this.prev;
MulticastDelegate obj_prev = d.prev;
if (delegates == null && d.delegates == null) {
return true;
} else if (delegates == null ^ d.delegates == null) {
return false;
} else {
if (delegates.Length != d.delegates.Length)
return false;

do {
if (this_prev == null)
return obj_prev == null;
for (int i = 0; i < delegates.Length; ++i) {
if (!delegates [i].Equals (d.delegates [i]))
return false;
}

if (!this_prev.Compare (obj_prev))
return false;

this_prev = this_prev.prev;
obj_prev = obj_prev.prev;
} while (true);
return true;
}
}

//
Expand All @@ -112,27 +118,10 @@ public sealed override int GetHashCode ()
// </summary>
public sealed override Delegate[] GetInvocationList ()
{
MulticastDelegate d;
d = (MulticastDelegate) this.Clone ();
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 };
}

var list = new List<Delegate> ();
for (; d != null; d = d.kpm_next) {
MulticastDelegate other = (MulticastDelegate) d.Clone ();
other.prev = null;
other.kpm_next = null;
list.Add (other);
}

return list.ToArray ();
if (delegates != null)
return (Delegate[]) delegates.Clone ();
else
return new Delegate[1] { this };
}

// <summary>
Expand All @@ -146,141 +135,126 @@ protected sealed override Delegate CombineImpl (Delegate follow)
if (follow == null)
return this;

MulticastDelegate combined, orig, clone;
MulticastDelegate other = (MulticastDelegate) follow;

if (this.GetType() != follow.GetType ())
throw new ArgumentException (Locale.GetText ("Incompatible Delegate Types. First is {0} second is {1}.", this.GetType ().FullName, follow.GetType ().FullName));
MulticastDelegate ret = AllocDelegateLike_internal (this);

combined = (MulticastDelegate)follow.Clone ();
combined.SetMulticastInvoke ();
if (delegates == null && other.delegates == null) {
ret.delegates = new Delegate [2] { this, other };
} else if (delegates == null) {
ret.delegates = new Delegate [1 + other.delegates.Length];

for (clone = combined, orig = ((MulticastDelegate)follow).prev; orig != null; orig = orig.prev) {

clone.prev = (MulticastDelegate)orig.Clone ();
clone = clone.prev;
}

clone.SetMulticastInvoke ();
clone.prev = (MulticastDelegate)this.Clone ();
ret.delegates [0] = this;
Array.Copy (other.delegates, 0, ret.delegates, 1, other.delegates.Length);
} else if (other.delegates == null) {
ret.delegates = new Delegate [delegates.Length + 1];

for (clone = clone.prev, orig = this.prev; orig != null; orig = orig.prev) {
Array.Copy (delegates, 0, ret.delegates, 0, delegates.Length);
ret.delegates [ret.delegates.Length - 1] = other;
} else {
ret.delegates = new Delegate [delegates.Length + other.delegates.Length];

clone.prev = (MulticastDelegate)orig.Clone ();
clone = clone.prev;
Array.Copy (delegates, 0, ret.delegates, 0, delegates.Length);
Array.Copy (other.delegates, 0, ret.delegates, delegates.Length, other.delegates.Length);
}

return combined;
return ret;
}

private bool BaseEquals (MulticastDelegate value)
protected sealed override Delegate RemoveImpl (Delegate value)
{
return base.Equals (value);
}
if (value == null)
return this;

/*
* Perform a slightly crippled version of
* Knuth-Pratt-Morris over MulticastDelegate chains.
* Border values are set as pointers in kpm_next;
* Generally, KPM border arrays are length n+1 for
* strings of n. This one works with length n at the
* expense of a few additional comparisions.
*/
private static MulticastDelegate KPM (MulticastDelegate needle, MulticastDelegate haystack,
out MulticastDelegate tail)
{
MulticastDelegate nx, hx;

// preprocess
hx = needle;
nx = needle.kpm_next = null;
do {
while ((nx != null) && (!nx.BaseEquals (hx)))
nx = nx.kpm_next;

hx = hx.prev;
if (hx == null)
break;

nx = nx == null ? needle : nx.prev;
if (hx.BaseEquals (nx))
hx.kpm_next = nx.kpm_next;
else
hx.kpm_next = nx;

} while (true);

// match
MulticastDelegate match = haystack;
nx = needle;
hx = haystack;
do {
while (nx != null && !nx.BaseEquals (hx)) {
nx = nx.kpm_next;
match = match.prev;
MulticastDelegate other = (MulticastDelegate) value;

if (delegates == null && other.delegates == null) {
/* if they are not equal and the current one is not
* a multicastdelegate then we cannot delete it */
return this.Equals (other) ? null : this;
} else if (delegates == null) {
foreach (var d in other.delegates) {
if (this.Equals (d))
return null;
}
return this;
} else if (other.delegates == null) {
int idx = Array.LastIndexOf (delegates, other);
if (idx == -1)
return this;

if (delegates.Length <= 1) {
/* delegates.Length should never be equal or
* lower than 1, it should be 2 or greater */
throw new InvalidOperationException ();
}

nx = nx == null ? needle : nx.prev;
if (nx == null) {
// bingo
tail = hx.prev;
return match;
if (delegates.Length == 2)
return delegates [idx == 0 ? 1 : 0];

MulticastDelegate ret = AllocDelegateLike_internal (this);
ret.delegates = new Delegate [delegates.Length - 1];

Array.Copy (delegates, ret.delegates, idx);
Array.Copy (delegates, idx + 1, ret.delegates, idx, delegates.Length - idx - 1);

return ret;
} else {
/* wild case : remove MulticastDelegate from MulticastDelegate
* complexity is O(m * n), with n the number of elements in
* this.delegates and m the number of elements in other.delegates */
MulticastDelegate ret = AllocDelegateLike_internal (this);
ret.delegates = new Delegate [delegates.Length];

/* we should use a set with O(1) lookup complexity
* but HashSet is implemented in System.Core.dll */
List<Delegate> other_delegates = new List<Delegate> ();
for (int i = 0; i < other.delegates.Length; ++i)
other_delegates.Add (other.delegates [i]);

int idx = delegates.Length;

/* we need to remove elements from the end to the beginning, as
* the addition and removal of delegates behaves like a stack */
for (int i = delegates.Length - 1; i >= 0; --i) {
/* if delegates[i] is not in other_delegates,
* then we can safely add it to ret.delegates
* otherwise we remove it from other_delegates */
if (!other_delegates.Remove (delegates [i]))
ret.delegates [--idx] = delegates [i];
}

hx = hx.prev;
} while (hx != null);
/* the elements are at the end of the array, we
* need to move them back to the beginning of it */
int count = delegates.Length - idx;
Array.Copy (ret.delegates, idx, ret.delegates, 0, count);

tail = null;
return null;
}
if (count == 0)
return null;

protected sealed override Delegate RemoveImpl (Delegate value)
{
if (value == null)
return this;
if (count == 1)
return ret.delegates [0];

// match this with value
MulticastDelegate head, tail;
head = KPM ((MulticastDelegate)value, this, out tail);
if (head == null)
return this;
if (count != delegates.Length)
Array.Resize (ref ret.delegates, count);

// duplicate chain without head..tail
MulticastDelegate prev = null, retval = null, orig;
for (orig = this; (object)orig != (object)head; orig = orig.prev) {
MulticastDelegate clone = (MulticastDelegate)orig.Clone ();
if (prev != null)
prev.prev = clone;
else
retval = clone;
prev = clone;
return ret;
}
for (orig = tail; (object)orig != null; orig = orig.prev) {
MulticastDelegate clone = (MulticastDelegate)orig.Clone ();
if (prev != null)
prev.prev = clone;
else
retval = clone;
prev = clone;
}
if (prev != null)
prev.prev = null;

return retval;
}

public static bool operator == (MulticastDelegate d1, MulticastDelegate d2)
{
if (d1 == null)
return d2 == null;
return d2 == null;

return d1.Equals (d2);
}

public static bool operator != (MulticastDelegate d1, MulticastDelegate d2)
{
if (d1 == null)
return d2 != null;

return !d1.Equals (d2);
}
}
Expand Down
Loading

0 comments on commit 611a43e

Please sign in to comment.