Re: [Mono-dev] ARM/NativeClient port

2013-02-04 Thread Paolo Molaro
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

2010-12-10 Thread Paolo Molaro

-- 
-
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

2010-12-10 Thread Paolo Molaro
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

2010-12-01 Thread Paolo Molaro
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

2010-11-30 Thread Paolo Molaro
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

2010-11-11 Thread Paolo Molaro
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.

2010-11-04 Thread Paolo Molaro
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)

2010-09-15 Thread Paolo Molaro
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

2010-07-15 Thread Paolo Molaro
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

2010-07-13 Thread Paolo Molaro
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

2010-07-02 Thread Paolo Molaro
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

2010-07-02 Thread Paolo Molaro
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

2010-07-01 Thread Paolo Molaro
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

2010-05-05 Thread Paolo Molaro
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]

2010-04-20 Thread Paolo Molaro
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

2010-04-09 Thread Paolo Molaro
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

2010-03-30 Thread Paolo Molaro
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

2010-03-30 Thread Paolo Molaro
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

2010-03-26 Thread Paolo Molaro
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

2010-03-25 Thread Paolo Molaro
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

2010-03-24 Thread Paolo Molaro
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

2010-03-22 Thread Paolo Molaro
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

2010-03-10 Thread Paolo Molaro
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

2010-03-04 Thread Paolo Molaro
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.

2010-02-23 Thread Paolo Molaro
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

2009-12-04 Thread Paolo Molaro
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

2009-09-03 Thread Paolo Molaro
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

2009-08-18 Thread Paolo Molaro
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

2009-06-26 Thread Paolo Molaro
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

2009-04-24 Thread Paolo Molaro
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

2009-04-24 Thread Paolo Molaro
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

2009-04-24 Thread Paolo Molaro
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

2009-04-14 Thread Paolo Molaro
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

2009-02-11 Thread Paolo Molaro
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)

2009-02-03 Thread Paolo Molaro
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

2009-02-03 Thread Paolo Molaro
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

2009-02-02 Thread Paolo Molaro
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.

2009-01-29 Thread Paolo Molaro
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

2009-01-28 Thread Paolo Molaro
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

2008-11-18 Thread Paolo Molaro
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

2008-11-13 Thread Paolo Molaro
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

2008-11-13 Thread Paolo Molaro
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

2008-11-12 Thread Paolo Molaro
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

2008-11-11 Thread Paolo Molaro
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

2008-11-10 Thread Paolo Molaro
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

2008-11-07 Thread Paolo Molaro
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

2008-11-06 Thread Paolo Molaro
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

2008-11-06 Thread Paolo Molaro
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

2008-10-20 Thread Paolo Molaro
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

2008-10-15 Thread Paolo Molaro
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

2008-10-10 Thread Paolo Molaro
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?

2008-10-10 Thread Paolo Molaro
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

2008-10-10 Thread Paolo Molaro
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?

2008-10-10 Thread Paolo Molaro
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

2008-10-10 Thread Paolo Molaro
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

2008-10-10 Thread Paolo Molaro
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

2008-10-07 Thread Paolo Molaro
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

2008-10-07 Thread Paolo Molaro
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

2008-10-03 Thread Paolo Molaro
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 ?

2008-10-01 Thread Paolo Molaro
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

2008-10-01 Thread Paolo Molaro
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

2008-07-30 Thread Paolo Molaro
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

2008-03-21 Thread Paolo Molaro
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

2008-03-20 Thread Paolo Molaro
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

2008-03-20 Thread Paolo Molaro
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

2008-03-15 Thread Paolo Molaro
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

2008-03-14 Thread Paolo Molaro
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.

2008-03-14 Thread Paolo Molaro
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.

2008-03-14 Thread Paolo Molaro
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

2008-03-14 Thread Paolo Molaro
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

2008-02-04 Thread Paolo Molaro
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

2008-02-04 Thread Paolo Molaro
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

2008-01-28 Thread Paolo Molaro
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

2008-01-28 Thread Paolo Molaro
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

2008-01-11 Thread Paolo Molaro
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

2008-01-08 Thread Paolo Molaro
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?

2008-01-07 Thread Paolo Molaro
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

2008-01-02 Thread Paolo Molaro
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

2007-12-20 Thread Paolo Molaro
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

2007-12-20 Thread Paolo Molaro
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

2007-12-20 Thread Paolo Molaro
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

2007-12-20 Thread Paolo Molaro
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

2007-12-19 Thread Paolo Molaro
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?

2007-12-19 Thread Paolo Molaro
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

2007-12-19 Thread Paolo Molaro
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

2007-12-17 Thread Paolo Molaro
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

2007-12-06 Thread Paolo Molaro
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

2007-12-06 Thread Paolo Molaro
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

2007-12-06 Thread Paolo Molaro
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

2007-12-05 Thread Paolo Molaro
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()

2007-12-03 Thread Paolo Molaro
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?

2007-12-03 Thread Paolo Molaro
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

2007-11-23 Thread Paolo Molaro
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

2007-11-20 Thread Paolo Molaro
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

2007-11-19 Thread Paolo Molaro
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

2007-11-19 Thread Paolo Molaro
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)?

2007-11-15 Thread Paolo Molaro
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...

2007-11-15 Thread Paolo Molaro
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.

2007-11-15 Thread Paolo Molaro
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

2007-11-15 Thread Paolo Molaro
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


  1   2   3   4   5   >