[Mono-bugs] [Bug 27796][Nor] Changed - patch to speed to mono_class_vtable

[bugzilla-daemon@rocky.ximian.com]

Please do not reply to this email- if you want to comment on the bug, go to the
URL shown below and enter your comments there.

Changed by lupus@ximian.com.

http://bugzilla.ximian.com/show_bug.cgi?id=27796

--- shadow/27796	Mon Jul 15 16:32:39 2002
+++ shadow/27796.tmp.13940	Tue Jul 16 04:19:36 2002
@@ -94,6 +94,42 @@
 per-domain one... Would a global lock work here?
 
 ------- Additional Comments From dick@ximian.com  2002-07-15 16:32 -------
 Re critical regions, they are already just straight calls to
 pthread_mutex_lock().  It might be worth experimenting with inlining
 the wrapper function, though.
+
+------- Additional Comments From lupus@ximian.com  2002-07-16 04:19 -------
+Dick: I wrote a test: using a critical region instead of a pthread
+mutex is 15% slower. We could certainly try to reduce the overhead,
+but you're right, it doesn't account for the extra time here.
+
+Zoltan: as you wrote in the first report, the problem is the lock:
+there is no contention, but it takes a lot of time to acquire/release
+it. I used a variant of your patch that takes and releases the lock
+even if it uses the cache and the total time was the same as without
+the cache. In my tests the hash lookup takes < 1/3 of the time of the
+lock.
+My proposal is to store in the class just the cached_vtable pointer:
+there is still a race here: two threads may set the cached_vtable
+roughtly at the same time:
+
+Domain1             Domain2            Domain1 (another thread)
+if c->c_vtable      if c->c_vtable     ...
+set c->c_vtable     ....               c->c_vtable->domain == current
+...                 set c->c_vtable    use c->c_vtable from domain2
+
+This race can be avoided if the cache check is done on a copy:
+
+MonoVTable *cached = class->cached_vtable;
+if (cached->class == class && cached->domain == domain)
+    return cached;
+
+...
+Inside the domain lock we just have:
+if (!class->cached_vtable)
+     class->cached_vtable = vtable;
+
+How does that sound? It should be race-free and avoid the lock at the
+same time.
+
+