Re: [Mono-dev] ARM/NativeClient port
On 01/30/13 Nikolay Igotti wrote: 1. all code/data access has to be in lower 1G range of address space 2. all branch targets have to be 16 bytes (bundle) aligned, unless it's direct branch to the instruction which need no masking (see 3.) 3. code with register arguments (loads, stores, branches) must explicitly enforce 1. and 2. by masking upper and lower bits (by bic reg, reg, #0xc00f) 4. all code must be valid instructions, not matter if reachable or not 5. the only exception from 4 is 16 bytes data bundles starting with UNDEF instruction (0xE125BE70), 12 remaining bytes could be used for anything 6. no executable code could be easily modified in runtime, unless in data bundle, or immediate argument of MOVT (A1), MOVW (A2), ORR/MOV/MVN Even in this case - NaCl runtime call is needed for modifications to take effect. 7. No direct PC manipulations allowed (mov, add to PC), it's allowed to be used like x86 PC register (modulo PC-relative loads). 8. All bl/blx must be bundle-end aligned and LR is masked before return, as everything else in 3. Can you define precisely what a bundle is in this context? 16 bytes? Does it need to be aligned? Do the data bundles need alignment, too? 9. Register R9 is used as TLS base, and could only be accessed as ldr rd, [r9] and ldr rd [r9, #4]. Most troublesome part for porting is using of patchable inline constants in trampolines. Our idea is to emit per-method (or per class?) jump table somewhere in .data, which contains list of all relocations, and use some register to point to this table. So for example, trampoline like this: ldr ip, [pc, #0] b skip .word target skip: mov lr, pc mov pc, ip would become (if r10 is used as jump table base register): .align 4 # for NaCl only ldr ip, [r10, #32] # unique (per-method or class) index for every callsite nop # for NaCl only, to have bl at bundle end bic r10, r10, #0xc00f # for NaCl only bl ip # or blx r10 could point somewhere in method metadata, where its relocation table is stored. So our question is if someone sees problem with such approach, or could suggest better alternative. Also advises which register could be used as the jump table base, and where to store such a table (maybe patch info?) are very welcome. [...] If there will be no strong objections, we plan to implement such a solution under configure/compilation flag for both NaCl and generic ARM port, and ask Mono maintainers to commit it. The use of an extra register makes this unsuitable for the genric ARM port, IMHO. Can't you combine a data bundle with up to three trampolines which can easily access the 12 bytes in the data bundle with pc-relative addressing? IE, assuming a bundle is 16 bytes, you allocate 64 at a time and the layout is: [data bundle: undef, data1, data2, data3] [tramp1: ldr ip [pc, data1]; nop; bic...; blx ip] [tramp2: ldr ip [pc, data2]; nop; bic...; blx ip] [tramp3: ldr ip [pc, data3]; nop; bic...; blx ip] and then you hand out the individual trampolines from this chunk until full. lupus ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] profiler in master
-- - lu...@debian.org debian/rules lu...@ximian.com Monkeys do it better ---BeginMessage--- On 12/10/10 Duane Wandless wrote: I'm trying out the profiler on OSX against trunk. I'm not getting any useful info. Do I have to build with special flags? Is there something else I'm missing. Thanks, OSX as different default shared lib symbol search rules. Try this patch: diff --git a/mono/profiler/Makefile.am b/mono/profiler/Makefile.am index 26f2e1d..4aa72df 100644 --- a/mono/profiler/Makefile.am +++ b/mono/profiler/Makefile.am @@ -36,6 +36,10 @@ libmono_profiler_iomap_la_LIBADD = $(top_builddir)/mono/mini/libmono-$(API_VER). libmono_profiler_log_la_SOURCES = proflog.c libmono_profiler_log_la_LIBADD = $(top_builddir)/mono/mini/libmono-$(API_VER).la $(Z_LIBS) +if PLATFORM_DARWIN +libmono_profiler_log_la_LDFLAGS = -undefined suppress -flat_namespace +endif + mprof_report_SOURCES = decode.c mprof_report_LDADD = $(Z_LIBS) lupus -- - lu...@debian.org debian/rules lu...@ximian.com Monkeys do it better ---End Message--- ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] profiler in master
On 12/10/10 Duane Wandless wrote: That has the same result as before. Could you post the output of the following commands in mono/profiler? make clean make V=1 make install mono-sgen --version mono-sgen --profile=log:heapshot,report app.exe Thanks lupus -- - lu...@debian.org debian/rules lu...@ximian.com Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] trunk build error in FreeBSD
On 11/30/10 KISHIMOTO, Makoto wrote: I saw https://github.com/mono/mono/commit/30cba27527d5076896bf6bead5b9aa9c44d32d6f , IMHO, use of symbol ELF_CLASS is better than SIZEOF_VOID_P . I'm not sure that there is a standard for the ELF_CLASS definition, so I did the safe thing for now. If and when a system shows up where using SIZEOF_VOID_P won't work we'll fix it. lupus -- - lu...@debian.org debian/rules lu...@ximian.com Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] trunk build error in FreeBSD
On 11/30/10 KISHIMOTO, Makoto wrote: In my FreeBSD Box, proflog.c compile failed. Could you try current trunk? I committed a change that should fix the issue. lupus -- - lu...@debian.org debian/rules lu...@ximian.com Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] Build problem caused by sched_getcpu function
On 11/11/10 J.P. wrote: I try to build the source from git. but I have a problem below this. This problem is on x64 and x86 CentOS. I think it maybe caused by recently fix about decode and log profiler. Isn't it? Please fix the problem. I wanna use fixed mono version that solved thread issue. but I can't compile. This should be fixed in git. lupus -- - lu...@debian.org debian/rules lu...@ximian.com Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] Building error on profiler.c. maybe gc_init not work.
On 11/03/10 J.P. wrote: I have a error while compile the mono of git source. I think it caused by mono_gc_base_init function. Please correct this problem. I have configured normally on my x64 linux machine. You likely need to run autogen.sh in the top dir again. Using autogen is required when building from git. lupus -- - lu...@debian.org debian/rules lu...@ximian.com Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] Current status of global regalloc (regalloc2.c)
On 09/14/10 Sergei Dyshel wrote: So I have too stay with 'mini' :-). I've made some tweaks to it but still its division of vregs to local and global sometimes drastically decreases code performance by making a lot of redundant moves. So I started to think that may be global allocator is the only 'cure'. How many work, do you think, will it take to make it work on PowerPC for simple scenarios I described? FYI, we plan to start working on the global register allocator as soon as the work on the new garbage collector settles (that is in a few months, when mono 3.0 is nearing the release). Of course any help on it in the meantime is appreciated. lupus -- - lu...@debian.org debian/rules lu...@ximian.com Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] [PATCH] Make mono_dl_register_library into a fallback
On 07/13/10 Miguel de Icaza wrote: typedef void* (*MonoDlFallbackLoad) (const char *name, int flags, char **err, void *user_data); typedef void* (*MonoDlFallbackSymbol) (void *handle, const char *name, char **err, void *user_data); typedef void* (*MonoDlFallbackClose) (void *handle, void *user_data); void mono_dl_register_fallback (MonoDlFallbackLoad load_func, MonoDlFallbackSymbol symbol_func, MonoDlFallbackClose close_func, void *user_data); The old interface could be easily implemented on top of this new one (though we likely could drop it as well). This implements the suggested API based on Michael's patch. I still kept the mono_dl_register as we are now using in 4 different ports and we have a bunch of different tools generating that output as well as having pointed third parties to this API. As I said, if the tables are generated by tools, they could be easily changed to use the new API. As for other uses: I don't think we should promote the use of an inefficient API especially since the primary use would be in embedded systems where startup time and memory usage are important (the array is unsorted, so the search is linear, it is memory inefficient since the string pointer array uses more space than needed and on some systems it causes runtime relocations). Sure, if we're dealing with 10 entry points it's a non issue, but I know that in the end this will be used for huge APIs like OpenGL etc (the fact you're talking about tools generating the data hints that there are going to be lots of entries). Index: mono-dl.c === --- mono-dl.c (revision 160304) +++ mono-dl.c (working copy) +MonoDlMapping * +dl_mapping_open (const char *file); + +void * +dl_mapping_symbol (void *handle, const char *symbol); + +char * +dl_mapping_error (void); + These declarations do not belong here and should be removed, the rest looks fine. lupus -- - lu...@debian.org debian/rules lu...@ximian.com Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] [PATCH] Make mono_dl_register_library into a fallback
On 07/02/10 Michael Hutchinson wrote: This interface is not suitable as a fallback mechanism, it would be too cumbersome to use and it's probably not ideal even for its intended purpose. What about a callback registration system instead? The intended purpose, AFAIK, is to expose statically linked libraries to P/Invoke. That's what I'm using it for, and it's very straightforward to use this way. I have a trivial C# tool that reflects over the library with the P/Invokes and dumps the mappings to source files. The interface was just a quick hack from Miguel, in fact is has never been exposed as public API as you found out (it was only possible to use it in a custom mono build for consoles). If you have a tool generating the table now, changing it to generate the function needed by the new interface should be trivial, anyway. My patch isn't intended to provide a generic dynamic linker fallback. But it does change the code to fallback and you want to publish the API, so then we're stuck with an unsuitable API for a new feature. It's meant to provide an easy way to P/Invoke statically linked libraries for all embedders, not just those on platforms without dynamic linkers. If you have a working dynamic linker using DllImport (__Internal) is the way to go. That's certainly more flexible, but I'm not convinced it's necessary at this time without concrete use cases. If we're going to include the If you don't have concrete cases, why are you expanding the functionality of the quick and dirty (and non-public) API? old interface anyway - else we make embedders responsible for reimplementing its functionality - then why not go this path for now, and reimplement it on top of the callbacks later? Because it means exposing an API and attaching to it functionality it's not suited for. The tricks you mention could be much more useful if callbacks could intercept lookups for individual symbols, rather than acting as a fallback for handling whole libraries. But this would require much more substantial changes to the dynamic linker code. A separate API entry point could be added for that. To recap: *) that API was never public. *) I'm not comfortable with publishing that API and supporting it with the fallback functionality. *) if we need a fallback mechanism something like my proposed interface is much better and I think is suitable for publishing (maybe with an additional flags value to give a priority vs the existing lookups). lupus -- - lu...@debian.org debian/rules lu...@ximian.com Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] [PATCH] Make mono_dl_register_library into a fallback
On 07/01/10 Michael Hutchinson wrote: The mono_dl_register_library function can currently be used to register P/Invoke mappings for platforms that do not have a dynamic linker. The attached patch makes it also function as a fallback when the system's dynamic linker cannot resolve the library, so that it is always available to embedders. I'll also need to restore mono/utils/mono-embed.h to the public headers in the autotools build (but this patch was created using MSVC on Windows). OK to commit to trunk? This interface is not suitable as a fallback mechanism, it would be too cumbersome to use and it's probably not ideal even for its intended purpose. What about a callback registration system instead? It is more flexible as a fallback since it doesn't require to register upfront all the possible names and load the all functions at startup. It's likely better also for the original purpose when, for example, even if dynamic linking is not possible it is possible to lookup a symbol at runtime. It opens up the possibility of also generating the code at runtime (for redirecting some win32 p/invokes to winforms for example, or other tricks). Something along these lines: typedef void* (*MonoDlFallbackLoad) (const char *name, int flags, char **err, void *user_data); typedef void* (*MonoDlFallbackSymbol) (void *handle, const char *name, char **err, void *user_data); typedef void* (*MonoDlFallbackClose) (void *handle, void *user_data); void mono_dl_register_fallback (MonoDlFallbackLoad load_func, MonoDlFallbackSymbol symbol_func, MonoDlFallbackClose close_func, void *user_data); The old interface could be easily implemented on top of this new one (though we likely could drop it as well). lupus -- - lu...@debian.org debian/rules lu...@ximian.com Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] [PATCH] Extract mono_exception_get_message_string from mono_print_exception
On 07/01/10 Michael Hutchinson wrote: The attached patch adds a new mono_exception_get_message_string function to the Mono public API, extracted from mono_print_exception. This makes it possible for embedders easily to convert exceptions to strings without printing them and without using private API. It is already possible to use the embedding API to get the same result without using any private API, of course. Anyway, we can certainly add an helper method, but it should be more general: MonoString *mono_object_to_string (MonoObject *obj, MonoObject **exc); Note that it works for any object and that it allows the user to handle exceptions thrown from the implict method invocation. The result can be used as is or mono_string_to_utf8 () can be called on it. lupus -- - lu...@debian.org debian/rules lu...@ximian.com Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] [PATCH] altstack for osx
On 06/23/10 Geoff Norton wrote: Attached is a patch which enables sigaltstack for OSX (x86 and amd64). The configure.in check passes on amd64 and fails on x86 for some reason (the signal isn't delivered to the child ?) but the support appears to work correctly on both. --- a/mono/mini/exceptions-amd64.c +++ b/mono/mini/exceptions-amd64.c @@ -723,7 +723,6 @@ handle_signal_exception (gpointer obj, gboolean test_only) gboolean mono_arch_handle_exception (void *sigctx, gpointer obj, gboolean test_only) { -#if defined(MONO_ARCH_USE_SIGACTION) defined(UCONTEXT_GREGS) /* * Handling the exception in the signal handler is problematic, since the original * signal is disabled, and we could run arbitrary code though the debugger. So @@ -748,29 +747,8 @@ mono_arch_handle_exception (void *sigctx, gpointer obj, gboolean test_only) UCONTEXT_REG_RIP (sigctx) = (guint64)handle_signal_exception; return TRUE; -#else - MonoContext mctx; - - mono_arch_sigctx_to_monoctx (sigctx, mctx); - - if (mono_debugger_handle_exception (mctx, (MonoObject *)obj)) - return TRUE; - - mono_handle_exception (mctx, obj, MONO_CONTEXT_GET_IP (mctx), test_only); - - mono_arch_monoctx_to_sigctx (mctx, sigctx); - - return TRUE; -#endif I would leave the alternate code in, it may be needed for other ports. diff --git a/mono/mini/mini-exceptions.c b/mono/mini/mini-exceptions.c index 5a73ac9..3881b7e 100644 --- a/mono/mini/mini-exceptions.c +++ b/mono/mini/mini-exceptions.c @@ -1594,13 +1594,23 @@ mono_handle_exception (MonoContext *ctx, gpointer obj, gpointer original_ip, gbo #error Can't use sigaltstack without sigaction #endif +/* + * Handling a ovf on osx requires a lot more stack space than on linux because + * we might end up in CoreFoundation getting the locale You might want to adjust also the minimum thread stack size to account for this (adding a check in metadata/threads.c). Bonus points for also fixing the race with the mono_threads_set_default_stacksize() calls in threadpool.c (maybe adding a stack size argument to mono_thread_create_internal() in a separate patch). @@ -1682,13 +1699,19 @@ static gboolean try_restore_stack_protection (MonoJitTlsData *jit_tls, int extra_bytes) { gint32 unprotect_size = jit_tls-stack_ovf_guard_size; + /* OSX requires a ton of stack space when we're handling an overflow exception, because + * we might call into corefoundation to get the locale and other such things + * as such we'll just give the entire page back to the system + */ In this case some of the more complex stack overflow scenarios won't be handled. We unprotect a page at a time to have a chance of handling an additional overflow during finally or catch handlers. Anyway, the patch is ok to commit. Thanks! lupus -- - lu...@debian.org debian/rules lu...@ximian.com Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] [PATCH]Add ephemeron support for sgen
On 05/04/10 Rodrigo Kumpera wrote: Once this patch is accepted I'll move into finishing the managed side of it. Right now CWT is implemented doing linear search, but the idea is to switch to open addressing. To do that, the only change required to sgen is to store a tombstone instead of NULL when clearing unreachable keys. The tombstone object needs to be known to both the managed and unmanaged code: it may be a good idea to register it with the GC in the same icall that registers the array (it would be per-domain, but storing it as an additional field in EphemeronLinkNode should be fine). --- a/mono/metadata/sgen-gc.c +++ b/mono/metadata/sgen-gc.c +/* LOCKING: requires that the GC lock is held */ +static void +clear_unreachable_ephemerons (CopyOrMarkObjectFunc copy_func, char *start, char *end) +{ + int was_in_nursery, was_promoted; + EphemeronLinkNode *current = ephemeron_list, *prev = NULL; + MonoArray *array; + mword **ptr; + uintptr_t i, length; + + while (current) { + char *object = current-array; + + if (!object_is_reachable (object, start, end)) { + EphemeronLinkNode *tmp = current; + + DEBUG (5, fprintf (gc_debug_file, Dead Ephemeron array at %p\n, object)); + + if (prev) + prev-next = current-next; + else + ephemeron_list = current-next; + + current = current-next; + free_internal_mem (tmp, INTERNAL_MEM_EPHEMERON_LINK); + + continue; + } + + was_in_nursery = ptr_in_nursery (object); + copy_func ((void**)object); + current-array = object; + + /*The array was promoted. Promote all elements of the array and add global remsets from the ones left behind in the nursery. + */ + was_promoted = was_in_nursery !ptr_in_nursery (object); + + DEBUG (5, fprintf (gc_debug_file, Clearing unreachable entries for ephemeron array at %p\n, object)); + + array = (MonoArray*)object; + ptr = (mword**)mono_array_addr_with_size (array, sizeof (mword) * 2, 0); + length = mono_array_length (array); + + for (i = 0; i length; ++i) { + char *key = (char*)ptr [0]; + char *value = (char*)ptr [1]; Using a: typedef struct { void *key; void *value; } Ephemeron; instead of using ptr[0]/ptr[1] should make the code nicer here. + + if (key) + DEBUG (5, fprintf (gc_debug_file, [%d] key %p (%s) value %p (%s)\n, i, + key, key object_is_reachable (key, start, end) ? reachable : unreachable, + value, value object_is_reachable (value, start, end) ? reachable : unreachable)); + + if (key !object_is_reachable (key, start, end)) { + DEBUG (5, fprintf (gc_debug_file, \tclearing index %d\n, i)); + ptr [0] = NULL; + ptr [1] = NULL; + key = NULL; + } + + if (key was_promoted) { + if (ptr_in_nursery (key)) {/*key was not promoted*/ + DEBUG (5, fprintf (gc_debug_file, \tAdded remset to key %p\n, key)); + add_to_global_remset (ptr, FALSE); + } + if (ptr_in_nursery (value)) {/*value was not promoted*/ + DEBUG (5, fprintf (gc_debug_file, \tAdded remset to value %p\n, value)); + add_to_global_remset (ptr + 1, FALSE); + } + } + ptr += 2; +} I would move the ptr += 2 together with the ++i (or use a single loop indexer) and use the appropriate continue statements instead of the multiple checks for a NULL key (it will also simplify the code when tombstone support will be added). +/* LOCKING: requires that the GC lock is held */ +static int +mark_ephemerons_in_range (CopyOrMarkObjectFunc copy_func, char *start, char *end) +{ + int nothing_marked = 1; + EphemeronLinkNode *current = ephemeron_list; + MonoArray *array; + mword **ptr; + uintptr_t i, length; + + while (current) { A for loop here is likely more appropriate, instead of the multiple current = current-next statements. + char *object = current-array, *old; + DEBUG (5, fprintf (gc_debug_file, Ephemeron array at %p\n, object)); + /*We ignore arrays in old gend during minor collections
Re: [Mono-dev] [PATCH] Android Support [3/4]
On 04/19/10 Jonathan Pryor wrote: Index: mono/io-layer/collection.c === --- mono/io-layer/collection.c(revision 155735) +++ mono/io-layer/collection.c(working copy) @@ -58,7 +58,10 @@ ret = pthread_attr_init (attr); g_assert (ret == 0); -#ifdef HAVE_PTHREAD_ATTR_SETSTACKSIZE +/* Android implements pthread_attr_setstacksize(), but errors out when using + * PTHREAD_STACK_MIN: http://code.google.com/p/android/issues/detail?id=7808 + */ +#if defined(HAVE_PTHREAD_ATTR_SETSTACKSIZE) !defined(PLATFORM_ANDROID) if (set_stacksize == 0) { #if defined(__FreeBSD__) || defined(__NetBSD__) || defined(__OpenBSD__) ret = pthread_attr_setstacksize (attr, 65536); Instead of adding more checks there, just change the code to do: ret = pthread_attr_setstacksize (attr, MAX (65536, PTHREAD_STACK_MIN)); Index: mono/io-layer/mono-mutex.c === --- mono/io-layer/mono-mutex.c(revision 155735) +++ mono/io-layer/mono-mutex.c(working copy) @@ -22,11 +22,24 @@ #ifndef HAVE_PTHREAD_MUTEX_TIMEDLOCK +/* Android does not implement pthread_mutex_timedlock(), but does provide an + * unusual declaration: http://code.google.com/p/android/issues/detail?id=7807 + */ +#if defined(PLATFORM_ANDROID) int pthread_mutex_timedlock (pthread_mutex_t *mutex, + struct timespec *timeout); +#else +int pthread_mutex_timedlock (pthread_mutex_t *mutex, const struct timespec *timeout); +#endif Just do this at the start: #ifdef PLATFORM_ANDROID #define CONST_NEEDED const #else #define CONST_NEEDED const #endif And then insert CONST_NEEDED where appropriate instead of the ugly duplication and ifdef mess. Index: mono/mini/exceptions-arm.c === --- mono/mini/exceptions-arm.c(revision 155735) +++ mono/mini/exceptions-arm.c(working copy) @@ -12,7 +12,11 @@ #include glib.h #include signal.h #include string.h +#if defined(PLATFORM_ANDROID) +#include asm/sigcontext.h +#else #include ucontext.h +#endif Please make sure configure has the appropriate header checks and use: #ifdef HAVE_ASM_SIGCONTEXT_H #include asm/sigcontext.h #endif #ifdef HAVE_UCONTEXT_H #include ucontext.h #endif We must use feature checks and not platform checks as much as possible. lupus -- - lu...@debian.org debian/rules lu...@ximian.com Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] [PATCH] Mono and the MOSA Project
On 04/07/10 Phil Garcia wrote: Since we are implementing everything in .NET/C# we don?t need InternalCalls to call external API bindings. In fact, the external declarations in the source code get into the way. So we have moved the InternalCalls method declarations into separate partial class files (for example, String.Internal.cs). This patch allows us to compile Mono class libraries without referencing those external methods declarations and substitute own managed versions. I don't think that shuffling around the code like that is acceptable for us. A change that could be included would be to surround the icalls with a #if check and then you compile with that define. I suggest something like NO_ICALLS. We had updated the Object class too, but it caused a single failed test so we have excluded it from here. The test failed due to a change in the order of the Object class methods. We are not sure if this is just a paranoia check (but kumpera on IRC said tons of user code can depend on it). Otherwise, all the other tests passed. We checked the new mscorlib from .NET 4.0 through ildasm and Mono doesn't follow the same method ordering for Object. Is the ordering of methods for the Object class really important for Mono? If you pointed out the name of the test we could fix it to not depend on the order if it's really a bug in the test. lupus -- - lu...@debian.org debian/rules lu...@ximian.com Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] [PATCH 02/10] config: Add support for Haiku
On 03/30/10 Andreas Färber wrote: --- a/mono/metadata/mono-config.c +++ b/mono/metadata/mono-config.c @@ -39,6 +39,8 @@ #define CONFIG_OS aix #elif defined(__hpux) #define CONFIG_OS hpux +#elif defined(__HAIKU__) +#define CONFIG_OS haiku #else This is ok to commit. lupus -- - lu...@debian.org debian/rules lu...@ximian.com Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] [PATCH v2 07/10] console-unix: Compile fix for Haiku
On 03/30/10 Andreas Färber wrote: --- a/mono/metadata/console-unix.c +++ b/mono/metadata/console-unix.c @@ -43,7 +43,8 @@ #ifdef HAVE_SYS_FILIO_H #include sys/filio.h #endif -#ifndef TIOCGWINSZ +/* Haiku has FIONREAD in sys/ioctl.h */ +#if !defined(TIOCGWINSZ) || !defined(FIONREAD) #ifdef HAVE_SYS_IOCTL_H #include sys/ioctl.h #endif I committed a better fix in svn. lupus -- - lu...@debian.org debian/rules lu...@ximian.com Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] [PATCH] Implement guarded finally blocks
On 03/24/10 Rodrigo Kumpera wrote: --- a/mono/mini/mini-exceptions.c +++ b/mono/mini/mini-exceptions.c [...] +static gboolean +find_last_handler_block (MonoDomain *domain, MonoContext *ctx, MonoJitInfo *ji, gpointer data) +{ + int i; + gpointer ip; + FindHandlerBlockData *pdata = data; + + if (ji-method-wrapper_type) + return FALSE; + + ip = MONO_CONTEXT_GET_IP (ctx); + + for (i = 0; i ji-num_clauses; ++i) { + MonoJitExceptionInfo *ei = ji-clauses + i; + if (ei-flags != MONO_EXCEPTION_CLAUSE_FINALLY) + continue; + /*If ip points to the first instruction it means the handler block didn't start + so we can leave its execution to the EH machinery*/ + if (ei-handler_start ip ip ei-data.handler_end) { + pdata-ji = ji; + pdata-ei = ei; + pdata-ctx = *ctx; + break; Shouldn't the stack walk be interrupted here once we found the finally clause? --- a/mono/mini/mini-posix.c +++ b/mono/mini/mini-posix.c @@ -189,7 +190,15 @@ SIG_HANDLER_SIGNATURE (sigusr1_signal_handler) if (mono_debugger_agent_thread_interrupt (ctx, ji)) return; - + + /* We can't do handler block checking from metadata since it requires doing + * a stack walk with context. + */ + mono_arch_sigctx_to_monoctx (ctx, mctx); + if (mono_install_handler_block_guard (thread, mctx)) { + return; + } +# Leftover here. --- a/mono/mini/mini-x86.c +++ b/mono/mini/mini-x86.c @@ -5809,6 +5809,70 @@ mono_arch_decompose_long_opts (MonoCompile *cfg, MonoInst *long_ins) #endif /* MONO_ARCH_SIMD_INTRINSICS */ } +/*MONO_ARCH_HAVE_HANDLER_BLOCK_GUARD*/ +gpointer +mono_arch_install_handler_block_guard (MonoJitInfo *ji, MonoContext *ctx, gpointer new_value) +{ + int i; + int offset; + MonoJitExceptionInfo *clause = NULL; + gpointer ip, *sp, old_value; + char *bp; + const unsigned char *handler; + + ip = MONO_CONTEXT_GET_IP (ctx); + + for (i = 0; i ji-num_clauses; ++i) { + clause = ji-clauses [i]; + if (clause-flags != MONO_EXCEPTION_CLAUSE_FINALLY) + continue; + if (clause-handler_start ip clause-data.handler_end ip) + break; + } + + /*no matching finally */ + if (i == ji-num_clauses) + return NULL; + + /*If we stopped on the instruction right before the try, we haven't actually started executing it*/ + if (ip == clause-handler_start) + return NULL; + Up to here there is nothing that is arch-specific. All this code should be moved to the caller. The rest of the changes look fine to me. You might need to disable this code with aot, since I'm not sure the complete clause data is saved in that case for this to work: did you test it already? Or the aot changes would need to be implemented, anyway Thanks! lupus -- - lu...@debian.org debian/rules lu...@ximian.com Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] [PATCH] Two patches to make SGen work on Darwin/x86
On 03/24/10 Mark Probst wrote: Here's an updated patch that fixes this problem, both for mono-ehash as well as for mono-hash. --- a/mono/metadata/boehm-gc.c +++ b/mono/metadata/boehm-gc.c @@ -932,5 +932,11 @@ mono_gc_get_write_barrier (void) #endif +void +mono_gc_invoke_without_moving (void (*func) (void*), void *data) +{ + func (data); +} + Please introduce a typedef for the function pointer type. I think the naming here is misleading, since the issue is not just about moving the objects, but also about them being collected. Moreover, I think the feature should be actually implemented in Boehm since it's provided by the library and it can be useful for other cases, too. --- a/mono/metadata/sgen-gc.c +++ b/mono/metadata/sgen-gc.c @@ -597,6 +597,7 @@ safe_object_get_size (MonoObject* o) * ## */ static LOCK_DECLARE (gc_mutex); +static LOCK_DECLARE (interruption_mutex); It may be better to put the declaration of the mutexes far from each other so that there are more chances they'll end up in separate cachelines and hence provide an effective advantage wrt using a single mutex. static int gc_disabled = 0; static int num_minor_gcs = 0; static int num_major_gcs = 0; @@ -3337,6 +3338,8 @@ collect_nursery (size_t requested_size) TV_DECLARE (atv); TV_DECLARE (btv); + LOCK_INTERRUPTION; You need to put the lock acquisition at the start of stop_world(), here it can cause a deadlock, since a thread may be stopped while holding interruption_mutex and this call here will wait indefinitely. Also, to prepare for the future changes, introduce two functions: void acquire_gc_locks (void) and void release_gc_locks (void) and call them in stop_world()/restart(). The rest looks ok to me. lupus -- - lu...@debian.org debian/rules lu...@ximian.com Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] [PATCH] Two patches to make SGen work on Darwin/x86
On 03/24/10 Mark Probst wrote: The first patch make mono-ehash SGen-aware, the second implements CEE_MONO_TLS on Darwin/x86. --- a/mono/utils/mono-ehash.c +++ b/mono/utils/mono-ehash.c [...] +#ifdef HAVE_SGEN_GC + if (type 0 || type MONO_HASH_KEY_VALUE_GC) + g_error (wrong type for gc hashtable); + /* + * We use a user defined marking function to avoid having to register a GC root for + * each hash node. + */ + if (!hash_descr) + hash_descr = mono_gc_make_root_descr_user (mono_g_hash_mark); + if (type != MONO_HASH_CONSERVATIVE_GC) + mono_gc_register_root_wbarrier ((char*)hash, sizeof (MonoGHashTable), hash_descr); +#endif Uhm, when Zoltan changed the code in mono-hash.c to use this mono_g_hash_mark root descriptor I had the gut feeling it wasn't correct. Seeing the change again helped me focus on the bug this introduces. Look at do_rehash in mono-ehash.c (mono-hash.c has a similar issue) and imagine what happens when a GC occours just after these two lines: table = hash-table; hash-table = mg_new0 (Slot *, hash-table_size); All the entries are reachable from table, but the GC can only see from hash-table, which contains no hash table entries. This means the objects me be deallocated or moved but their pointer in the hash won't be updated, leading to crashes. One solution is to introduce a function similar to GC_call_with_alloc_lock() in the Boehm GC, so that the unsafe table manipulation can be done without risk of interruption from the GC. Another one is to expose the critical region enter/exit code from sgen-gc.c. In both cases we must take care that no GC allocation can be done inside the protected regions (the GC lock is not recursive and it must stay that way). The TLS change looks fine to me. Thanks! lupus -- - lu...@debian.org debian/rules lu...@ximian.com Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] [RFC] Handle lack of SA_SIGINFO
On 03/22/10 Andreas Färber wrote: SA_SIGINFO and sigaction are part of the optional POSIX XSI feature. In mini, there's MONO_ARCH_USE_SIGACTION but it doesn't cover everything [...] +#ifdef SA_SIGINFO + if (save_sigcont.sa_flags SA_SIGINFO) { if (save_sigcont.sa_sigaction != NULL save_sigcont.sa_sigaction != (void *)SIG_DFL save_sigcont.sa_sigaction != (void *)SIG_IGN) (*save_sigcont.sa_sigaction) (signo, the_siginfo, data); + } else +#else + if (save_sigcont.sa_handler != NULL + save_sigcont.sa_handler != (void *)SIG_DFL + save_sigcont.sa_handler != (void *)SIG_IGN) + (*save_sigcont.sa_handler) (signo); +#endif } These kind of changes are quite ugly. Please introduce a cpp macro that handles this stuff, so that the code doesn't become a forest of #ifdefs. Something like: #define INVOKE_SIGHANDLER_IF_VALID (siga,signo,sinfo,data) do {\ if (((siga)-sa_flags SA_SIGINFO) (siga)-sa_sigaction != NULL ...) \ (siga)-sa_sigaction ((signo), (sinfo), (data));\ } while (0) and the equialent for the non-SA_SIGINFO case. lupus -- - lu...@debian.org debian/rules lu...@ximian.com Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] Fixing Thread:Abort handling of finally clauses
On 03/09/10 Rodrigo Kumpera wrote: One is to patch the return address of a finally clause to jump into runtime code that will handle raising the ThreadAbortException. This is very tricky because we need to store precise unwind information to be able to figure out where that value is. The advantage is that is doesn't slow down the fast path. Note that sooner or later we must implement the precise info anyway since we need it for reliable exception handling, precise GC of stack locations etc. It willl also be considerably easier once we move the x86 jit to allocate enough stack for the calls at method entry, so avoiding all the overhead of adjusting esp all the time before and after the calls (instead of push instructions we'll use store from the esp base reg). At the end there is no difference in complexity between storing a flag and storing to the return address. The other option is to change how finally clauses work so they make this easier. Zoltan mentioned that we should restore from EH context and use a variable to tell what to do next. The pseudo-code for the fast-path are something like: Currently: try_body: ... call finally jmp rest_of_function finally: ... ret rest_of_function: ... Zoltan's suggestion: try body: ... mov 0, [EBP + ?] //this is the variable that tells to resume unwinding or not jmp finally; finally: ... cmp 0, [EBP + ?] jmp_if_zero rest_of_function call resume_unwinding rest_of_function: ... Looking at the pseudo code, currently we do 3 branches (call, ret, jmp) with Zoltan's suggestion we would do only two (jmp, jz), thou one is conditional. The memory bandwidth is the same, both require one load and one store. Zoltan's suggestion should result in larger code thou. If you want to go that route, the call/jmp pair could be optimized to a push rest_of_function/jmp finally and still maintain correctness. From our irc log, you raised a few issues with this approach, first that is might cause issues with the ppc ABI. This approach is basically the same to how we handle catch clauses and the later doesn't seen to have issues. As I said on irc, finally clauses and catch clauses are fundamentally different. You also mentioned that a finally clause can be called in a different stack frame that of its original method. I fail to see how could that happen, specially if we'll be restoring to it. Consider this code: void method () { try { if (boolean) throw ex; } finally { ... } } Now, the finally clause can be reached in two different ways, I will list the stack frames involved: boolean TRUEFALSE method method runtime EH finally finally Note the difference. In both cases we need to maintain correctness wrt the ABI (as I explained for the PPC case, MIPS is similar and likely other archs follow the same pattern) and also for our own ability to handle stack walks from within the finally handler, both for exception handling and for the precise GC support. With a catch cluase, instead, you have the following: void method () { try { if (boolean) throw ex; } catch { ... } } boolean TRUEFALSE method method catch catch So the two cases are different. I do support Zoltan that this approach is the way to go, but I rather hear your opinion first. Even if it ends up been marginally slower, it has the advantage of been much much simpler, which wins us in development time and reliability. I don't think Zoltan's code is correct in this case, apart from the above issues of stack frame consistency and additional code size. There is an additional complication with finally clauses, in that a leave instruction may leave multiple nested protected blocks and all the finally clauses must be executed. I attached a sample that should print just 1, from your pseudocode above this case wouldn't be handled correctly, though I don't have an llvmono uild to try it out. AFAIK, it should also verify correctly. lupus -- - lu...@debian.org debian/rules lu...@ximian.com Monkeys do it better .assembly extern mscorlib { .ver 2:0:0:0 .publickeytoken = (B7 7A 5C 56 19 34 E0 89 ) // .z\V.4.. } .assembly 'finally' { .custom instance void class [mscorlib]System.Runtime.CompilerServices.RuntimeCompatibilityAttribute::'.ctor'() = ( 01 00 01 00 54 02 16 57 72 61 70 4E 6F 6E 45 78 // T..WrapNonEx 63 65 70 74 69 6F 6E 54 68 72 6F 77 73 01 ) // ceptionThrows. .hash algorithm 0x8004 .ver 0:0:0:0 } .module finally.exe // GUID =
Re: [Mono-dev] System.Threading.Monitor::Exit fails in latest trees
On 03/04/10 cpMon wrote: I never get a signal when System.Threading.Monitor::Exit gets called too many times. We used to raise an exception, but IIRC, the MS runtime doesn't throw it either, so we don't anymore. Further, I traced it down into the mono 2.6.1 code tree, and mono_monitor_exit is never called. The trampoline generates the code, but it's never called. Can you provide a quick fix? It seems like a glaring bug. There is no bug, on your architecture the fast path is optimized and it won't go to the unmanaged function in the runtime. lupus -- - lu...@debian.org debian/rules lu...@ximian.com Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] ir instructions.
On 02/22/10 Rodrigo Kumpera wrote: On Sat, Feb 20, 2010 at 2:07 AM, Jerry Maine - KF5ADY crashfou...@gmail.com I remember you talking about instead of having marcos and defines describe certain attributes of IR instructions, having them be defined in a single data structure like what is done for when calling LLVM. What would that entail? That would entail been able to encode more properties and relations of instructions without all the repetition that currently exist in our JIT. All the repetition can be trivially eliminated: nobody did that yet because it's mostly a waste of time at this point (and some invariants in opcode order need to be maintained right now, so changing things blindly could break code). There are 3 different problems I think deserve been solved. The first is encoding of instruction properties such as commutative or side-effect free. For this it would be a matter of adding an extra parameter to the MINI_OP macro or'ing those properties and then filling a table with this data. Right, so there is no need for the bloat of tablegen for this (not counting the build issues it would bring on). The second is encode relations between instructions. For example, op_to_op_dest_membase is only enabled on x86/amd64 and to small amount of operations. Doing this for all SIMD ops would be a a lot of repetitive work. See the attached program that will generate all the stuff you need there and more. It can be trivially enhanced to generate also the backend code or mono_op_to_op_imm() support. The last is to reduce the amount of repetition between instructions definitions, it has to be very template oriented and support customization of the result. For example, I want to define a single template for binary SIMD ops that produce reg_reg, reg_membase, reg_memindex variants, which don't have side effects and, finally, can be easily instantiated for the many vector elements. This is asking for a lot. But it would simplify the JIT a lot and make it produce better code easier. See the attached program, it does already basically all the things you outlined. All with half an hour of scripting and 60 lines of code. So, no, there is no need for tablegen in mono, adapting it to hour needs would require much more than half an hour and being so different is more likely to introduce subtle issues. Once the templates are defined, you can simply add a new SSE instruction by addig its name to the proper list. lupus -- - lu...@debian.org debian/rules lu...@ximian.com Monkeys do it better #!/usr/bin/perl -w my %templates = ( # typical sse instructions XREG XREG XREG - MEMBASE[2] = [qw (ADDPD DIVPD MULPD SUBPD MAXPD)], # typical integer instruction IREG IREG IREG - IMM[2:NONE] = [qw (IADD ISUB IMUL IDIV)], ); my @data = ( # a more complex case: we generate several instructions 'COMPARE compare NONE IREG IREG - IMM[2:NONE], MEMBASE_REG[1], MEMBASE_IMM[1 2:NONE], REG_MEMBASE[2]', ); foreach my $template (keys %templates) { foreach my $op (@{$templates{$template}}) { push @data, $op . lc($op) . $template; } } sub output { my @spec = @_; print MONO_OP(OP_$spec[0], \$spec[1]\, $spec[2], $spec[3], $spec[4])\n; } sub output_variant { my ($variant, @spec) = @_; my @orig = @spec; return unless $variant =~ /(\S+)\s*\[(.*)\]/; my $name = $1; my @regs = split (/\s/, $2); $spec [0] .= _$name; $spec [1] .= _ . lc $name; my $membase; foreach my $reg (@regs) { my ($num, $new) = split (/:/, $reg); $new = IREG unless defined $new; $spec [2 + $num] = $new; # we use the convention that the first one is the base pointer $membase = $num unless defined $membase; } output (@spec); my $orig = $spec[0]; $orig =~ s/_MEMBASE//; # to be used in the switch in functions like op_to_op_* with something like: # #define OP_2_OP_REG1_MEMBASE(a,b) case a: return b; print OP_2_OP_REG${membase}_MEMBASE(OP_$orig,OP_$spec[0])\n if defined $membase $name =~ /MEMBASE/; } sub generate { my $template = shift; my ($opcode, $variant) = split (/-/, $template); my @spec = split (/\s/, $opcode); output (@spec); return unless (defined $variant) and (length $variant); foreach my $var (split (/,/, $variant)) { # variants could go to a different output queue to # maintain instruction order currently required in the JIT output_variant ($var, @spec) } } foreach my $op (@data) { generate ($op); } ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] Getting rid of String.InternalSetLength
On 12/03/09 Mark Probst wrote: SGen, our new garbage collector, doesn't explicitly store an object's size but determines it via the object's vtable and, in the case of arrays and strings, via the length field. String.InternalSetLength changes that length field, which means that, from SGen's view, the string's size changes. That's a problem because depending on an object's size it is either stored in a regular heap section or, if it is larger than a certain threshold, in the large object section (LOS). SGen depends on being able to distinguish between the two, so it must not happen that an object changes size, i.e. we have to get rid of String.InternalSetLength, which is used by StringBuilder. The attached patch fixes this problem, but of course it has to do more copying. Does anybody object to this? Any alternatives? Good catch for this issue. I think we should keep the optimization. To fix it it should be enough to add a check for the problematic case: if setting the new length would change the storage space, the copy codepath is taken instead. Let's have this in String.cs: static readonly int LOS_length = GetLOSLength (); // icall optimized by the jit to a constant. and then you can have: --- System.Text/StringBuilder.cs (revision 147553) +++ System.Text/StringBuilder.cs (working copy) @@ -207,8 +207,7 @@ if (null != _cached_str) return _cached_str; - // If we only have a half-full buffer we return a new string. - if (_length (_str.Length 1)) + if (_length (_str.Length 1) || (_str.Length = string.LOS_length _length string.LOS_length)) { // use String.SubstringUnchecked instead of String.Substring // as the former is guaranteed to create a new string object lupus -- - lu...@debian.org debian/rules lu...@ximian.com Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] New error handling framework for mono
On 08/20/09 Rodrigo Kumpera wrote: I don't like MONO_ERROR_ON_SIGNAL_CONTEXT: most code is not going to know in what mode it was called in. Simply always use the message buffer. I changed this to have a MONO_ERROR_STORE_FULL_MESSAGE flag. My reasoning behind this is that some messages are exceptionally large, specially when it does include the assembly path. Look at loader.c:330, just the format string is more than 320 chars long and I don't want to embed it inside the MonoError machinery. Now about MONO_ERROR_ON_SIGNAL_CONTEXT, it made sense to me when the caller did init MonoError . Now that this change, it has to change. I've added a MONO_ERROR_DONT_COPY_STRINGS flag but, honestly, I don't think it is that helpfull and it doesn't help at all into make it safe for use under signal context. I'm not happy with both the MONO_ERROR_STORE_FULL_MESSAGE and MONO_ERROR_DONT_COPY_STRINGS flags you introduced. For the message this could be handled automatically: if it doesn't fit into the buffer you can malloc it at set time, the code that initializes the MonoError has no way to know if the message will fit or not. Similarly, the initializing code can't know if any of the strings need to be strduped because it's far away from the code that sets it. This should be decided at the callsite, either passing a flag or calling a separate function that does the dup. A small note on overlong messages. I think we should have a policy like this: *) error messages should be short sentences *) any non-essential description of the error and suggestions for workarounds should go into either or both the logging mechanism and the documentation (wiki etc.) But what should be done when we get an allocation failure on the mono_exception_set_* function? Should we just convert such error into an OOM, abort due to a double-fault or silently ignore those arguments (and print something on console). It is tricky. In some cases we want to communicate an OOM exception to the managed code. In some others we can't: we need to give the caller code a chance to handle the error and not crash. Using a flag here can be helpful: something like MONO_ERROR_INCOMPLETE to signal that the full info was not stored for whatever reason. That should about strike a good balance between being robust and keeping track of as much info as we can. Aborting here is not an option, IMHO. --- /dev/null +++ b/mono/utils/mono-error.c @@ -0,0 +1,385 @@ +/* + * mono-error.c: Error handling code + * + * Authors: + * Rodrigo Kumpera(rkump...@novell.com) + * Copyright 2009 Novell, Inc (http://www.novell.com) + */ +#include glib.h + +#include mono-error.h +#include mono-error-internals.h + +#include mono/metadata/exception.h +#include mono/metadata/object-internals.h +#include mono/metadata/debug-helpers.h + +#define mono_error_get_message(E) ((E)-full_message? (E)-full_message : (E)-message) + +#define set_error_message() do { \ + va_start (args, msg_format); \ + if (error-flags MONO_ERROR_STORE_FULL_MESSAGE) \ + error-full_message = g_strdup_vprintf (msg_format, args); \ + else\ + g_vsnprintf (error-message, 128, msg_format, args); \ This should be sizeof (error-message) instead of the magic number 128. + va_end (args); \ +} while (0) + +void +mono_error_init_flags (MonoError *oerror, unsigned short flags) +{ + MonoErrorInternal *error = (MonoErrorInternal*)oerror; + g_assert (sizeof (MonoError) == sizeof (MonoErrorInternal)); + + error-error_code = MONO_ERROR_NONE; + error-flags = flags; + error-type_name = error-assembly_name = error-member_name = error-full_message = error-exception_name_space = error-exception_name = NULL; + error-klass = NULL; + error-message [0] = 0; Performance tip: until error_code is MONO_ERROR_NONE there is no need to initialize all the other fields (except flags). This speeds up the common path. Of course you need to check error_code in the cleanup function. +void +mono_error_set_corlib_exception (MonoError *oerror, const char *name_space, const char *name) +{ + MonoErrorInternal *error = (MonoErrorInternal*)oerror; + + if (error-flags MONO_ERROR_DONT_COPY_STRINGS) { + error-exception_name_space = name_space; + error-exception_name = name; + } else { + error-exception_name_space = g_strdup (name_space); + error-exception_name = g_strdup (name); + } This should set error_code as well (maybe have it passed as an argument). +void +mono_error_set_out_of_memory (MonoError *oerror, const char *msg_format, ...) +{ + MonoErrorInternal *error = (MonoErrorInternal*)oerror; + va_list args; + + error-error_code = MONO_ERROR_OUT_OF_MEMORY; + error-flags = ~MONO_ERROR_STORE_FULL_MESSAGE; /*Can't really allocate memory under OOM*/ + va_start (args, msg_format); + g_vsnprintf
Re: [Mono-dev] New error handling framework for mono
On 08/18/09 Rodrigo Kumpera wrote: Still open is how this would be integrated on 2.6 I think we should keep this internal for 2.6 and expose it in 2.8 wth the other API changes, once we have gained experience with the new API. and if functions should error out if passed an already set error object. I think as a general rule the called function should set the error code to OK. We already have this pattern (like in mono_image_open ()) and we should keep it. Once we have called a function that can and does result in an error, it is pointless to continue execution. A comment about the use of TLS data that was raised in the thread: *) we use that pattern already in mono and it has been proved to be a huge source of bugs *) it introduces issues when an additional error happens while handling the first one (info is either overwritten and lost or you have to manually keep track of it) The first one is reason enough to avoid doing the same mistake twice. --- a/mono/metadata/Makefile.am +++ b/mono/metadata/Makefile.am @@ -160,6 +160,9 @@ libmonoruntime_la_SOURCES = \ threadpool-internals.h \ verify.c\ verify-internals.h \ + mono-error.c\ + mono-error.h\ + mono-error-internal.h \ This should be in mono/utils, so we can use it also in the code there. --- /dev/null +++ b/mono/metadata/mono-error-internals.h +void +mono_error_set_type_load (MonoError *error, MonoClass *klass, char *msg_format, ...) MONO_INTERNAL; + +void +mono_error_set_type_load2 (MonoError *error, char *type_name, char *assembly_name, char *msg_format, ...) MONO_INTERNAL; load2 is ugly, use something like mono_error_set_type_load_name, instead. The other one could be called mono_error_set_type_load_class to keep more symmetry in the naming. +MonoException* +mono_error_prepare_exception (MonoError *error, MonoDomain *domain) MONO_INTERNAL; This is one function where you could actually start using the new mechanism, adding a MonoError *error_out as the last argument. --- /dev/null +++ b/mono/metadata/mono-error.c @@ -0,0 +1,253 @@ + +#define mono_error_get_message(E) (((E)-flags MONO_ERROR_ON_SIGNAL_CONTEXT) ? (E)-message : (E)-full_message) I don't like MONO_ERROR_ON_SIGNAL_CONTEXT: most code is not going to know in what mode it was called in. Simply always use the message buffer. +void +mono_error_init (MonoError *error, unsigned short flags) The common usage will be without any flags, so I would have the simpler void mono_error_init (MonoError *error) and void mono_error_init_flags (MonoError *error, unsigned short flags) in the case we actually want to set the non-default flags. +void +mono_error_cleanup (MonoError *error) +{ + if (error-flags MONO_ERROR_ON_SIGNAL_CONTEXT) //no memory was allocated + return; A flag that tells the code the strings are to be freed is more general-purpouse than MONO_ERROR_ON_SIGNAL_CONTEXT, IMHO. +void +mono_error_set_assembly_name (MonoError *error, char *assembly_name) It's a good idea to have the string arguments be const char*. +void +mono_error_set_assembly_load (MonoError *error, char *assembly_name, char *msg_format, ...) +{ + va_list args; + error-error_code = MONO_EXCEPTION_FILE_NOT_FOUND; File not found is not the only failure case for an assembly load, we should be more flexible here. Think of a bad image, for example, but also of an out of memory issue. --- /dev/null +++ b/mono/metadata/mono-error.h @@ -0,0 +1,35 @@ +#ifndef __MONO_ERROR_H__ +#define __MONO_ERROR_H__ + +#include mono/metadata/class.h +#include mono/metadata/metadata.h + +/*Don't allocate memory or call signal unsafe functions*/ +#define MONO_ERROR_ON_SIGNAL_CONTEXT 0x0001 We should use an enum for the flags. +typedef struct { + unsigned short error_code; //MONO_EXCEPTION_* +unsigned short flags; //MONO_ERROR_* + + char *type_name; + char *assembly_name; + char *member_name; + MonoClass *klass; + char *full_message; + gpointer padding [4]; +char message [128]; The field names should be clearly marked as don't touch, except maybe error_code. Names like hidden_1, hidden_2 etch should work. Also use void* as the type. In the internal code at the top of the file you can then do: #define type_name ((char*)hidden_1) to keep the code readable. It's kind of an hack. An alternative is to have a separate struct with the correct types and names and cast to it on public function entry. Thanks, looking forward an updated patch! lupus -- - lu...@debian.org debian/rules lu...@ximian.com Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] [PATCH] Handle more gracefully invalid generic instantiations
On 06/26/09 Rodrigo Kumpera wrote: The attached patch changes class.c/inflate_generic_type to not abort the runtime when facing a bad instantiation. My only issue is that I'm not sure if mono_class_inflate_generic_type* and mono_class_inflate_generic_class should set a loader error when returning null. I don't think this is necessary as this is something their callers should do. But maybe mono_class_inflate_generic_method_full should. In general only the low-level code knows the reason for the failure, so the low-level code should propagate this info (and the exact reason for the failure is often useful when debugging). Note how instead your patch removes some potentially useful info. For that reason I'm not much a fan of having functions return a boolean, because that gives very little information. On the other hand, using something like GError, with it's forced malloc() usage may not be an appropriate pattern for use in the whole runtime. A compromise suggestion could be something like this: typedef struct { unsigned short error_code; // this can be MONO_EXCEPTION_* unsigned short flags; void *data0; // same as exception_data in MonoClass or other fields in MonoLoaderError void *data1; void *data2; void *datan; char message [128]; } MonoError; A few considerations: *) the struct would be passed by ref as the last argument of the runtime functions *) it can be stack allocated, so the malloc is avoided and it can be used also in signals etc. This means that message should be kept small, no more than 256, though. *) eventually it should be also the mechanism for reporting errors with the embedding interface, so there are a few dummy fields for future expansion, but the internals should be considered private and once published the struct won't be changed. *) setting the error should be done with a few simple functions like: void mono_error_set_ok(MonoError *error); // no error void mono_error_set_bad_image (MonoError *error, MonoImage *image, char *msg_format, ...); etc. *) I think it would be a good idea to always require the error argument to be non-null. *) error checking would be done with the (macro equivalent) of: gboolean mono_error_ok (MonoError *error); See below how the first snippet of code would become. --- a/mono/metadata/class.c +++ b/mono/metadata/class.c @@ -490,20 +490,24 @@ mono_class_is_open_constructed_type (MonoType *t) } } -static MonoType* -inflate_generic_type (MonoImage *image, MonoType *type, MonoGenericContext *context) +/* + * This function returns TRUE if it could proper inflate @type. + * The resulting type can be found in @res + */ +static gboolean +inflate_generic_type (MonoImage *image, MonoType *type, MonoGenericContext *context, MonoType **res) { +#define SUCCESS(VAL) do { *res = VAL; return TRUE; } while (0) +#define ERROR() do { *res = NULL; return FALSE; } while (0) switch (type-type) { case MONO_TYPE_MVAR: { MonoType *nt; int num = mono_type_get_generic_param_num (type); MonoGenericInst *inst = context-method_inst; if (!inst || !inst-type_argv) - return NULL; + SUCCESS (NULL); if (num = inst-type_argc) - g_error (MVAR %d (%s) cannot be expanded in this context with %d instantiations, - num, mono_generic_param_info (type-data.generic_param)-name, inst-type_argc); - + ERROR (); static MonoType* inflate_generic_type (MonoImage *image, MonoType *type, MonoGenericContext *context, MonoError *error) { mono_error_set_ok (error); switch (type-type) { case MONO_TYPE_MVAR: { MonoType *nt; int num = mono_type_get_generic_param_num (type); MonoGenericInst *inst = context-method_inst; if (!inst || !inst-type_argv) return NULL; if (num = inst-type_argc) mono_error_set_bad_image (error, image, MVAR %d (%s) cannot be expanded in this context with %d instantiations, num, mono_generic_param_info (type-data.generic_param)-name, inst-type_argc); return NULL; ... Comments welcome (also by future users of the embedding interface). lupus -- - lu...@debian.org debian/rules lu...@ximian.com Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] [PATCH] Mono port to the MIPS 64-bit
On 04/23/09 Mark Mason wrote: As for splitting MIPS/MIPS64 - I think keeping them together may be more appropriate. The MIPS64 code generator is less of a divergence from MIPS32 than PPC64 is from PPC --- there's a lot of potential code reuse to be had by keeping them together. That's why I'd started the n32 port in the way that I did (and that Ian company continued with for extending it). Yes, the ports should be shared. These days for ppc only the machine description is separated, I guess miguel was thinking of the ppc work-in-progress merge changes when things were separate, but that was done just to help fix the code before the merge and there is no point to do that on mips. In fact (if there is any volunteer:) the same should be done for the s390 and s390x ports so that all that duplicated code is reduced. As for the machine description files: I just committed a change in svn that allows multiple files to be used, so for mips and mips64 you could split the existing file into: cpu-mips-common.md and cpu-mips32.md and cpu-mips64.md where the latter files will include only the opcodes that are actually changed in 32 and 64 bit systems, again reducing duplication. The current cpu-mips.md diff in bugzilla is suboptimal as it forces the larges sizes to be used. With the split it will be much better. The same could be done for ppc and ppc64. lupus -- - lu...@debian.org debian/rules lu...@ximian.com Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] [PATCH] Mono port to the MIPS 64-bit
On 04/24/09 Ian Dichkovsky wrote: And we have prepared ChangeLogs and added them to https://bugzilla.novell.com/show_bug.cgi?id=497320 ChangeLog.diff ChangeLog_libgc.diff ChangeLog_mono_arch.diff ChangeLog_mono_io-layer.diff ChangeLog_mono_mini.diff Thanks for the port! A few general comments on the changes: *) please post the next iteration on this list, each patch with its changelog (bugzilla is not the best en to comment on patches) *) please use something like MONO_PPC_32_64_CASE in the ppc code when you need simple values to differ in 32 and 64 bit mode. For example you just increase trampoline sizes, but for 32 code they should remain smaller and not waste memory. This helps reducing also #ifdefs in the code in a few places *) mono_arch_find_jit_info() is duplicated entirely, it's better to aoid too much code duplication there. *) in some places you use (unsigned long long) casts: it's better to use gsize, so the code deals better with the 32 bit case. *) some of the mini-mips.h changes likely break existing 32 bit code: you need to #ifdef the 64 bit specific stuff (MONO_ARCH_INST_IS_REGPAIR...) *) things like: +#if SIZEOF_VOID_P == 4 add_int32_arg (cinfo, cinfo-args[n]); +#else + add_int64_arg (cinfo, cinfo-args[n]); +#endif are better expressed with a single #ifdef: #if SIZEOF_VOID_P == 4 #define add_reg_arg(a,b)add_int32_arg (a, b) #else #define add_reg_arg(a,b)add_int64_arg (a, b) #endif and then using add_reg_arg() in the code. *) in quite a few cases indentation is incorrect, with the opening brace put on a line on its own when it should be in the same line as if/while/etc. In other cases there is the orrible: ... } else { ... which should be instead: ... } else { ... *) in general it seems that in a few places the older code was simply replaced. Now I don't know the arch details, so the new code may as well be correct also for the 32 bit port, but it seems that in some cases too much code just code removed and things were tests on 64 bit only. Anyway, good work, looking forward the updated patches and Mark's comments. lupus -- - lu...@debian.org debian/rules lu...@ximian.com Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] Use eglib as a default for mono 2.6
On 04/23/09 Miguel de Icaza wrote: The cons are: * We still need to deprecate/break the API for the functions listed in [1] At some point we need to break API/ABI anyway, since it's required for the new GC, we need to hide some implementation details that are currently exposed, like MonoArrayType, MonoMethodSignature, MonoType itself, MonoTypeNameParse, MonoArray and a few ugly API entry points. We need to coordinate to break the API/ABI just once for all the needs, we can't break it 2-3 times. There are still a handful of places where glib is better than eglib, tiny bits here and there, but nothing really major. There are several reasons for having our own code, in no particular order: *) remove a dependency which is getting bigger every day *) being able to inspect inside the data structures (GHashTable for example is opaque and several times I needed to inspect it for debugging; in other cases we'd need to check if the hash distribution is good and we can't with GHashTable) *) GLib doesn't support some APIs we need: a GHashTable foreach that can be early-terminated is one example, we currently waste time visiting it all even if we found what we were looking for. *) we'll never be able to control out of memory cases with glib since it just dies. This can happen in many cases even if plenty of memory is otherwise available for recovery or graceful handling. *) glib installs a compiler-specific file, so the same compiler must be used for mono and glib (this mainly confuses the solaris users, but there are also other cases) *) mono can't be used as a plugin in single-threaded apps that use glib (glib inited without threading support- we die) *) we need about 50 kb of readonly text section to implement the features we need, just linking with glib brings 20 KB of writable memory and several pages of text. *) we really just use a tiny subset of glib the API/ABI break is a good time to switch for the above advantages We could also make sure that the structures map 1 to 1 to the Glib structures for the ones that we use, that would prevent the need to even break the API when using the remapping macros. It doesn't matter, we need to break the API/ABI anyway and such hacks are better avoided. But I can see why this last one might make some people uncomfortable. [1] This is the list of public API entry points that expose Glib data structures. metadata/assembly.h:void mono_assembly_foreach(GFunc func, gpointer user_data); metadata/verify.h:GSList* mono_image_verify_tables (MonoImage *image, int level); metadata/verify.h:GSList* mono_method_verify (MonoMethod *method, int level); metadata/verify.h:voidmono_free_verify_list(GSList *list); [...] There are others, like the ones exposing MonoTypeNameParse, plus all the ones where one is supposed to free results with g_free(). As for the cases where glib might be faster than eglib (small struct allocations, for examples) it's likely that we should optimize mono not do abuse them, because if it's significant in the overall time for mono apps something looks wrong. lupus -- - lu...@debian.org debian/rules lu...@ximian.com Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] [PATCH] Less sharing between AppDomains
On 04/10/09 Mark Probst wrote: The first patch moves the NumberFormatter out of Thread. As it is now, the NumberFormatter is shared between AppDomains. The second patch makes sure that an array obtained in another AppDomain is not retained in the calling domain but copied. This looks fine to commit. lupus -- - lu...@debian.org debian/rules lu...@ximian.com Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] Thread shutdown hook patch
On 02/09/09 Rodrigo Kumpera wrote: The attached patch adds a new hook to allow threads to shutdown after the GC finalizer has finished. The motivation for it is to improve profiler's reliability at shutdown time. The new callback notifies the thread when regular shutdown starts and gives it a change to not finish at this time. Later on the same callback is used to notify the thread that the last stage in the shutdown sequence has been reached and it must shutdown. The callback is per-thread as I don't want to have tools like the profiler messing up with embedded users. I think this instroduces too much complexity: since the API is public we'd need to end up maintaining the semantics as they happen to be now. It would be much clearer to have a flag on the thread that behaves similarly to critical finalizers: the thread that has it set will be destroyed as late as possible on shutdown. As for the callback, the existing mono_profiler_thread_end () should be enough: if it isn't we need to discuss how we can fix that instead of adding yet another callback. lupus -- - lu...@debian.org debian/rules lu...@ximian.com Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] high precision real cpu time in MONO (on linux)
On 01/31/09 Basile Starynkevitch wrote: Apparently mono has some way to get it, since mono-2.2 contains the file mono/utils/mono-time.[ch] with functions gint64 mono_100ns_ticks(void) MONO_INTERNAL; gint64 mono_100ns_datetime (void) MONO_INTERNAL; I blindly guess that MONO_INTERNAL would mean that these functions are not accessible from C# 3.0 code. You should use the StopWatch class (the implementation internally will use mono_100ns_ticks) for monotonic time and DateTime if you want the time (equivalent of calling gettimeofday() in C). If you need more control than that, you can p/invoke clock_gettime(), though you'll lose somewhat in portability. lupus -- - lu...@debian.org debian/rules lu...@ximian.com Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] Mono and IBM
On 02/01/09 Andreas Färber wrote: There might be a mono port for AIX, which might run in the PASE environment, but i don't know the status of that. Some folks recently tried a new port to AIX/ppc64. They gave up last week because, supposedly, Mono relies on an undefined behaviour of NULL dereferencing. Don't know details though. A competent programmer like our ppc maintainer could likely do an AIX port in under a week, we just don't have access to the hardware and nobody offered it or the money to push us to do the port. There is nothing fundamental in mono that depends on NULL dereferencing (in fact our jit already has the IR needed to represent a null check, so a non-optimized fix for AIX stupidly making the 0 page readable is in the order of 20-30 lines of code). lupus -- - lu...@debian.org debian/rules lu...@ximian.com Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] [PATCH] Enable TLS for PPC32/64
On 01/28/09 Steven Munroe wrote: if there WAS a problem will some chip in the past we address that separately. But this changes are important to the server class machines and I have carefully crafted this patch to only optimize for the case where we know (from the auxv) what the hardware actually is. Don't penalize the latest hardware for the sins of the a chip that no one makes any more... You're asking us to accept a possibly risky change without providing any numbers on the benefits (which is what I requested in my mail). Right now the only numbers I have are this: when profiling mono on ppc I have never seen the icache flush function appear anywhere near the first few pages of hot code. As you say the change is important you won't have any difficulty in providing hard numbers and system information where mono gets a speedup with the change (running something like mono --compile-all /usr/local/lib/mono/2.0/gmcs.exe may be your best bet for exercising the icache flushing, but any relevant benchmark using mono you can come up with is fine). If you're looking at improving the speed of mono on ppc I can suggest many other things which will provide significant speedups (like for example remove FORCE_INDIR_CALL in mini-ppc.c). lupus -- - lu...@debian.org debian/rules lu...@ximian.com Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] mini-OS patches.
On 01/28/09 Miguel de Icaza wrote: The current patch splits out the OS-specific functionality into three files mini-darwin.c, mini-posix.c and mini-windows.c, these mostly deal with setting up exceptions today. This looks fine to me. I wouldn't bother with the configure changes, though and just always add all the files to the build and protect the contents with the appropriate #ifdef. When committing, this should likely be done with a svn copy, so that the history of the changes is mostly preserved. Thanks! lupus -- - lu...@debian.org debian/rules lu...@ximian.com Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] [PATCH] Enable TLS for PPC32/64
On 01/25/09 Steven Munroe wrote: What you consider superfluous addresses an annoying bug where the mkbundle generates code that can never be assembled and results in a 2GB a.out file. This is a result of brain-dead code the than can't handle biarch builds (where the build is for the non-default arch). Has far I can tell mkbundle is disabled for PPC32 and has been for some time ... mkbundle used to work on linux and osx on ppc32. IIRC it was disabled because for some reason that I didn't investigate, the pkg-config call failed while running make test on the POS that is OSX, while it worked when the test was executed standalone. So there should be no reason to disable it on ppc64: if there is an actual bug it needs to be fixed. 2009-01-24 Steven Munroe munro...@us.ibm.com This patch is contributed under the terms of the MIT/X11 license * exceptions-ppc.c (mono_arch_get_restore_context): Correct g_assert test. * tramp-ppc.c (mono_arch_create_trampoline_code): Use MONO_PPC_32_64_CASE to set tramp_size for 64-bit. * Makefile.am: Disable mkbundle for POWERPC64. diff -urN mono-svn0/mono/mono/mini/exceptions-ppc.c mono-svn-fix/mono/mono/mini/exceptions-ppc.c --- mono-svn0/mono/mono/mini/exceptions-ppc.c 2009-01-23 17:05:56.0 -0600 +++ mono-svn-fix/mono/mono/mini/exceptions-ppc.c 2009-01-24 12:21:08.0 -0600 @@ -218,7 +218,7 @@ /* never reached */ ppc_break (code); - g_assert ((code - start) size); + g_assert ((code - start) = size); mono_arch_flush_icache (start, code - start); return start; } diff -urN mono-svn0/mono/mono/mini/tramp-ppc.c mono-svn-fix/mono/mono/mini/tramp-ppc.c --- mono-svn0/mono/mono/mini/tramp-ppc.c 2009-01-23 17:05:56.0 -0600 +++ mono-svn-fix/mono/mono/mini/tramp-ppc.c 2009-01-24 12:22:03.0 -0600 @@ -504,7 +504,7 @@ guint8 *jump; int tramp_size; - tramp_size = 32; + tramp_size = MONO_PPC_32_64_CASE (32, 44); code = buf = mono_global_codeman_reserve (tramp_size); This part is ok to commit. 009-01-24 Steven Munroe munro...@us.ibm.com This patch is contributed under the terms of the MIT/X11 license * ppc-codegen.h: Make operand order and case consistent (assembler order) for ppc_load_reg_update, ppc_load_multiple_regs, ppc_store_multiple_regs, ppc_lwz, ppc_lhz, ppc_lbz, ppc_stw,ppc_sth, ppc_stb, ppc_stwu, ppc_lbzu, ppc_lfdu, ppc_lfsu, ppc_lfsux, ppc_lfsx, ppc_lha, ppc_lhau, ppc_lhzu, ppc_lmw, ppc_lwzu, ppc_stbu, ppc_stfdu, ppc_stfsu, ppc_sthu, ppc_stmw. Use i or ui instead of d for immediated operands to immediate arthimetic and logical instructions in macros ppc_addi, ppc_addis, ppc_ori, ppc_addic, ppc_addicd, ppc_andid, ppc_andisd. [__mono_ppc64__]: Make operand order and case consistent (assembler order) for ppc_load_multiple_regs, ppc_store_multiple_regs. Simplify the DS form and make them consistent with D forms for ppc_load_reg, ppc_load_reg_update, ppc_store_reg, ppc_store_reg_update. ppc_ld, ppc_lwa, ppc_ldu, ppc_std, ppc_stdu. Define ppc_lwax and ppc_lwaux. * exceptions-ppc.c (restore_regs_from_context): Correct operand order (offset then base reg) for ppc_load_multiple_regs. (emit_save_saved_regs) Correct operand order for ppc_store_multiple_regs. (mono_arch_get_call_filter): Correct operand order for ppc_load_multiple_regs. * mini-ppc.c (emit_memcpy): Fix operand order for ppc_load_reg_update and ppc_store_reg_update. (mono_arch_output_basic_block): Correct operand order for ppc_lha. (mono_arch_emit_epilog): Correct operand order for ppc_load_multiple_regs. * tramp-ppc.c (mono_arch_create_trampoline_code): Correct operand order for ppc_store_multiple_regs and ppc_load_multiple_regs. This patch is fine. 2009-01-24 Steven Munroe munro...@us.ibm.com This patch is contributed under the terms of the MIT/X11 license * ppc-codegen.h: Change ppc_is_imm16 and ppc_is_imm32 to avoid compiler warnings in 64-bit. Will this generate warnings on ppc32 instead? * mini-ppc.c (emit_memcpy): Use type long instead of int for values used to large immediate fields to eliminate range warnings. (mono_arch_emit_outarg_vt) [__APPLE__]: Declare size. (emit_float_to_int); Change vars offset and sub_offset to type long. (emit_load_volatile_arguments) [__APPLE__]: Declare size. (emit_reserve_param_area): (emit_unreserve_param_area: Change var size to type long. (mono_arch_output_basic_block): Copy cfg-stack_usage to local type long to avoid warnings. Also compute LR save offset in a local long ret_offset. [!__mono_ppc64__]: Use smaller
Re: [Mono-dev] mono 2.0.1 that are failing on FreeBSD
On 11/17/08 Phillip N. wrote: Failed tests: exception.exe remoting2.exe remoting3.exe generics-sharing.2.exe generic-null-call.2.exe thunks.exe bug-78549.exe gmake: *** [testjit] Error 1 *** Error code 2 I was building a LiveCD for taking the problem closer to any of you, but as i can read some already have a vm with FreeBSD. If i should provide more info, just tell. You need to provide more info, like run each test individually inside gdb and get as much info as possible about the failure. lupus -- - [EMAIL PROTECTED] debian/rules [EMAIL PROTECTED] Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] Ideas for Mono on Windows
On 11/11/08 Jonathan Chambers wrote: 1) We should consider using MSVC as the default compiler for C code on Windows. I can compile the entire Visual Studio solution for the runtime in minutes. It takes 20-30 seconds if I do a parallel build. We can also use the Visual Studio debugger on Windows, which IMO is betten than gdb on Windows. So this is already done by you, AFAIK. I even add new .c files to the MSVC build files (when I remember about it:). it). Another option is to moved to MSBuild/xbuild for the class libraries. I think you're taking it from the wrong side. There is no need to move the Linux build system to xbuild or whatever. As other have mentioned, the build files can be generated from the .sources files quite easily. You can write the conversion script in perl or even C# and we can have rules in the makefiles to run them automatically when the source files are modified. AFAIK, the only two things missing are: 1) a windows developer that volunteers writing the script and MSBuild/xbuild files. 2) a windows developer that documents the thing for other windows developers so they are not tricked into going down the cygwin route anymore and just click on the right buttons instead. As you see we only need some windows developer to step up to the plate. I suggest starting to build the complete assemblies first, without going into the details of bootstrapping. This should take an hour or two. And then we (mostly hari I guess) can help with the bootstrap details. So, who volunteers? lupus -- - [EMAIL PROTECTED] debian/rules [EMAIL PROTECTED] Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] Ideas for Mono on Windows
On 11/13/08 Jonathan Chambers wrote: I have started writing this tool in C#. It correctly generates a csproj file from the Makefile and the library.dll.sources files. It already works on most assemblies in mcs/class. I'm working on generating the corresponding NUnit projects as well.I'll hopefully post something in a couple of days. Excellent news! IMHO, the sooner you commit your script, the sooner other windows developers can help improving it. lupus -- - [EMAIL PROTECTED] debian/rules [EMAIL PROTECTED] Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] [PATCH] Optimize Mono.Simd non-accelerated bitwise-ops
On 11/10/08 Cedric Vivier wrote: This patch implements these ops as two 64-bit bitwise ops [2] instead of processing Sadly this is not safe as the alignment we currently can guarantee is only to 4 bytes and a few architectures don't support loading a 64 bit value unless it's 8-byte aligned. Could you implement it using uints instead? lupus -- - [EMAIL PROTECTED] debian/rules [EMAIL PROTECTED] Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] WindowsBase in mcs
On 11/10/08 Alan McGovern wrote: The only other option is for there to be a place on disk which is: A) Guaranteed to be writable B) Guaranteed to have enough space to create the package. If those guarantees can be given, I'll gladly make the change. If they can't, then I don't want to completely break this API just to save memory. This argument is ridiculous. How do you guarantee with the current code that you have enough RAM available instead? You can use /tmp: if there is not enough disk space you'll throw an exception. And there is nothing here that would break the API: the API does not require nor does it guarantee that all the data is kept in memory. Please don't make up excuses: keeping the data in memory is simply a bug in the current code and as such it should be fixed. On 11/10/08 Alan McGovern wrote: If you're actually writing the package to disk, you need free space == size (package) * 2 as you will have to store a temporary full copy of the package as well as the final copy. This is not true in the common case: you write the data to the disk file once and at the end you write the zip directory. At the end of the process you can rename from the temp file to the destination file, for example. If somebody does something stupid like removing streams after they have been added, they'll also pay the price of the additional overhead. lupus -- - [EMAIL PROTECTED] debian/rules [EMAIL PROTECTED] Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] WindowsBase in mcs
On 11/06/08 Alan McGovern wrote: However this use case is likely to never be hit in System.IO.Packaging. The API requires that when you 'write' data to a zip archive, the data is also kept in memory. If you did try to use a 2gb file with this API, you'd end up trying to allocate a 2gb block of memory. There is no requirement in the API that data is kept in memory, this bug needs to be fixed, if you don't know how, at least file a bug report in bugzilla so we don't forget about it. lupus -- - [EMAIL PROTECTED] debian/rules [EMAIL PROTECTED] Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] Mono.Simd Acceleration Attributes
On 11/07/08 Christophe Guillon wrote: It seems that as soon as the Mono.Simd primitives have a well defined semantic it is not useful to specify which architecture feature is able to emulate each of these primitives. I would have expected this to be the choice of the virtual execution environment. It _is_ ultimately a choice of the runtime. These attributes are never inspected by the runtime to decide whether to optimize a method call or not. - if my underlying hardware XXX (not SSE2) is able to support efficiently add with saturation, I do not have to know whether SSE2 also supports it, the virtual machine for XXX can use the corresponding add with saturation instruction of XXX at the call sites of AddWithSaturation() anyway, When the runtime will implement that optimization, the attribute will be changed to include SSE2 and your architecture (say AltiVec or Neon etc). Yes, this requires a re-release of Mono.Simd, but it's not a big deal as the changes will be relatively rare and if you are happy to use unoptimized Mono.Simd anyway it doesn't matter. - if my underlying hardware features SSE2, the attribute is not useful, the virtual machine knows the underlying hardware and thus know that a SSE2 instruction is able to emulate this, It's useful to the Mono.Simd programmers, the runtime doesn't use it. - if the attribute is there to restrict the mapping to only SSE2 (and above) machines, it is an important restriction to the usage of the library. Imagine as above that I have in the future a hardware support XXX that is able to do AddWithSaturation on Vector16b; if I want a virtual machine to execute efficiently this primitive on XXX I would first have to modify the Mono.Simd library to add the corresponding XXX attribute and modify the primitives declaration to account for it. Nope, this is not correct. The behaviour is as follows: 1) the runtime will choose whether a method is optimized or not depending on the optimization flags (-O=simd, on by default) and on the features of the current processor. 2) the attributes on the methods are never inspected by the runtime: they are there to guide the programmers using Mono.Simd in determining what kind of optimizations are usually available or currently enabled. The reasoning is this: using unoptimized Mono.Simd is currently significantly slower than he equivalent scalar code. This has mostly to do with the additional copies that happen because of the operator overloading. This overhead is expected to decrease as we add more jit optimizations. So you have two cases: 1) the slowdown is not significant to you (you must test! Run your program with mono -O=simd and with mono -O=-simd): in this case you should ignore completely the acceleration attributes and just enjoy the speedup that the jit will give you when it can optimize the methods. 2) if the slowdown is significant you might want to have two codepaths, mostly in the same way in C/C++ you have a C implementation and a simd implementation of the critical functions. Now the question becomes: how do you choose at runtime if you want to use Mono.Simd or the scalar codepath? We offset two patters: a) do a coarse decision: you take a look at the methods you use in your algorithms and see that they are optimized when SSE2 is enabled, so you just do: static readobly bool use_mono_simd = (SimdRuntime.AccelMode AccelMode.SSE2) != 0; ... if (use_mono_simd) // simd codepath else //scalar codepath b) a fine-grained decision based on all or some of the methods you use: for each method you check (SimdRuntime.MethodAccelerationMode (typeof(...), ...) SimdRuntime.AccelMode) != 0 until you determine that enough of your methods are accelerated to make it worth using the Mono.Simd codepath. Note that we may eventually either return the attribute not based on the metadata in the assembly, but based on the runtime understanding: this will avoid the need to have an updated Mono.Simd assembly when new optimizations are added. Just use the b pattern if you want to avoid that issue and remember that you don't usually need to check all the methods, but just the ones you actually need to be optimized. lupus -- - [EMAIL PROTECTED] debian/rules [EMAIL PROTECTED] Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] mono resources requirements
On 11/06/08 Nicolò Chieffo wrote: Hello, I would like to run mono in a low resources machine (ARM cpu, low ram) which is the minimum resource requirement to run mono? (I need system.timers, system.runtime.remoting, events and delegates with BeginInvoke) See http://www.mono-project.com/Small_footprint. Is there a compact framework for low disk space? See http://www.mono-project.com/Linker: this is much more flexible than the compact framework, since you can include just what you need, without been limited (both in how minimal you can get and in how many features you might need) by the CF assembly set. lupus -- - [EMAIL PROTECTED] debian/rules [EMAIL PROTECTED] Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] WindowsBase in mcs
On 11/05/08 Alan McGovern wrote: 2) minizip exposes 'long' types in it's public API. At the moment I've wrapped these as IntPtr types. This will run on all platforms except Win64. To resolve this, we'd need to create a wrapper API which exposes the 'long' types as int64_t. When dealing with file offsets long is the wrong type to use, since it's broken for 32 bit systems too, not only for win64. int64_t should be used (or gint64...), but the code must be changed to fix this issue, there is no point in adding a wrapper API, which wouldn't solve the problem. lupus -- - [EMAIL PROTECTED] debian/rules [EMAIL PROTECTED] Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] [PATCH] Implement Diagnostics.Debugger.IsAttached
On 10/18/08 Cedric Vivier wrote: Please review attached patch implementing Diagnostics.Debugger.IsAttached. Please be careful with your patches and don't include whitespace changes mixed with functionality changes. Thanks. lupus -- - [EMAIL PROTECTED] debian/rules [EMAIL PROTECTED] Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] Dll not found when deploying a mono app into a different machine
On 10/13/08 Andoni Morales wrote: I'm having a very strange problem on Windows I've created a multimedia player based on GStreamer an Gtk that runs both on Windows and Linux. The GStreamer backend is written in C and wrapped to C# using the GAPI tools, importing my own dll called libcesarplayer.dll. I compile and link libcesaplayer.dll on windows (I've tried both Mingw and MSVC) and all works well. But If I try to run my app in my laptop, for example, I got a DllNotFoundException. So, it just works in the computer I used to compile libcesarplayer.dll. Do I have to compile it in a special way? It's likely that the library depends on another library that you have installed and that you didn't copy (or you use a windows binary on a linux system, who knows, you provide too few details). You can run with: MONO_LOG_LEVEL=debug MONO_LOG_MASK=dll mono yourprogram.exe as detailed in the man page to see what error might happen when loading your library. lupus -- - [EMAIL PROTECTED] debian/rules [EMAIL PROTECTED] Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] Patch: Fix fast generic virtual method calls with remoting
On 10/09/08 Mark Probst wrote: Anyway, updated patch attached. The changes look fine to me. Maybe it's a good idea to have a remoting test that triggers this also in the runtime tests. lupus -- - [EMAIL PROTECTED] debian/rules [EMAIL PROTECTED] Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] Mono 2.0 (arm): Reverse PInvoke can cause memory corruption?
On 10/10/08 FirstName LastName wrote: I have a general question regarding GC. When the GC runs, does it stop all threads in the process, even those created in unmanaged. Only the ones it knows about, but see below. I asking the following because I'm wondering what would happen if the GC collects and at the same time, a reverse pinvoke is done from a native thread (back to managed code)? The fix I committed will register the thread with the GC before the delegate is executed on that thread. Note that you must not manipulate managed objects in threads that were not known to the runtime (they are known when they where created by mono, the main thread, the ones where reverse p/invoke delegates are called and the ones registered explicitly). Try setting GC_DONT_GC=1 and running the app to see the crashes are due to the GC. lupus -- - [EMAIL PROTECTED] debian/rules [EMAIL PROTECTED] Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] Please advise. Threading in unmanaged code
On 10/10/08 Carsten Harnisch wrote: As I understood mono is using phreads and epoll, so having created a mono-thread and using this thread to call an unmanged piece of code, will this unmanaged code be run in the context of the underlaying pthread? Or does the mono i/o layer call the unmanaged code with its own thread pool? The call will happen in the same thread. lupus -- - [EMAIL PROTECTED] debian/rules [EMAIL PROTECTED] Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] How to create a JIT trampoline?
On 10/10/08 Kornél Pál wrote: Currently mono_image_fixup_vtable actually compiles methods that causes type loads as well and requires the assembly to be loaded. Please help with letting me know how could I avoiding this by using JIT trampolines. I have no idea how to create them and late fixup their address. You can use mono_runtime_create_jump_trampoline (), though it's not really clear what domain you should use, I guess the root domain would work in most cases. If this is to work for other domains as well, you will need more work, I don't know the details of how the methods exported this way are supposed to behave. To optimize the calls after the first something like the current jump_target_got_slot_hash hash may be used so the function address is replaced with the compiled method's code instead of going through the trampoline every time. lupus -- - [EMAIL PROTECTED] debian/rules [EMAIL PROTECTED] Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] Mono's JIT and SSE instructions
On 10/10/08 Bojan Rajkovic wrote: How does Mono deal with SSE-capable processors (or Altivec capable processors)? That is, does the JIT emit any SSE calls when there's a chance to use them, or are they totally unused in Mono? We use some of the SSE instructios if available on x86. On amd64 they are obviously used to implement the fp support. We don't use any altivec instruction as that is pretty much pointless. We don't auto vectorize from IL code, but we're developing an extension that allows people to take advantage of sse instructions with intrinsics (people watching the changes list can see it already). lupus -- - [EMAIL PROTECTED] debian/rules [EMAIL PROTECTED] Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] Mono's JIT and SSE instructions
On 10/11/08 Dax wrote: Paolo Molaro wrote: We don't auto vectorize from IL code, but we're developing an extension that allows people to take advantage of sse instructions with intrinsics (people watching the changes list can see it already). That said, what versions of SSE will be supported? Original SSE only, since it's probably available on every mono-compatible machine, everything up to SSSE3/SSE4..? We'll use what is needed and available on the cpu that's running the mono process. lupus -- - [EMAIL PROTECTED] debian/rules [EMAIL PROTECTED] Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] Using using System.Threading namespace
On 10/06/08 [EMAIL PROTECTED] wrote: I'd like to try System.Threading and System.Threading.Collections on Linux to work a little bit with ParallelFX. Is it going to be included together with the mono sources or should I just go to the Mono GSoc repository? We'll include them in mono when the corresponding libraries will switch from unsupported previews to beta. lupus -- - [EMAIL PROTECTED] debian/rules [EMAIL PROTECTED] Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] Mono crashes when using callbacks from unmanaged to managed world
On 10/07/08 FirstName LastName wrote: I have been doing some tests with Mono quite a while. I'm working with Mono on a ARM. When I try to call a managed callback from the unmanaged world in a unmanaged thread, I get the following: ** ERROR **: file mini.c: line 10315 (mono_get_lmf_addr): should not be reached Currently only x86, amd64 and ppc have support for that. Feel free to file a bug report. lupus -- - [EMAIL PROTECTED] debian/rules [EMAIL PROTECTED] Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] problem building mono, can't get monolite to work
On 10/03/08 [EMAIL PROTECTED] wrote: Ok, I'm using the included one but, any clue why I can't get the monolite to work? monolite should simply not be used, it's an expert option that is not fool proof and is not guaranteed to work, there are too many issues with it that are not even worth solving. Always start from a released mono tarball as the build instructions say.. lupus -- - [EMAIL PROTECTED] debian/rules [EMAIL PROTECTED] Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] What is the state of mono interpreter ?
On 09/29/08 James Mansion wrote: Rodrigo Kumpera wrote: These are shortcomings of the current AOT implementation of mono that are easier to fix than implement a fast interpreter. Who said anything about fast? The mono interpreter already operated on a different internal representation, not on IL code directly, so it's not jit-fast, but still not as slow as Rodrigo would imply with his message. The main issue is that a few people showed interest in bringing the intepreter back to life, saying it was important to them, but they never dedicated 1 man hour to the task (or if they did, they didn't share the code). So, if those people that claim an interpreter is importnat didn't dedicate 1 man hour, why should we? (Note I'm not pointing figers to anyone here, several people wanted an interpreter but were unwilling to spend anything on it, money, man hours or anything else except email postings:). lupus -- - [EMAIL PROTECTED] debian/rules [EMAIL PROTECTED] Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] Patch: Fast virtual generic method calls
On 10/01/08 Mark Probst wrote: Here's the patch again, updated with code to keep track of how many times a generic virtual method is invoked and to insert it in the thunk only if a threshold (currently 100) is reached. 100 seems high, especially since the lookup can be a O(n) list walk. Maybe 10 is more appropriate. Index: metadata/object.c === --- metadata/object.c (revision 114206) +++ metadata/object.c (working copy) +static void +init_thunk_free_lists (MonoDomain *domain) +{ + if (domain-thunk_free_lists) + return; + domain-thunk_free_lists = mono_mempool_alloc0 (domain-mp, sizeof (gpointer) * NUM_FREE_LISTS); mono_domain_alloc0 () should be used, so that all the domain allocations are properly tracked. +/* + * LOCKING: The domain lock must be held. + */ +static void +invalidate_generic_virtual_thunk (MonoDomain *domain, gpointer code) +{ + guint32 *p = code; + MonoThunkFreeList *l = (MonoThunkFreeList*)(p - 1); + + init_thunk_free_lists (domain); + + while (domain-thunk_free_lists [0] domain-thunk_free_lists [0]-length = MAX_WAIT_LENGTH) { + MonoThunkFreeList *item = domain-thunk_free_lists [0]; + int length = item-length; + int i; + + /* unlink the first item from the wait list */ + domain-thunk_free_lists [0] = item-next; + domain-thunk_free_lists [0]-length = length - 1; + + i = list_index_for_size (item-size); + + g_print (putting thunk of size %d in list %d\n, item-size, i); Leftover. +/** + * mono_method_add_generic_virtual_case: Maybe invocation is better than case? Any other suggestion? +void +mono_method_add_generic_virtual_case (MonoDomain *domain, gpointer *vtable_slot, + MonoGenericInst *method_inst, gpointer code) +{ + static gboolean inited = FALSE; + static int num_added = 0; + + GenericVirtualCase *gvc, *list; + MonoImtBuilderEntry *entries; + int i; + GPtrArray *sorted; + + mono_domain_lock (domain); + if (!domain-generic_virtual_cases) + domain-generic_virtual_cases = g_hash_table_new (mono_aligned_addr_hash, NULL); + + /* Check whether the case was already added */ + gvc = g_hash_table_lookup (domain-generic_virtual_cases, vtable_slot); + while (gvc) { + if (gvc-inst == method_inst) + break; + gvc = gvc-next; + } This is the O(n) loop I mentioned. It shouldn't be a big deal, though. + + /* If not found, make a new one */ + if (!gvc) { + gvc = mono_mempool_alloc (domain-mp, sizeof (GenericVirtualCase)); mono_domain_alloc () here. Index: metadata/domain-internals.h === --- metadata/domain-internals.h (revision 114206) +++ metadata/domain-internals.h (working copy) @@ -141,6 +141,12 @@ MONO_APPDOMAIN_UNLOADED } MonoAppDomainState; +typedef struct _MonoThunkFreeList { + guint32 size; + struct _MonoThunkFreeList *next; + int length; /* only valid for the wait list */ Put the pointer first, so sie and length fit inside the other 8 bytes on 64 bit systems and the total size will be 16 instead of 24. --- mini/tramp-x86.c (revision 114206) +++ mini/tramp-x86.c (working copy) @@ -94,7 +94,7 @@ } else { printf (Invalid trampoline sequence: %x %x %x %x %x %x %x\n, code [0], code [1], code [2], code [3], code [4], code [5], code [6]); - g_assert_not_reached (); + //g_assert_not_reached (); Leftover. The rest of the code looks fine to me. It would be nice to also see some speedup numbers from this change:) Thanks! lupus -- - [EMAIL PROTECTED] debian/rules [EMAIL PROTECTED] Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] Ping on nternal call builders
On 07/30/08 Kornél Pál wrote: Zoltan Varga wrote: This patch replaces a small, fast, simple piece of code in mono_emit_inst_for_method () with something far more complex. Also, The three icall builders are reference implementations for demonstrating what icall builders can do. I don't insist on OffsetToStringData being replaced with an icall builder. Note that however that my tests showed no difference in executing the code. (I didn't do tests on JIT time.) If you propose a considerable amount of increased complexity and your reference 'improvements' are actually a lot worse you should at least choose better references to show. about replacing icalls with generated IL code: - the code to generate the icalls is usually much bigger and more complex than the icall itself. That's true but UnsafeAddrOfPinnedArrayElement (that is so simple code that it should do exactly the same in C and IL) for example was running 5.7x faster and I believe only because of omitting the managed to native transition. This cases are rare and there are far better ways to solve this particular issue: it could be done with a simple burg rule for the old jit or with few generated instructions with the new IR. - it replaces code generated by the gcc optimizing C compiler with code generated by out JIT, which is of much lower quality. So even tough we incur a managed-to-native transition overhead by calling native code, we get some (or all) of it back by having gcc compiled code in the icall. I don't believe that methods like UnsafeAddrOfPinnedArrayElement (and there are more methods similarly using fields not exposed to corlib that could benefit from this as well) could be optimized better by gcc than the JIT. There are not many cases like UnsafeAddrOfPinnedArrayElement (the implementation also likely needs security checks, so in the end the implementation will be more complex and the transition overhead will be less significant). I belive that memcpy for example should be implemented using native assembly but it still is implemented in C#. For most common cases the current implementation is faster than a generic memcpy implementation (I tested this at the time by simply using the libc implementation as managed code). I assume you mean OffsetToStringData. When I wanted to implement UnsafeAddrOfPinnedArrayElement in mono_emit_inst_for_method it was not accepted. You needed to introduce your own opcode with the old jit. With the linear IR switch we can allow implementing more methods and icalls by emitting multiple instructions instead of a single tree as in the old code. Also note another consideration: we'll add special cases only for methods and icalls that actually have a perf impact is a real application and so far it doesn't look like UnsafeAddrOfPinnedArrayElement is a bottleneck (feel free to show us applications where it is significant). There is no point in adding complexity to mono for a microbenchmark only. Moving OffsetToStringData out of mono_emit_inst_for_method seems not to be accepted either. Because you added lots of complexity for no gain and slowed down the code. I see OffsetToStringData and UnsafeAddrOfPinnedArrayElement being similar in that they do nothing complex just care about internal structure details of the runtime. Sure, but your OffsetToStringData change makes things worse and the UnsafeAddrOfPinnedArrayElement change is better done in other ways. icall builders are my proposed solution but I'm open to any other solution as well. See above. lupus -- - [EMAIL PROTECTED] debian/rules [EMAIL PROTECTED] Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] [PATCH] Implement internal Encodings usingunified code base
On 03/21/08 Kornél Pál wrote: From: Paolo Molaro [EMAIL PROTECTED] Sure, the existing unsafe methods are perfectly fine, there is no need to add more virtual unsafe methods (this is about 2.0). In 1.1 the unsafe methods would simply become internal. This is unfortunately unture because we cannot call the public unsafe methods because it would result in the problems described in bug 363713. To fix bug 363713 a private (actually internal) unsafe implementation is required. This patch fixes the GetByteCount() issue described in the bug: Index: System.Text/Encoding.cs === --- System.Text/Encoding.cs (revision 98527) +++ System.Text/Encoding.cs (working copy) @@ -1150,7 +1150,7 @@ for (int p = 0; p count; p++) c [p] = chars [p]; - return GetByteCount (c); + return GetByteCount (c, 0, count); } [CLSCompliantAttribute(false)] For the case of GetByteCount (), the GetByteCount (char[],int,int) overload is the only one that needs to be implemented by a subclass. Efficient implementations can implement the unsafe variant and that will avoid the array creation. The MS people likely did this to not break compatibility and allow non-unsafe derived classes of Encoding. All our internal implementations can override the unsafe variant. If there is another issue related to GetByteCount() I'd like to know otherwise we can continue and examine the other buggy cases. Also note that by having private implementations code of Encoding and Encoder/Decoder methods can be shared as well by passing state information to these four methods (and passing zero or initial state when called from the stateless Encoding). Sure, this is a separate issue about the implementation of the unsafe variants and there is no difference related to your design in it. It is likely that the implementation for most complex encodings should be into the encoder classes. We'd have to write a few tests to make sure if it is a requirement. To fix bug 363713 we have to take slower code paths (the same that MS.NET uses) for some methods that will result in buffer copy operations. This bug only affects inherited encodings (but there are a lot of inheritable encoding classes). This is why I introduced the IsInternal property; instances of internal classes can take faster code paths, while instances of inherited classes will take MS compatible (slower) code paths. I decided to I have no objections to a mechanism like IsInternal. implement this code path redirection in Encoding.cs and call the four internal virtual (in fact internally must-implement) unsafe methods. This way the source code of inherited encoding classes will have a clean look because only four methods has to be implemented the other ones will only call the ...Internal implementations in Encoding.cs that does all the work. Well, the issue is that in this case all the work is basically argument checking + a call to the different virtual unsafe method. Argument checking I already showed that having static methods to share the code has the same property of removing duplication and the advantage of making visible what is exactly checked at the point where the data is entering the unsafe user code - trusted runtime code boundary. And when you have implemented the checks in common static methods, the new unsafe virtual calls have basically no need to exist and they become both a runtime burden and an additional issue when auditing the code. Because of fallbacks the actual required byte cound depends on the fallback buffer as well. Also note that UnicodeEncoding in MS.NET 2.0 has support for surrogates and can detect invalid surrogate sequences that means that the fallback buffer will be used to replace invalid input data. If you don't have a single implementation for GetByteCount you will have to duplicate code for strings, char arrays and char pointers as well. So having a single I never proposed having duplicated code for those cases, quite the opposite, so this whole argument of yours is moot. Your proposed solution (that would be just as much change in the infrastructure) is to use static implementation for these four methods in contrast to virtual methods. This would need more code in individual encoding classes: - Argument checking (can be implemented using static methods in Encoding.cs) I don't see how my proposal to have common static methods to do argument checking amounts to code duplication. - IsInternal checks: not much code but can easily go out of sync (some methods require it while others not because the method is overridden in all internal Encoding classes) Not an issue as you say. - calling MS compatible slow implementation (could be a static method in Encoding.cs) Not an issue. The slow code path can go into separate common methods, but this so so trivial
Re: [Mono-dev] [PATCH] Managed Marshal.Copy implemantations
On 03/20/08 Kornél Pál wrote: cases). So you'd need at least another icall that says if the Copy operations are allowed on the array. It's better to have a single icall that does it all, returning 0 for the cases where the copy is not allowed. In that case the name GetElementSize would be wrong. OK, I see the problem. Would it be reasonable to expose internal runtime structures to managed code so that it could read internal array properties rather than relying on internal calls? Yes, we expose them with internal calls:) Eventually we may expose MonoVTable as such, but there is little chance something like MonoClass will be ever exported. Internal calls is the way to go. lupus -- - [EMAIL PROTECTED] debian/rules [EMAIL PROTECTED] Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] [PATCH] Implement internal Encodings using unified code base
On 03/20/08 Kornél Pál wrote: - Unsafe code is required anyway because Encoding has public unsafe methods. Handling strings and character arrays in a single function requires unsafe code as well. Using a single implementation avoids accidental differences between string, array and pointer based public methods. Sure, the existing unsafe methods are perfectly fine, there is no need to add more virtual unsafe methods (this is about 2.0). In 1.1 the unsafe methods would simply become internal. - All argument checks are done in the abstract Encoding.cs and encoding implementations only have to provide the actual implementation. I believe that this is more secure than using different checks is each encoding implementation because they can easily go out of sync. Non-argument as the complex checks in my proposal would go into common code in static methods. - Argument validation could be moved to static methods in Encoding.cs that could result in better security (by not exposing unchecked unsafe code) but Microsoft implementation has bad desingn. There are a lot of unnecessary buffer copy operations (mostly related to string - char[] conversion). To fix https://bugzilla.novell.com/show_bug.cgi?id=363713 while avoiding unecessary buffer copy operations there has to be some internal virtual methods with a check like in IsInternal property. I don't see any need for a different infrastructure to fix that bug, it's simply a bug in our implementation. In 2.0 all the GetByteCount () overloads must call the unsafe overload to do the work. There is no need for additional virtual methods, because the method is there already. Look at UTF8Encoding for GetByteCount(): it does already mostly the right thing (it calls the internal implementation directly instead of going through the virtual unsafe method, trivially fixable). - After a lot of brainstorming I think that having these four virtual unsafe methods provides the solution. (GetStringImpl is required in .NET 1.x only because a bug in MS implementation.) Only single implementation per method per encoding is required, argument checks are shared between encodings, and Ok, let's see the different cases. GetByteCount() can be implemented this way without infrastructure changes. GetCharCount() looks like the same. Same with GetChars (). And I see nothing in the GetBytes() interface that would prevent it. This is all with the assumption that the 363713 bug report suggests that all the overloads are expected to call the unsafe virtual variant for the actual implementation in the end (though it is likely that, for example, GetBytes(char[]) would call GetBytes(char[],int,int), this doesn't change the conclusion). Is this assumption not correct? So please make a concrete example that would require the different infrastructure as I don't see the need (though you're definitely right that our Encoding classes need fixes). Encoding.cs can call faster code paths than MS.NET does without overriding methods not overridden in MS.NET. Concrete examples where this would apply, please. - Also note that I have plans to integrate Encoders/Decoders to this infrastructure by adding an Encoder/Decoder parameter at the end of the argument list of these four ...Impl methods that will let us use Encoder/Decoder as a state store rather than having actual code implementation. Please describe why we'd need this. These goals could only be achieved without these four virtual unsafe methods by adding some more internal virtual (not unsafe) methods and having a lot of duplicate code. This would be much more difficult to maintain so I I don't see where the duplicate code would come from. Currently our implementations has weak fallback support. For example UnicodeEncoding has none while MS.NET has. Eventually we will have to say goodby to count * 2 because that is not compatible with fallbacks. .NET 1.x has no fallback support so there could be some performance gain by avoiding GetByteCountInternal but I think having a centralized implementation is worth to have little performance loss on .NET 1.x because people will eventually use later versions instead. Let's talk with a concrete example and assume that for UnicodeEncoding.GetByteCount() we need to do something more complicated than count * 2. What is the advantage of doing it in a separate function instead of in the actual method? This is the only simple bug fix: - if (charCount 0 || charIndex + charCount s.Length) + if (charCount 0 || charIndex (s.Length - charCount)) If you approve it, I'll commit it. Sure, this bugfix shuld be committed, both in trunk and in the 1.9 branch. Thanks. I tried to defend the infrastructure proposed by me. I see that the one proposed by you is cleaner but I think that it does not address all the issuse that mine does and extending it to address all the issues would Please make a concrete example of a bug that requires an
Re: [Mono-dev] [PATCH] Managed Marshal.Copy implemantations
On 02/22/08 Kornél Pál wrote: If there were something like a fast Array.GetElementSize() that could be used in Marshal.UnsafeAddrOfPinnedArrayElement as well as in System.Buffer methods. System.Buffer is supposed to be used over Array.Copy for better performance so I believe that having a managed System.Buffer would affect overall performance. You have written that GetElementSize() should not be added yet. Is there an ongoing work that would interfere with this or why should that wait? There are two issues. First that is related to a different change that the rest of the patch: it's better to do things incrementally instead of conflating multiple things into the same changeset. Second, GetElementSize () is not the right interface for use inside Buffer. Buffer only applies for a subset of the arrays (basic types and maybe valuetypes of basic types only, I didn't check all the allowed cases). So you'd need at least another icall that says if the Copy operations are allowed on the array. It's better to have a single icall that does it all, returning 0 for the cases where the copy is not allowed. In that case the name GetElementSize would be wrong. lupus -- - [EMAIL PROTECTED] debian/rules [EMAIL PROTECTED] Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] Pending patches
On 03/14/08 Kornél Pál wrote: I didn't receive any comments or approval on the following patches: Sorry for the delay, here are a few quick comments: http://lists.ximian.com/pipermail/mono-devel-list/2008-March/027166.html The changes look fine to me, but someone using windows will have to comment on them. http://lists.ximian.com/pipermail/mono-devel-list/2008-February/027044.html Index: mcs/class/corlib/System.Text/InternalEncoding.cs === --- mcs/class/corlib/System.Text/InternalEncoding.cs (revision 0) +++ mcs/class/corlib/System.Text/InternalEncoding.cs (revision 0) I don't like the introduction of this class and it doesn't seem to have much purpose. It also seems orthogonal to the rest of the changes, so please drop this change from the patch: you should try to introduce one change at a time instead of conflating several different unrelated changes in one patch. Index: mcs/class/corlib/System.Text/UTF8Encoding.cs === --- mcs/class/corlib/System.Text/UTF8Encoding.cs (revision 96483) +++ mcs/class/corlib/System.Text/UTF8Encoding.cs (working copy) @@ -40,6 +40,8 @@ // Magic number used by Windows for UTF-8. internal const int UTF8_CODE_PAGE = 65001; + private static readonly Type internalType = typeof (UTF8Encoding); + There is no need for caching typeof (), it will just waste memory. // Internal state. private bool emitIdentifier; #if !NET_2_0 @@ -73,6 +75,46 @@ windows_code_page = UnicodeEncoding.UNICODE_CODE_PAGE; } + internal override bool IsInternal { + get { return this.GetType () == internalType; } + } + + internal override unsafe int GetByteCountImpl (char* chars, int charCount) + { + char leftOver = '\0'; + return InternalGetByteCount (chars, charCount, ref leftOver, true); I would much prefer to see the argument validation close to the unsafe code and I don't particularly like the use of these internal virtual wrapper methods. The code duplication can be avoided by using simple static validation methods like (only for the cases when more that two lines of checks are needed): static void ValidateGetBytes (...same arguments as GetBytes ()...) { } Basically using virtual+unsafe should be severely limited in our managed assemblies or it's going to increase the cost of security checks significantly. Index: mcs/class/corlib/System.Text/Encoding.cs === --- mcs/class/corlib/System.Text/Encoding.cs (revision 96483) +++ mcs/class/corlib/System.Text/Encoding.cs (working copy) + internal unsafe int GetBytesInternal (string s, int charIndex, int charCount, byte [] bytes, int byteIndex) + { + if (s == null) + throw new ArgumentNullException (s); + if (bytes == null) + throw new ArgumentNullException (bytes); if (charIndex 0 || charIndex s.Length) throw new ArgumentOutOfRangeException (charIndex, _(ArgRange_Array)); - if (charCount 0 || charIndex + charCount s.Length) + if (charCount 0 || charIndex (s.Length - charCount)) These fixes should be in their own separate patch instead of being hidden in a large change. Index: mcs/class/corlib/System.Text/UnicodeEncoding.cs === --- mcs/class/corlib/System.Text/UnicodeEncoding.cs (revision 96483) +++ mcs/class/corlib/System.Text/UnicodeEncoding.cs (working copy) @@ -97,27 +99,19 @@ windows_code_page = UNICODE_CODE_PAGE; } + internal override bool IsInternal { + get { return this.GetType () == internalType; } + } + // Get the number of bytes needed to encode a character buffer. public override int GetByteCount (char[] chars, int index, int count) { - if (chars == null) { - throw new ArgumentNullException (chars); - } - if (index 0 || index chars.Length) { - throw new ArgumentOutOfRangeException (index, _(ArgRange_Array)); - } - if (count 0 || count (chars.Length - index)) { - throw new ArgumentOutOfRangeException (count, _(ArgRange_Array)); - } - return count * 2; + return GetByteCountInternal (chars, index, count); For these simple cases I think that hiding count*2 inside GetByteCountInternal() doesn't help code readability or any other property of the code. This would look much better if it was instead like this: public override int GetByteCount (char[] chars, int index, int count) {
Re: [Mono-dev] GC Question.
On 03/10/08 Bill Holmes wrote: Should I file a bug report on this? This is a limitation of our current JIT/GC, there is no need to file a bug about this. If you move the object creation in it's own method and call that from Main() it should behave as you expect (at least most of the time, our GC is still a conservative one). lupus -- - [EMAIL PROTECTED] debian/rules [EMAIL PROTECTED] Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] [PATCH] [Windows] eglib changes for Windows.
On 03/10/08 Bill Holmes wrote: On Mon, Mar 10, 2008 at 6:52 PM, Bill Holmes [EMAIL PROTECTED] wrote: Please review the attached diff file and let me know if it is OK to commit. It is fixes for a couple of build errors, and bugs with eglib on Windows. Looks fine to me, Please commit. lupus -- - [EMAIL PROTECTED] debian/rules [EMAIL PROTECTED] Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] [patch] partial implementation of System.Reflection.Emit.DynamicILInfo
On 03/07/08 Casey Marshall wrote: The attached patches add a partial implementation of DynamicILInfo -- basically enough that you can specify the IL code and locals for a method, and have that method runnable. I might work on this more if I have time, *plus* I'm not sure if this here is too naive, and would need to change a lot for a full implementation. A few comments below. Index: mono/metadata/object-internals.h === --- mono/metadata/object-internals.h (revision 97635) +++ mono/metadata/object-internals.h (working copy) @@ -1004,6 +1004,15 @@ } MonoReflectionGuidAttribute; typedef struct { +MonoObject object; +MonoArray *code; +gint32 code_len; +gint32 max_stack; +MonoArray *exceptions; +MonoArray *localsig; For newly introduced types, please put all the reference fields at the start. +} MonoReflectionDynamicILInfo; + +typedef struct { MonoObject object; MonoMethod *mhandle; MonoString *name; @@ -1019,8 +1028,10 @@ MonoArray *refs; GSList *referenced_by; MonoReflectionType *owner; +MonoReflectionDynamicILInfo *dynil; Use TABs to indent. Index: mono/metadata/reflection.c === --- mono/metadata/reflection.c(revision 97635) +++ mono/metadata/reflection.c(working copy) @@ -1454,7 +1455,7 @@ rmb-attrs = mb-attrs; rmb-iattrs = 0; rmb-call_conv = mb-call_conv; - rmb-code = NULL; +rmb-code = NULL; Your patch introduces whitespace damage. @@ -8682,6 +8684,16 @@ num_locals = rmb-ilgen-locals ? mono_array_length (rmb-ilgen-locals) : 0; if (rmb-ilgen-ex_handlers) num_clauses = method_count_clauses (rmb-ilgen); + } else if (rmb-dynil) { + char *ptr = mono_array_addr (rmb-dynil-localsig, guint8, 0); + code = mono_array_addr (rmb-dynil-code, guint8, 0); + code_size = rmb-dynil-code_len; + max_stack = rmb-dynil-max_stack; + if (*ptr == 0x07) + { Put the opening brace at the end of the previous line. + if (rmb-dynil) + { + const char *ptr = mono_array_addr (rmb-dynil-localsig, guint8, 0); + ptr += 2; + for (i = 0; i num_locals; i++) + { + MonoType *t = mono_metadata_parse_type_full(NULL, NULL, MONO_PARSE_LOCAL, + 0, ptr, ptr); Unfortunately it isn't this simple: the above will work only for very simple types. Extensive testing may be needed to see which tokens are assigned for user types. Index: class/corlib/System.Reflection.Emit/DynamicMethod.cs === --- class/corlib/System.Reflection.Emit/DynamicMethod.cs (revision 97638) +++ class/corlib/System.Reflection.Emit/DynamicMethod.cs (working copy) @@ -61,6 +61,7 @@ private object[] refs; private IntPtr referenced_by; private Type owner; +private DynamicILInfo dynil; Whitespace damage here as well. It would be nice if you added also the tests for the new functionality. This is good for a start, though. Note that for runtime changes for us to be able to accept them you need to explicitly license them under the MIT/X11 license. Thanks! lupus -- - [EMAIL PROTECTED] debian/rules [EMAIL PROTECTED] Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] New code to build interface vtables
On 01/29/08 Massimiliano Mantione wrote: Anyway, the patch passes everything here. Did you run the corlib tests, too? --- mono/metadata/class.c (revision 94247) +++ mono/metadata/class.c (working copy) +#if USE_NEW_INTERFACE_VTABLE_CODE + // Loop on all implemented interfaces... + for (i = 0; i class-interface_offsets_count; i++) { + MonoClass *parent = class-parent; + int ic_offset; + gboolean interface_is_explicitly_implemented_by_class; + int im_index; + + ic = class-interfaces_packed [i]; + ic_offset = mono_class_interface_offset (class, ic); + + // Check if this interface is explicitly implemented (instead of just inherited) + if (parent != NULL) { + int implemented_interfaces_index; + interface_is_explicitly_implemented_by_class = FALSE; + for (implemented_interfaces_index = 0; implemented_interfaces_index class-interface_count; implemented_interfaces_index++) { + if (ic == class-interfaces [implemented_interfaces_index]) { + interface_is_explicitly_implemented_by_class = TRUE; + break; + } + } You likely need to loop other all the hierarchy here, right? Because this is not restricted to just the immediate parent. + } else { + interface_is_explicitly_implemented_by_class = TRUE; + } + + // Loop on all interface methods... + for (im_index = 0; im_index ic-method.count; im_index++) { + MonoMethod *im = ic-methods [im_index]; + int im_slot = ic_offset + im-slot; + MonoMethod *override_im = (override_map != NULL) ? g_hash_table_lookup (override_map, im) : NULL; + + // If there is an explicit implementation, just use it right away, + // otherwise look for a matching method + if (override_im == NULL) { + int cm_index; + + // First look for a suitable method among the class methods + for (cm_index = 0; cm_index class-method.count; cm_index++) { + MonoMethod *cm = class-methods [cm_index]; + + TRACE_INTERFACE_VTABLE (printf ( For slot %d ('%s'.'%s':'%s'), trying method '%s'.'%s':'%s'... [EXPLICIT IMPLEMENTATION = %d][SLOT IS NULL = %d], im_slot, ic-name_space, ic-name, im-name, cm-klass-name_space, cm-klass-name, cm-name, interface_is_explicitly_implemented_by_class, (vtable [im_slot] == NULL))); + if ((cm-flags METHOD_ATTRIBUTE_VIRTUAL) check_interface_method_override (class, im, cm, TRUE, interface_is_explicitly_implemented_by_class, (vtable [im_slot] == NULL), security_enabled)) { + TRACE_INTERFACE_VTABLE (printf ([check ok]: ASSIGNING)); + vtable [im_slot] = cm; + /* Why do we need this? */ + if (vtable [im_slot]-slot 0) { + vtable [im_slot]-slot = im_slot; + } It's much simpler to use: if (cm-slot 0) cm-slot = im_slot; + } + TRACE_INTERFACE_VTABLE (printf (\n)); + } + + // If the slot is still empty, look in all the inherited virtual methods... + if ((vtable [im_slot] == NULL) class-parent != NULL) { + MonoClass *parent = class-parent; I think you need to loop over all the parents here. Please write the appropriate test cases where the interface method is implemented by a non-immediate parent. + // Reverse order, so that last added methods are preferred + for (cm_index = parent-vtable_size - 1; cm_index = 0; cm_index--) { + MonoMethod *cm = parent-vtable [cm_index]; + + TRACE_INTERFACE_VTABLE ((cm != NULL) printf (For slot %d ('%s'.'%s':'%s'), trying (ancestor) method '%s'.'%s':'%s'... , im_slot, ic-name_space, ic-name, im-name,
Re: [Mono-dev] Control-C handler
On 01/28/08 Jonathan Pryor wrote: On Mon, 2008-01-28 at 17:13 -0500, Jonathan Pryor wrote: On Mon, 2008-01-28 at 21:10 +0100, Paolo Molaro wrote: Deregistration is handled incorrectly: if there are two handlers for the same signal it gets disabled at the first uninstall. This has been fixed. Thanks, this looks much better. So the only remaining question, to me, is the type and ordering of the UnixSignal.WaitAny() methods (i.e. the 'params' question). I'm not sure why you think it is a problem: there used to be a bug in mcs when the params array wasn't the last argument, but I think it has been fixed, you may want to check that. You should write a test case to stress-test this code and see if it behaves correctly under a storm of signals, say at least 100k signals. You should check that no signal was lost. Repeat the same while one thread adds/removes handlers. Repeat with two threads doing a WaitAny() and using more than one signal. Repeat with two threads calling kill() to send the signal. Do all of this on a SMP box. The code is ok for HEAD, once it passes the above stress tests it can go also into the 1.9 branch, IMO (what should go right away in the branch it the Obsolete attribute for signal()). Thanks! lupus -- - [EMAIL PROTECTED] debian/rules [EMAIL PROTECTED] Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] Control-C handler
On 01/24/08 Jonathan Pryor wrote: After talking on IRC, the sane interface is to provide Stdlib.signal_default(), Stdlib.signal_error(), and Stdlib.signal_ignore() methods instead of using Stdlib.signal() with Well, those methods shouldn't be in that namespace and the names are not really nice. Ignore maps well to a static Ignore (int signal) method but I can't come up with a nice name for the other two cases. An alternative is: enum SignalAction { Default, Ignore, Error } and static int SetSignalAction (int signum, SignalAction action); Index: Mono.Unix/UnixSignal.cs === --- Mono.Unix/UnixSignal.cs (revision 0) +++ Mono.Unix/UnixSignal.cs (revision 0) +using System; +using System.Runtime.InteropServices; +using System.Threading; + +using Mono.Unix.Native; + +namespace Mono.Unix { + public class UnixSignal : WaitHandle { It should have a finalizer. + private Signum signum; + private int _signum; + private IntPtr signal_info; + + public UnixSignal (Signum signum) + { + this.signum = signum; + this._signum = NativeConvert.FromSignum (signum); There is no need to store reduntant info. + private void AssertValid () + { + if (signal_info == IntPtr.Zero) + throw new ObjectDisposedException (GetType().FullName); + } + + private unsafe SignalInfo* Info { + get {return (SignalInfo*) signal_info;} + } + + public bool IsSet { + get { + AssertValid (); + return Count 0; + } + } + + public unsafe bool Reset () + { + AssertValid (); + int n = Info-count; + Info-count = 0; + return n != 0; + } + + public unsafe int Count { + get {return Info-count;} + set {Info-count = value;} Crashes when Info is NULL. + #region WaitHandle overrides + protected unsafe override void Dispose (bool disposing) + { + if (signal_info == IntPtr.Zero) + return; + uninstall (_signum); + signal_info = IntPtr.Zero; + } This doesn't implement the documented Dispose pattern. Index: signal.c === --- signal.c (revision 92060) +++ signal.c (working copy) @@ -3,18 +3,29 @@ * * Authors: * Jonathan Pryor ([EMAIL PROTECTED]) + * Jonathan Pryor ([EMAIL PROTECTED]) * * Copyright (C) 2004-2005 Jonathan Pryor + * Copyright (C) 2008 Novell, Inc. */ #include signal.h +#ifndef PLATFORM_WIN32 +#include unistd.h +#include stdlib.h +#include string.h +#include poll.h As reported already the last time, you can't unconditionally use poll. @@ -49,6 +61,158 @@ psignal (sig, s); return errno == 0 ? 0 : -1; } + +static signal_info signals[64]; + +static inline signal_info* +get_info (int signum) +{ + if (signum 0 || signum = sizeof(signals)/sizeof(signals [0])) + return NULL; + return signals [signum]; +} + +static void +default_handler (int signum) +{ + signal_info* h = get_info (signum); + int fd = 0; + if (h) { + ++h-count; + if (h-write_fd 0) + fd = h-write_fd; + } + if (fd) { + char c = signum; + write (fd, c, 1); + } +} + +static pthread_mutex_t signals_mutex = PTHREAD_MUTEX_INITIALIZER; + +void* +Mono_Unix_UnixSignal_install (int sig) +{ + int mr; + signal_info* h; + + mr = pthread_mutex_lock (signals_mutex); + if (mr != 0) { + errno = mr; + return NULL; + } + + h = get_info (sig); + if (h == NULL) + errno = EINVAL; + else if (h-have_handler) + errno = EADDRINUSE; + else { + h-count = 0; + h-handler = signal (sig, default_handler); + if (h-handler == SIG_ERR) { + h-handler = NULL; + } + else + h-have_handler = 1; + } + + pthread_mutex_unlock (signals_mutex); + + return h; +} + +int +Mono_Unix_UnixSignal_uninstall (int sig) It is important (as in my initialy API sketch) that this function take the signal_info and not the dignal number, as this implementation allows only just an handler per signal in the API. lupus --
Re: [Mono-dev] Control-C handler
On 01/28/08 Jonathan Pryor wrote: It is important (as in my initialy API sketch) that this function take the signal_info and not the dignal number, as this implementation allows only just an handler per signal in the API. This has been corrected. We now support up to 64 signal handlers within a process, and the same signal can be registered multiple times; each such registration gets a different index, and is thus independent. Deregistration is handled incorrectly: if there are two handlers for the same signal it gets disabled at the first uninstall. The only other major change is the addition of a UnixSignal.WaitAny(UnixSignal[]) method, which allows waiting on more than one handle (since WaitHandle.WaitAny() can't currently be used with WaitHandle subclasses like UnixSignal). I suggest using params in the array argument. +int +Mono_Unix_UnixSignal_WaitAny (void** _signals, int count, int timeout /* milliseconds */) +{ + fd_set read_fds; + int mr, r; + int max_fd = 0; + + signal_info** signals = (signal_info**) _signals; + + mr = pthread_mutex_lock (signals_mutex); + if (mr != 0) { + errno = mr; + return -1; You don't unlock in the return path. An extensive test program that shows this stuff works before being committed would also help. Thanks. lupus -- - [EMAIL PROTECTED] debian/rules [EMAIL PROTECTED] Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] BSTR Marshalling in Mono
On 01/08/08 Jonathan Chambers wrote: Here is the first attempt at a patch. Nothing is changed with the default configuration. Things are controlled via an environment variable (MONO_COM), and extensible using the dllmap in the config file. The only other current configuration is MONO_MS, which supports Mainsoft COM components. If this looks acceptable, I will cleanup any issues and document the environment variable. Since we hardcode the functions, I wonder if this should really be configurable. Index: metadata/loader.h === --- metadata/loader.h (revision 92458) +++ metadata/loader.h (working copy) @@ -62,6 +62,9 @@ void mono_dllmap_insert (MonoImage *assembly, const char *dll, const char *func, const char *tdll, const char *tfunc); +int +mono_dllmap_lookup (MonoImage *assembly, const char *dll, const char* func, const char **rdll, const char **rfunc); IIF we expose this, it should be a MONO_INTERNAL function in a private header. Index: metadata/marshal.c === --- metadata/marshal.c(revision 92458) +++ metadata/marshal.c(working copy) mono_string_to_bstr (MonoString *string_obj) { @@ -1082,19 +1094,60 @@ return NULL; return SysAllocStringLen (mono_string_chars (string_obj), mono_string_length (string_obj)); #else - int slen = mono_string_length (string_obj); - char *ret = g_malloc (slen * 2 + 4 + 2); - if (ret == NULL) - return NULL; - memcpy (ret + 4, mono_string_chars (string_obj), slen * 2); - * ((guint32 *) ret) = slen * 2; - ret [4 + slen * 2] = 0; - ret [5 + slen * 2] = 0; + if (com_provider == MONO_COM_DEFAULT) { + int slen = mono_string_length (string_obj); + char *ret = g_malloc (slen * 2 + 4 + 2); + if (ret == NULL) + return NULL; + memcpy (ret + 4, mono_string_chars (string_obj), slen * 2); + * ((guint32 *) ret) = slen; + ret [4 + slen * 2] = 0; + ret [5 + slen * 2] = 0; - return ret + 4; + return ret + 4; + } + else if (com_provider == MONO_COM_MS) { The else goes into the same line as the closing }. + char *error_msg; + gpointer ret = NULL; + gunichar* str = NULL; + guint32 len; + MonoDl *module = NULL; + SysAllocStringLenFunc pSysAllocString = NULL; + const char* scope = liboleaut32.so; + const char* import = SysAllocStringLen; + const char* new_scope = NULL; + const char* new_import = NULL; + if (mono_dllmap_lookup (mono_defaults.corlib, scope, import, new_scope, new_import)) { + scope = new_scope; + import = new_import; + } + module = mono_dl_open(scope, MONO_DL_LAZY, error_msg); + if (error_msg) { + g_warning (Error loading COM support library '%s': %s, scope, error_msg); + g_assert_not_reached (); + return NULL; + } + error_msg = mono_dl_symbol (module, import, (gpointer*)pSysAllocString); + if (error_msg) { + g_warning (Error loading entry point '%s' in COM support library '%s': %s, import, scope, error_msg); + g_assert_not_reached (); + return NULL; + } Please put this code into its own function and share it with SysStringLen and SysFreeString: the tree lookups should be done at the same time. Thanks! lupus -- - [EMAIL PROTECTED] debian/rules [EMAIL PROTECTED] Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] Control-C handler
On 01/02/08 Jonathan Pryor wrote: That said, I think that there should be a way to use signals from C#. Even if such use is severely limited -- e.g. only setting a volatile variable within the signal handler -- signals are required for decent integration with Unix platforms, so saying don't use them isn't an adequate answer. We need to have a well defined, supported, set of things that signal handlers can do. We can't arbitrarily add gurantees: only things defined in the pthread and posix standards are allowed in a signal context, we can handwave as much as you want, but those are the limitations. And I didn't say you can't use signals in a mono app, you can (by using C signal handlers). The alternative -- requiring that all signal handlers be written in C -- isn't as useful. It would require that every Gnome Gtk# app (Tomboy, Beagle, etc) have a C library to properly handle session ending (SIGTERM followed by SIGKILL), which just sounds like severe overkill. No, it would simply require that Mono.Posix provide a sane interface instead of the current broken one. I'll propose one below. So why can't setting a volatile variable within a signal handler be supported? And/or use a Constrained Execution Region to *ensure* that the signal handler is JITed before the signal is emitted (does mono even support CERs yet?)? The issue is not in the setting of a volatile variable, it is in the need to execute managed code. CERs have no relevance to this issue. As you may not be familiar with the details, I'll explain them. Installing a C# signal handler means creating a delegate and then marshaling it to unmanaged code as a function pointer. The function pointer then enters managed code and executes the delegate. The first problem is that all this code needs to be already compiled to native code as running the jit in the signal handler breaks all the signal context limitations. In particular this means that both the target method and the delegate Invoke function itself are already compiled. In addition to this, any runtime service that they may use must be signal context safe. At the minimum this requires that the same delegate object instance that is marshaled is actually executed (as this involves a number of delegate invoke optimizazions in the runtime that we won't make signal context safe as it doesn't make sense). After this of course the delegate method itself must be very simple. I'll ignore multicast delegate issues. Once you start to realize all these restrictions you know that there is no way that you can make sure that any average C# programmer follows half of them. The existing Mono.Posix signal interface just tricks people into thinking that they can do this safely, while they can't. But let's assume that all the above is doable (and making a runtime guarantee that it is would also mean that the runtime will be severily constrained in the changes it can do for a lot of implementation details that actually matter for performance like delegate invocation). The next thing to consider is the code that the callback needs to execute. The first thing it does is to ensure the application domain for the thread where the signal handler executes actually matches the appdomain the delegate comes from, if it isn't it may need to set it and do several things that we can't guarantee are signal context safe (like the recently discussed deserialization of the culture info). If the delegate is of an instance method we also need to retrieve the object reference, which may require taking locks (for a non-moving collector we'll need a handle as we can't embed the object address in the native code stream). Given the above, there is basically no chance that anyone could correctly install a signal handler and if it was possible it would constrain and likely slow down many parts of the mono runtime. All of this can be easily overcome with a sane interface for signals provided by Mono.Posix (the implementation can be in either the helper lib or in the runtime). This would export a class like (pseudo code): class UnixSignalHandler { IntPtr flag_var_ptr; pulic UnixSignalHandler (int signal) { flag_var_ptr = Install (signal); if (flag_var_ptr == null) throw new Exception (); } ~UnixSignalHandler () { Uninstall (signal, flag_var_ptr); } public bool IsSignalled { get { if (*(int*)flag_var_ptr) { *(int*)flag_var_ptr = 0; return true; } return false; } } } Install is implemented in C and will reserve a slot in an array for storing a variable which is set in
Re: [Mono-dev] set culture uses serialization?
On 01/06/08 Robert Jordan wrote: Robert Jordan wrote: Since the current handling of CurrentThread.CultureInfo isn't quite perfect (changes of CurrentThread.CultureInfo's object state isn't reflected back into another app domains, like on MS.NET), I wonder why we don't use a more efficient way: cache the LCID instead of the full blown binary representation of the CultureInfo. Hmm, this is too restrictive, as it would miss changes of the CurrentThread.DateTimeFormat CurrentThread.NumberFormat objects, but it could be still used as an optimization for the case that those objects were not changed. This should be the most probable case anyways. We could see if it's possible to delay the serialization at the appdomain switch time, so setting the culture would be cheap (switching appdomain would be more costly, though). Someone needs to write more extensive tests and check if this matches the semantics of .net. lupus -- - [EMAIL PROTECTED] debian/rules [EMAIL PROTECTED] Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] Control-C handler
On 12/21/07 pablosantosluac wrote: Ok, I think I got a bit lost here... how should I proceed then? There is no guarantee that you can hook to signal handlers from C#, we don't support that usage and its inclusion in Mono.Posix is a mistake. I'm actually going to add code to mono to detect such unsupported uses so we can flag error reports from applications that hook signal handlers as unusable and we can discard them. lupus -- - [EMAIL PROTECTED] debian/rules [EMAIL PROTECTED] Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] mono/mini/driver.c patch for RHEL3 compatability
On 12/19/07 C.J. Adams-Collier wrote: Zoltan asked that I make a change to configure.in as well, but I wanted to send this before I forgot about it. The configure.in change is what is more important as you need to make sure you won't break the correct systems in your quest for supporting an old and broken distribution. --- mono/mini/driver.c 2007-12-19 15:04:53.0 -0800 +++ mono/mini/patched-driver.c 2007-12-19 15:05:20.0 -0800 @@ -706,8 +706,16 @@ #if HAVE_SCHED_SETAFFINITY if (getenv (MONO_NO_SMP)) { +# ifdef GLIBC_RHEL3_SCHED_SETAFFINITY Use HAVE_BROKEN_SCHED_SETAFFINITY here. lupus -- - [EMAIL PROTECTED] debian/rules [EMAIL PROTECTED] Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] 1.2.6: threads.h
On 12/14/07 Sebastian Good wrote: I am compiling an embedding library which #includes mono/metadata/threads.h, however in 1.2.6 this file does not compile cleanly. threads.h apparently further includes mono/metadata/threads-types.h, which is no longer shipped with mono. Commenting out lines 70-72 (which reference the MonoThreadState class) solves the problem. Trivial programs seem to run fine, though I am not sure if there are any further knock-on effects. (I am currently compiling with the Windows version of Mono 1.2.6) This is fixed in svn: in the mean time just remove the 2-3 functions from your copy of the threads.h header file. lupus -- - [EMAIL PROTECTED] debian/rules [EMAIL PROTECTED] Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] [PATCH] Managed Marshal.Copy implemantations
On 12/17/07 Kornél Pál wrote: (for example the original C implementation has a bug and in your code it's duplicated several times) What is that bug? Integer overflow when doing the array range check. and you also managed to hide the assignment of a local var inside of a conditional expression. Is that a bad practice? Yes, there is no reason to try to hide code. Your code also omits a check that the current code does of the rank of the array. I don't see any reason for that check because as far as I know byte[,] for example cannot be casted to byte[]. Did I miss something? It's an extra check because the shared methods will have Array as the type and they could be misused. My only problem is that I don't know how could I effectively obtain a pinned pointer to a managed object (currently Array) using C#. Do you have a good idea? Until we support precise stack scanning in the GC it's not an issue and you can use a simple IntPtr. When we'll add that support, the JIT will be changed to insert an explicit pinning pointer local variable to store the result of the new internal call. lupus -- - [EMAIL PROTECTED] debian/rules [EMAIL PROTECTED] Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] mono/mini/driver.c patch for RHEL3 compatability
On 12/20/07 C S Vadiraj wrote: --- mono/mini/driver.c 2007-12-19 15:04:53.0 -0800 +++ mono/mini/patched-driver.c 2007-12-19 15:05:20.0 -0800 @@ -706,8 +706,16 @@ #if HAVE_SCHED_SETAFFINITY if (getenv (MONO_NO_SMP)) { +# ifdef GLIBC_RHEL3_SCHED_SETAFFINITY +cpu_set_t proc_mask; +CPU_ZERO(proc_mask); +CPU_SET(0, proc_mask); + +sched_setaffinity (getpid(), proc_mask); You sure sched_setaffinity takes only two parameters. I feel the issue is not in here but in glibc. Older Red Hat shipped with a broken sched_setaffinity() interface. lupus -- - [EMAIL PROTECTED] debian/rules [EMAIL PROTECTED] Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] Static Bundling
On 12/18/07 Avery Pennarun wrote: I've been looking at using mkbundle myself for my own work. It's network stuff, not graphical, so I wouldn't expect gdiplus to be a problem for me. But what exactly is that case that it can't cope with? Dynamically-linked elf libraries? Could I just rebuild with static libs instead? mkbundle solves the needs of 99% of the people that need it's capabilities. It doesn't solve all the worlds problems, so you can't expect it to. From the manpage: mkbundle generates an executable program that will contain static copies of the assemblies listed on the command line. And it does exactly what it's documented to do. If you need additional capabilities you can roll your own, like linking the libs to your program yourself and redirecting the p/invoke calls as explained in man mono-config. Also, let me take this occasion to remind everyone that statically bundling the libmono library and any other LGPL library requires (if you distribute the binary) that you also distribute all the stuff that is needed by the user to relink your code to a different version of the library (this is just one of the ways to comply with the license, see the LGPL for more details). lupus -- - [EMAIL PROTECTED] debian/rules [EMAIL PROTECTED] Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] Is it possible to run ASP.NET from a CD?
On 12/19/07 Joe Audette wrote: Was wondering if its theoretically possible to run xsp2 directly from a cd. I've seen some posts about embedding mono but don't really know what is involved. If anyone can confirm whether its possible or point me to any information that might help I'd appreciate it. What I'm thinking about is being able to run mojoportal directly from a cd probably with SQLite. Obviously it would be read only but I can see if it could be done it would be great for making demos and also maybe I could make a utility to import export from other db platforms so a site could be backed up and archived to cd in a way that you can see the site as it was at the time of backup. You basically always need a ram disk, so if you setup the proper links (or mount points: there is a filesystem that allows you to view a ro filesystem as rw) you can easily do it. lupus -- - [EMAIL PROTECTED] debian/rules [EMAIL PROTECTED] Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] BSTR Marshalling in Mono
On 12/19/07 Jonathan Chambers wrote: COM Interop support in mono works pretty well for most basic uses, but has some limitations when it comes to strings. Mainly, BSTR marshalling on non-Windows platforms is just a default implementation. The problem is that most COM systems (both Mainsoft's COM and Mozilla's XPCOM) have specific requirements on strings. They need to follow a certain format (usually a length prefixed string), use special allocators/deallocators, and use the correct byte size (2-byte vs. 4-byte encoding). [...] 1. Expose some methods in the runtime so users could embed mono and adjust the BSTR marshalling behavior with callbacks. Something like mono_set_bstr_to_string_marshal, mono_set_bstr_from_string_marshal, mono_set_bstr_free. This would require users to embed mono. 2. Use the dll map in the config file to let the user specify entry points to perform BSTR marshalling. This seems a better choice than the first. There is then a technical question as to how to implement this. The second solution would be better as it doesn't force embedding mono. For this new feature, though, we'd need to be able to handle both xpcom and COM at the same time, so a global setting wouldn't be enough. Is there a way we could use to distinguish the two cases at the time we're emitting the marshaling code? As for the implementation, we could load a shared library that implements the needed bstr methods (this is indeed how I planned at the time the COM support would work), but these shared libs would need to be separate from mono, as we don't want the mono build to have to depend on xulrunner or com dev libs to be installed. Or we could see if it's possible to access the stuff we need with dlopen/dlsym directly on the xpcom/com libs and use that inside the runtime: this ahs the advantage that there is no build or runtime dependency (unless the feature is actually used at runtime) and the small drawback that the two systems are hardcoded (though we'd hope few other com-like systems will be developed). lupus -- - [EMAIL PROTECTED] debian/rules [EMAIL PROTECTED] Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] [PATCH] Managed Marshal.Copy implemantations
On 12/17/07 Kornél Pál wrote: Can I commit this patch or should Marshal.Copy remain unmanaged or do you have any other objections? I'm not opposed to have this stuff implemented in managed code, but I don't like the specific way it has been done as it involves lots of duplicate code (for example the original C implementation has a bug and in your code it's duplicated several times) and you also managed to hide the assignment of a local var inside of a conditional expression. Your code also omits a check that the current code does of the rank of the array. I would support a change that reimplemented in managed code copy_to_unmanaged()/copy_from_unmanaged(), maybe passing the array element size as an additional argument. To do this you just need an additional internal call that returns a IntPtr to the start of the elements given an Array (the JIT can recognize this icall and turn it into an addition of a constant). lupus -- - [EMAIL PROTECTED] debian/rules [EMAIL PROTECTED] Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] runtime dbg library API proposal/impl
On 12/05/07 Rodrigo Kumpera wrote: One thing missing are functions to probe the version/capabilities of the library. Since the idea is to have the debugger use as much of the dbg api as possible but falling back to the current code in case the lib doesn't support specific stuff. This is automatically handled by DllImport. Also note that once the API is fixed, we'll just add to it: the existence of an API entry point is trivial to check for. We could add a major version number to be able to do some extra sanity check, but it will basically not change very often if at all: one of the things we're trying to address with this library is breakages in the debugger as a consequence of runtime internal changes and we don't want to add one more potential unstable API to make things worse. lupus -- - [EMAIL PROTECTED] debian/rules [EMAIL PROTECTED] Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] runtime dbg library API proposal/impl
On 12/05/07 Martin Baulig wrote: Maybe it would also be a good idea to use the existing debugger to test this rather than having just for debugging purposed something which is 100% broken and has to be removed later anyways. Many of the functions are there exatly because this way we can test the library as part of the runtime regression test runs: we can't require the debugger to be built and running at that point. lupus -- - [EMAIL PROTECTED] debian/rules [EMAIL PROTECTED] Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] runtime dbg library API proposal/impl
On 12/05/07 Martin Baulig wrote: There is just one important thing to keep in mind: public TargetAddress MonoClassGetMonoImage (ITargetMemoryAccess memory, TargetAddress klass) { IntPtr image = mono_debugger_support_mono_class_get_image ( memory.ReadMemory, klass.Address); return new TargetAddress (image); } You have to use the `memory' instance to read memory (we may add other methods to it if necessary) - but you do not own it. It is absolutely forbidden to store this anywhere or use it after the method returned control back to the debugger. Can you elaborate why you have this requirement? When you're debugging a target process it's either alive and you can use ptrace or access /proc/pid/mem or it's dead and you access the core file: why would you want to switch to one mechanism or the other on the fly? lupus -- - [EMAIL PROTECTED] debian/rules [EMAIL PROTECTED] Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
[Mono-dev] runtime dbg library API proposal/impl
Hi. The attached patch implements part of the debugging library the runtime will provide for use in the debugger, to reduce the dependencies the debugger has on the runtime internals. Note this is very basic for now and also includes stuff the debugger won't use, but that will be essential to allow testing the library during the runtime build/test cycle. I'd like advice especially for the case of handling threads: should we have a mono_dbg_target_set_thread (target, thread_id) interface and set/get_context calls without the additional thread_id argument or use something like in the header file below? I also intend to make stop/continue work on individual threads, keeping the existing interface for stopping all the threads. By tomorrow I'll try to expand the API to include helper functions to access some of the runtime data structures (like MonoClass' name, namespace, parent fields etc). Also, as discussed with Martin, the read/write word functions will be later changed to optionally use a callback instead of using ptrace directly (so that the debugger could provide a callback that reads data from core files, for example). It would be also good if people familiar with windows debugging could make sure the API is implementable there as well. Comments welcome. Thanks! lupus -- - [EMAIL PROTECTED] debian/rules [EMAIL PROTECTED] Monkeys do it better Index: mono-dbg.c === --- mono-dbg.c (revision 0) +++ mono-dbg.c (revision 0) @@ -0,0 +1,210 @@ + +#include mono-dbg-internals.h + +/** + * mono_dbg_target_new: + * @process_id: pid for unix systems or process id on windows + * @flags: flags (currently none) + * + * Create and initialize the structure needed to be able to debug + * the process identified by @process_id. + * + * Returns: a pointer to the target structure or #NULL on error. + */ +MonoDbgTarget* +mono_dbg_target_new (void *process_id, MonoDbgTargetFlags flags) +{ + MonoDbgTarget* target = malloc (sizeof (MonoDbgTarget)); + target-flags = flags; +#ifdef PLATFORM_WIN32 + target-process_id = process_id; + target-thread_id = NULL; /* FIXME */ +#else + target-process_id = (pid_t)process_id; + target-thread_id = target-process_id; +#endif + + return target; +} + +/** + * mono_dbg_target_free: + * @target: a target structure pointer returned from mono_dbg_target_new () + * + * Free all the resources associated with debugging the given target process. + */ +void +mono_dbg_target_free (MonoDbgTarget *target) +{ + free (target); +} + +/** + * mono_dbg_target_attach: + * @target: + * + * Start debugging the process specified by @target. On successfull + * return the process will be stopped. + * + * Returns: 0 on success. + */ +int +mono_dbg_target_attach (MonoDbgTarget *target) +{ +#ifdef PLATFORM_WIN32 + return -1; +#else + int status; + pid_t pid; + int result = ptrace (PTRACE_ATTACH, target-process_id, NULL, NULL); + if (result 0) { + return result; + } + /* wait for the target to stop */ + pid = wait (status); + return 0; +#endif +} + +/** + * mono_dbg_target_detach: + * @target: + * + * Stop debugging the current process. + * + * Returns: 0 on success. + */ +int +mono_dbg_target_detach (MonoDbgTarget *target) +{ +#ifdef PLATFORM_WIN32 + return -1; +#else + int result = ptrace (PTRACE_DETACH, target-process_id, NULL, NULL); + return result; +#endif +} + +/** + * mono_dbg_target_continue: + * @target: + * + * Cause the target process to continue execution. + * + * Returns: 0 on success. + */ +int +mono_dbg_target_continue (MonoDbgTarget *target) +{ +#ifdef PLATFORM_WIN32 + return -1; +#else + int result = ptrace (PTRACE_CONT, target-process_id, NULL, NULL); + return result; +#endif +} + +/** + * mono_dbg_target_stop: + * @target: + * + * Cause the target process to stop execution. + * + * Returns: 0 on success. + */ +int +mono_dbg_target_stop (MonoDbgTarget *target) +{ +#ifdef PLATFORM_WIN32 + return -1; +#else + int status; + kill (target-process_id, SIGINT); + wait (status); + return 0; +#endif +} + +/** + * mono_dbg_target_kill: + * @target: + * + * Terminate the target process. + * + * Returns: 0 on success. + */ +int +mono_dbg_target_kill (MonoDbgTarget *target) +{ +#ifdef PLATFORM_WIN32 + return -1; +#else + int result = ptrace (PTRACE_KILL, target-process_id, NULL, NULL); + return result; +#endif +} + +/** + * mono_dbg_target_read: + * @target: + * @address: memory address in the target process + * @out_data: pointer to the memory location where the read word is stored + * + * Read a pointer-sized word from the target process at address @address. + * @address should be pointer-size aligned. + * The data is written through the pointer @out_data. + * + * Returns: 0 on success. + */ +int +mono_dbg_target_read (MonoDbgTarget *target, void *address, void **out_data) +{ +#ifdef PLATFORM_WIN32 + return -1;
Re: [Mono-dev] String.GetHashCode()
On 12/01/07 Robert Jordan wrote: Since strings are immutable, shouldn't it be possible to compute the hashcode once and store it rather than recomputing it over and over again? Is there some really obvious reason that i can't think of that would make this a bad idea? This idea already came up a couple of years ago (2002 or so). [...] Therefore, the optimization should consider the string length and append the hash code after the string's bytes only if its length is greater than some amount (1K or so). I'll reply to this mail and try to summarize all the responses and the constraints we need to obey. 1) we can't change the C-side MonoString struct, because it is part of the Mono ABI 2) we should try to not increase memory usage 3) we should try to not slowdown the case were the hashcode is calculated for the first time on a string instance 4) we can't use utf8 to hold string chars because this would break the mscorlib/CLI ABI (see how fixed on a string is compiled by the C# compiler, for example) 5) GetHashCode should never be called for a string that is not yet fully built (like in StringBuilder), so there is no worry aout the string changing after the hash code field has been set Point 1 means that if we were to add a new field to the string it can only be added at the end, after the character data (and the terminating null char). This also means that if we use an int, we have to be careful and properly align it (this also means more instructions to access this field). About memory usage: objects in Mono are always aligned on 8 byte boundaries so in some cases the overhead is 50%, 33%, 25%, 20% (for strings up to 15 chars) and sometimes it is 0. I would say that, as a mean, the memory overhead is 15% for up to 20 chars, 5% for up to 100 chars and it doesn't matter above that. After having written the moving GC, where fast computing of an object's size is important, I would also try to avoid allocating the extra field only for big strings (this would also slowdown string allocation somewhat). So, given the above constraints and considerations, I suggest the following: 1) wait for the moving GC to become the default: in that case we can use the same mechanism used to store object hash codes in the object header: this means no extra memory usage for strings and just a small slowdown when inserting the hashcode in the header (it needs a locked op). Implementing this mechanism with the current GC would slowdown a bit some common operations (lock/unlock), with the moving GC we would take the hit anyway. 2) in alternative, use a 16 bit hash code field: the memory overhead is smaller (5-10% for up to 20 chars, 2-3% for up to 100 chars on average), it requires no alignment calculation so it's fast to access. The drawback is that for very big hashtables ( 100 K entries) we'd have few bits. A cheap way to increase bits would be to or the hash with the first char shifted left 16 bits (but this wouldn't solve the issue for strings all beginning with the same char). I'm sure this drawback is not very significant, but I'm also sure someone will write a specific benchmark to exploit this weakness and whine in some blog or on the list about it:) 2 is very simple to implement so we could easily get some numbers (hint, hint). lupus -- - [EMAIL PROTECTED] debian/rules [EMAIL PROTECTED] Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] Profiler and coverage problem?
On 11/30/07 Csaba Balazs wrote: class ParentClass { private int tval = 1; public int PValue { get { return 2*tval; } } } [...] It says, the ParentClass.PValue (get_PValue) is not used, but we can see its value on the screen. Why isn't it covered? I would like it to be in the COV file. The method gets inlined. You can get the coverage info for it by running mono with inlining disabled: mono -O=-inline program.exe. I should likely change the coverage profiler to force inlining off so the results are not surprising (it would be more work to enable precise coverage info when inlining is enabled). lupus -- - [EMAIL PROTECTED] debian/rules [EMAIL PROTECTED] Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] ToString() performace in Mono
On 11/22/07 pablosantosluac wrote: And the following results: compareCompare.exe val is 599 and time 3525 c:\Archivos de programa\Mono-1.2.5.2\bin\mono.exe compareCompare.exe val is 599 and time 11577 Please provide also info on the cpu specs (which cpu and frequency). On a 2.4 GHz core 2 duo I get: val is 599 and time 4131 The profiler output is: prof counts: total/unmanaged: 1033/333 158 15.31 % NumberStore:.ctor (int) 82 7.95 % System.Text.StringBuilder:Append (char) 59 5.72 % System.NumberFormatter:FormatGeneral (System.NumberFormatter/NumberStore,int,System.Globalization.NumberFormatInfo,bool,bool) 43 4.17 % mono(GC_mark_from 42 4.07 % NumberStore:AppendIntegerString (int,System.Text.StringBuilder) 40 3.88 % (wrapper alloc) System.Object:Alloc (intptr,int) 39 3.78 % /lib/i686/cmov/libc.so.6(memset 34 3.29 % NumberStore:get_IntegerDigits () 33 3.20 % System.String:memcpy4 (byte*,byte*,int) 32 3.10 % mono(mono_array_new_specific 27 2.62 % System.String:memset (byte*,int,int) 27 2.62 % System.Text.StringBuilder:.ctor (string,int,int,int) 23 2.23 % System.NumberFormatter:FormatGeneral (System.NumberFormatter/NumberStore) [...] This shows what needs improvement. It also shows you'll see better numbers on Linux as the string allocation is more optimized there. It may be worth doing something with the byte array in NumberStore: ideally it should use the stringbuilder itself to store the number's characters, but this means quite a few changes to the code. The code should also likely be changed to the standard implementation of using divides by 10: then we could optimize that division in the jit. lupus -- - [EMAIL PROTECTED] debian/rules [EMAIL PROTECTED] Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] [PATCH] Generic sharing: Static field access
On 11/19/07 Zoltan Varga wrote: The problem with the trampoline is that since the class to init is dynamically decided, there is nothing to patch, so all calls will go through the generic trampoline code, which is much slower than a simple managed-to-native transition. No, here is again the trampoline pseudo code I described in the first email (vtable is in a register): deref vtable-initialized cmp/and/test condbranch slow_path: ret slow_path: call class_init ret We do the unmanaged transition only when the cctor has not been run yet. The x86 implementation could be something like (with vtable in edx): test $INIT_BIT,initialize_offset(%edx) jz slow_path ret slow_path: call class_init ret The common path is 3 instructions, we need a single trampoline for all the process and this works for both generics and AOT. I think the tiny amount of arch-specific code is worth writing to avoid the increased memory usage with the other solutions. BTW, for AOT code we could put the above code inside the AOT image itself, so it would be a direct call with no PLT entry involved. lupus -- - [EMAIL PROTECTED] debian/rules [EMAIL PROTECTED] Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] [PATCH] Generic sharing: Static field access
On 11/19/07 Mark Probst wrote: I don't really understand why a new trampoline is needed here. Since the argument to the trampoline is dynamic, it is not possible to patch the caller code, so a normal call to mono_runtime_class_init () would be sufficient. To be honest, I don't see any reason why we shouldn't use an icall in those cases. For some reason we defaulted to the trampoline, but it doesn't seem to be necessary. The new trampoline is not strictly essential to get the code working, but it is needed to not make it too slow (we have the same perf issue with AOT code). The new trampoline code would be a singleton and passing the argument in a register allows it to be just: deref vtable-initialized compare/and ret branch to icall so basically just 4-5 instructions executed in the common case instead of all the cost of going to unmanaged and back. There is an alternative solution which we could implement as well, though this requires one additional pointer per MonoVTable. When the MonoVTable is created this pointer is set to the icall and once the type is initialized it is set to a tiny function that just does: ret With this the initialization becomes a memory dereference and an indirect call+ret. The runtime cost is similar to the above solution (in AOT code we'd even avoid a PLT call), but it has the additional memory overhead. Other suggestions for optimizing this codepath are welcome as well. lupus -- - [EMAIL PROTECTED] debian/rules [EMAIL PROTECTED] Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] [PATCH] Generic sharing: Static field access
On 11/19/07 Zoltan Varga wrote: A simpler solution would be to emit the code below inline instead of making a call to a trampoline at all. This moves the memory overhead at the callsite, increasing icache footprint (we're talking about 10 bytes for x86 and 16+ bytes for other architectures). This means about 4 KB more code for mscorlib and 24 KB for a mcs compile (we should investigate what's up with mcs, though: the number of class inits we insert seems excessive). The trampoline would instead be shared between all the types in both generics and AOT (and there is a single conditional branch in the whole process instead of hundreds scattered around, saving also on the needed branch predictor resources). Inlining the check also means introducing additional basic blocks, making the existing ones smaller, which can have other side effects on the quality of the generated code. lupus -- - [EMAIL PROTECTED] debian/rules [EMAIL PROTECTED] Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] Status of CodeAccessSecurity (CAS)?
On 11/14/07 David Wolinsky wrote: In mono version r88278 ... this code crashes really bad (see below). I just wanted to know if anyone was actively working on this? Also is anyone working on the FileIOPermission? Please file a bug report, though implementing CAS is not a priority for us some other people (or you:) may want to contribute. In this particular case we likely do the CAS initialization from the wrong place so you end up with a circular initialization issue. lupus -- - [EMAIL PROTECTED] debian/rules [EMAIL PROTECTED] Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] Tracking test runs...
On 11/13/07 David Miller wrote: https://bugzilla.novell.com/tr_list_runs.cgi?plan_id=702 Do I really have to create a novell login just to look at the test runs? I've been trying to avoid having to create an account, and I can understand requiring it for submitting new bugs, but for looking at existing public bugs and testrun results, that simply doesn't make any sense and seem to me to deter new contributors. I agree that the results should be readily available, this test setup has just been started, so allow for some teething problems:) Once the current release work is completed I'd also like the testopia features to be available to contributors, so that testing for architectures and systems we don't have access to (or we don't have testing bandwith for) can be done by contributors and collected in the same place as the test runs we do in house. lupus -- - [EMAIL PROTECTED] debian/rules [EMAIL PROTECTED] Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] [PATCH]: Rewrite instruction list handling.
On 11/12/07 David Miller wrote: In fact, significant chunks special list handling code got removed. And I am certain many other significant cleanups and simplifications become possible after this patch goes in. I agree with the general change, but it's better to just add a prev field in MonoInst. This way we don't need to change all the code in all the architectures and risk regressions (some of which can be subtle as with many low-level JIT changes). With the new field in MonoInst that change can be done much more incrementally and so it can also be incrementally tested. Thanks! lupus -- - [EMAIL PROTECTED] debian/rules [EMAIL PROTECTED] Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] MONO_LOG_MASK type
On 11/08/07 Sanghyeon Seo wrote: mono(1) lists type as a possible value for MONO_LOG_MASK, but it doesn't seem to do anything. I thought it would log when types are loaded (is that the intended purpose?). Is there other way to see what types are loaded? There is curently no trace event associated with TYPE. You can use the profiler interface for this, though. lupus -- - [EMAIL PROTECTED] debian/rules [EMAIL PROTECTED] Monkeys do it better ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list