[Mono-dev] IMT patch

[Rodrigo Kumpera]

Hi Massimiliano.

Do you have any measurements of the collision rates the hash function
generates over corlib? This should provide enough data to verify the hash
quality. If collision is too bad, you could store the IMT slot in MonoMethod
and do slot distribution when the interface is loaded.

I've got some points about the IMT thunks:

You are using indirect absolute jmps over the vtable pointer, which is
slower than relative jmps. The code could check if the target method is
already compiled and issue a relative jump directly. The compile trampolines
could patch the thunks, but this could end been really tricky.

The thunks will generate too much relocation in AOT'ed code, one for each
collision, changing it to one for each thunk should improve that, just load
the vtable at the beginning in a register. By the way, AOT'ed code could use
a lot of relative jumps, so this point might be mild if relative jumps are
used.

How fast is the hash function? Shouldn't the IMT slot be cached in
MonoMethod?


The patch looks great, congrats!


Rodrigo



On 5/31/07, Massimiliano Mantione <massi at ximian.com> wrote:
>
>
> Here is the IMT patch for an initial review.
> See here for an explanation of what IMT is:
> http://primates.ximian.com/~massi/blog/archive/2007/May-10.html
>
> Initially it looked "mildly unstable": when rebuilding the whole Mono
> I got a couple of NRE in the mcs symbol writer while compiling some
> library; just typing make again worked, so the problem was very hard
> to reproduce.
> However, now I updated my tree, and I just cannot reproduce it anymore.
> I fully rebuilt it three times, running the tests... no crashes.
> The IMT size is set to a ridiculous 4 to get lots of collisions and
> stress the thunks.
>
> TODO:
> - Cleanup the code (mostly remove print statements).
>   BTW, do you think some of the print statements can stay there anyway?
> - Move "mono_convert_imt_slot_to_vtable_slot" elsewhere (mini.c?).
> - Does setting "MonoClass.imt_collisions_bitmap" need locking?
>   If yes, it should be the loader lock, but does acquiring it with the
>   domain lock held (we are building the MonoVTable) risk a deadlock?
> - Add cumulative size of thunks to mono stats.
> - Also add general "performance" of the hash function to the stats?
>   I mean, collision ratio and things like that...
> - Measure memory savings and call slowdown.
> - Tune IMT size and hash function (even if IMO the hash is already OK).
> - Maybe rewrite the thunks to do binary search instead of linear.
> - Port to all the other architectures.
> - Remove the "non IMT" code paths.
>
> Do you think this should be committed before the other archs are ported,
> or should we wait for the port to be complete and commit without the
> current code path?
>
> Ciao,
>   Massi
>
>
> _______________________________________________
> Mono-devel-list mailing list
> Mono-devel-list at lists.ximian.com
> http://lists.ximian.com/mailman/listinfo/mono-devel-list
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.ximian.com/pipermail/mono-devel-list/attachments/20070531/95e1fc26/attachment.html