public inbox for java-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [RFA] Boehm GC support addition for debugging
@ 2006-06-19 16:23 Keith Seitz
  2006-06-20 19:13 ` Bryce McKinlay
  0 siblings, 1 reply; 16+ messages in thread
From: Keith Seitz @ 2006-06-19 16:23 UTC (permalink / raw)
  To: Java Patch List

[-- Attachment #1: Type: text/plain, Size: 820 bytes --]

Hi,

It turns out I lied when I said I was only going to stub the 
_Jv_ThreadDebug* methods. I think I will submit them for approval, but 
first I need to add two new functions the GC interface: one to suspend 
threads and one to resume them. Is there a related doc patch that is 
required for GC interface expansions? I don't see anything. Is there 
another GC implementation in which I need to stub these methods?

I've ifdef'd out the code for this because I'm still awaiting word on 
the latest revision of my Boehm GC external thread suspension patch.

Keith

ChangeLog
2006-06-19  Keith Seitz  <keiths@redhat.com>

         * include/boehm-gc.h (_Jv_SuspendThread): Declare.
         (_Jv_ResumeThread): Declare.
         * boehm-gc.cc (_Jv_SuspnedThread): New function.
         (_Jv_ResumeThread): New function.

[-- Attachment #2: boehm-gc.patch --]
[-- Type: text/x-patch, Size: 1291 bytes --]

Index: include/boehm-gc.h
===================================================================
--- include/boehm-gc.h	(revision 114727)
+++ include/boehm-gc.h	(working copy)
@@ -1,7 +1,7 @@
 // -*- c++ -*-
 // boehm-gc.h - Defines for Boehm collector.
 
-/* Copyright (C) 1998, 1999, 2002, 2004  Free Software Foundation
+/* Copyright (C) 1998, 1999, 2002, 2004, 2006  Free Software Foundation
 
    This file is part of libgcj.
 
@@ -83,4 +83,10 @@
 // _Jv_AllocBytes (jsize size) should go here, too.  But clients don't
 // usually include this header.
 
+// Suspend the given thread. This includes suspending the calling thread.
+extern "C" void _Jv_SuspendThread (pthread_t);
+
+// Resume a suspended thread.
+extern "C" void _Jv_ResumeThread (pthread_t);
+
 #endif /* __JV_BOEHM_GC__ */
Index: boehm.cc
===================================================================
--- boehm.cc	(revision 114727)
+++ boehm.cc	(working copy)
@@ -673,3 +673,21 @@
 #endif
 }
 
+void
+_Jv_SuspendThread (pthread_t thread)
+{
+#ifdef WAITING_FOR_GC_PATCH_APPROVAL
+  if (thread == pthread_self ())
+    GC_suspend_self ();
+  else
+    GC_suspend_thread (thread);
+#endif
+}
+
+void
+_Jv_ResumeThread (pthread_t thread)
+{
+#ifdef WAITING_FOR_GC_PATCH_APPROVAL
+  GC_resume_thread (thread);
+#endif
+}

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [RFA] Boehm GC support addition for debugging
  2006-06-19 16:23 [RFA] Boehm GC support addition for debugging Keith Seitz
@ 2006-06-20 19:13 ` Bryce McKinlay
  2006-06-20 19:23   ` Keith Seitz
                     ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Bryce McKinlay @ 2006-06-20 19:13 UTC (permalink / raw)
  To: Keith Seitz; +Cc: Java Patch List

Keith Seitz wrote:
> It turns out I lied when I said I was only going to stub the 
> _Jv_ThreadDebug* methods. I think I will submit them for approval, but 
> first I need to add two new functions the GC interface: one to suspend 
> threads and one to resume them. Is there a related doc patch that is 
> required for GC interface expansions? I don't see anything. Is there 
> another GC implementation in which I need to stub these methods?
nogc.cc should also be updated.

> I've ifdef'd out the code for this because I'm still awaiting word on 
> the latest revision of my Boehm GC external thread suspension patch.
>
> Keith
>
> ChangeLog
> 2006-06-19  Keith Seitz  <keiths@redhat.com>
>
>         * include/boehm-gc.h (_Jv_SuspendThread): Declare.
>         (_Jv_ResumeThread): Declare.
>         * boehm-gc.cc (_Jv_SuspnedThread): New function.
>         (_Jv_ResumeThread): New function.

OK, but please check in the Boehm GC patch at the same time and remove 
the #ifdef's. This is trunk we're talking about, and the thread 
suspension code is unlikely to break anything that currently works. If 
there are problems discovered with the thread suspension code, then we 
can just fix them in trunk.

Thanks,

Bryce

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [RFA] Boehm GC support addition for debugging
  2006-06-20 19:13 ` Bryce McKinlay
@ 2006-06-20 19:23   ` Keith Seitz
  2006-06-20 19:28     ` Bryce McKinlay
  2006-06-20 19:51   ` Keith Seitz
  2006-06-21 21:11   ` Keith Seitz
  2 siblings, 1 reply; 16+ messages in thread
From: Keith Seitz @ 2006-06-20 19:23 UTC (permalink / raw)
  To: Bryce McKinlay; +Cc: Java Patch List

Bryce McKinlay wrote:

> Keith Seitz wrote:
>> Is there  another GC implementation in which I need to stub these methods?
> nogc.cc should also be updated.

Ah, okay, Will do. I knew I probably missed one.

>> I've ifdef'd out the code for this because I'm still awaiting word on 
>> the latest revision of my Boehm GC external thread suspension patch.

> OK, but please check in the Boehm GC patch at the same time and remove 
> the #ifdef's. This is trunk we're talking about, and the thread 
> suspension code is unlikely to break anything that currently works. If 
> there are problems discovered with the thread suspension code, then we 
> can just fix them in trunk.

Hmm. Okay, well, so should I consider the GC patch "approved" _for now_ 
(or at least until it is a problem for me)?

I just want to be explicitly clear on this since Hans has not officially 
approved anything w.r.t. my GC patch.

Keith

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [RFA] Boehm GC support addition for debugging
  2006-06-20 19:23   ` Keith Seitz
@ 2006-06-20 19:28     ` Bryce McKinlay
  2006-06-21 20:58       ` Keith Seitz
  0 siblings, 1 reply; 16+ messages in thread
From: Bryce McKinlay @ 2006-06-20 19:28 UTC (permalink / raw)
  To: Keith Seitz; +Cc: Java Patch List

Keith Seitz wrote:
>> OK, but please check in the Boehm GC patch at the same time and 
>> remove the #ifdef's. This is trunk we're talking about, and the 
>> thread suspension code is unlikely to break anything that currently 
>> works. If there are problems discovered with the thread suspension 
>> code, then we can just fix them in trunk.
>
> Hmm. Okay, well, so should I consider the GC patch "approved" _for 
> now_ (or at least until it is a problem for me)?

Yes, it is approved for libgcj trunk.

Bryce

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [RFA] Boehm GC support addition for debugging
  2006-06-20 19:13 ` Bryce McKinlay
  2006-06-20 19:23   ` Keith Seitz
@ 2006-06-20 19:51   ` Keith Seitz
  2006-06-21  0:05     ` Bryce McKinlay
  2006-06-21 21:11   ` Keith Seitz
  2 siblings, 1 reply; 16+ messages in thread
From: Keith Seitz @ 2006-06-20 19:51 UTC (permalink / raw)
  To: Bryce McKinlay; +Cc: Java Patch List

Bryce McKinlay wrote:

>>         * include/boehm-gc.h (_Jv_SuspendThread): Declare.
>>         (_Jv_ResumeThread): Declare.
>>         * boehm-gc.cc (_Jv_SuspnedThread): New function.
>>         (_Jv_ResumeThread): New function.

Grr. Now I have a problem with this:

+ void
+ _Jv_SuspendThread (pthread_t thread)
+ {
+   if (thread == pthread_self ())
+     GC_suspend_self ();
+   else
+     GC_suspend_thread (thread);
+ }
+
+ void
+ _Jv_ResumeThread (pthread_t thread)
+ {
+   GC_resume_thread (thread);
+ }

This going to break non-pthreads builds. There doesn't appear to be any 
abstract platform-independent threading convention between the GC and 
gcj. The GC uses GC_thread (which is private to the GC), and gcj uses 
either java.lang.Thread or _Jv_Thread_t.

Alas, it looks like we have a couple of options:

1) Place ifdefs around the code for all the various cases. I presume 
these are pthreads, win32, and ???

2) Make GC_thread and GC_lookup_thread public in the GC

3) Abstract what the GC wants in posix-threads.cc 
(_Jv_Thread_t->data->thread) and win32-threads.cc 
(_Jv_Thread_t->data->thread_obj). Still have to do #1, though...

4) Something else I've missed

Sorry about the can of worms. I should have sought a solution to this 
before posting my patch. I'll keep digging.

Keith

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [RFA] Boehm GC support addition for debugging
  2006-06-20 19:51   ` Keith Seitz
@ 2006-06-21  0:05     ` Bryce McKinlay
  2006-06-21 14:43       ` Keith Seitz
  0 siblings, 1 reply; 16+ messages in thread
From: Bryce McKinlay @ 2006-06-21  0:05 UTC (permalink / raw)
  To: Keith Seitz; +Cc: Java Patch List

[-- Attachment #1: Type: text/plain, Size: 539 bytes --]

Keith Seitz wrote:
> This going to break non-pthreads builds. There doesn't appear to be 
> any abstract platform-independent threading convention between the GC 
> and gcj. The GC uses GC_thread (which is private to the GC), and gcj 
> uses either java.lang.Thread or _Jv_Thread_t.

Would the following work? _Jv_SuspendThread would take a 
_Jv_ThreadDesc_t argument, and use _Jv_GetPlatformThreadID() on it. 
GC_suspend_thread() etc would be defined in gc.h with whatever the 
correct argument type is depending on the platform.

Bryce


[-- Attachment #2: libgcj-thread-id.patch --]
[-- Type: text/x-patch, Size: 1053 bytes --]

Index: win32-threads.h
===================================================================
--- win32-threads.h	(revision 114472)
+++ win32-threads.h	(working copy)
@@ -72,6 +72,15 @@
 
 typedef void _Jv_ThreadStartFunc (java::lang::Thread *);
 
+// Type identifying a win32 thread.
+typedef HANDLE _Jv_ThreadDesc_t;
+
+inline _Jv_ThreadDesc_t
+_Jv_GetPlatformThreadID(_Jv_Thread_t *t)
+{
+  return t->handle;
+}
+
 //
 // Condition variables.
 //
Index: posix-threads.h
===================================================================
--- posix-threads.h	(revision 114472)
+++ posix-threads.h	(working copy)
@@ -47,7 +47,6 @@
 
 typedef void _Jv_ThreadStartFunc (java::lang::Thread *);
 
-
 // Condition Variables used to implement wait/notify/sleep/interrupt.
 typedef struct
 {
@@ -82,6 +81,15 @@
   return (mu->owner != pthread_self());
 }
 
+// Type identifying a POSIX thread.
+typedef pthread_t _Jv_ThreadDesc_t;
+
+inline _Jv_ThreadDesc_t
+_Jv_GetPlatformThreadID(_Jv_Thread_t *t)
+{
+  return t->thread;
+}
+
 //
 // Condition variables.
 //

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [RFA] Boehm GC support addition for debugging
  2006-06-21  0:05     ` Bryce McKinlay
@ 2006-06-21 14:43       ` Keith Seitz
  2006-06-21 16:28         ` Bryce McKinlay
  0 siblings, 1 reply; 16+ messages in thread
From: Keith Seitz @ 2006-06-21 14:43 UTC (permalink / raw)
  To: Bryce McKinlay; +Cc: Java Patch List

Bryce McKinlay wrote:

> Would the following work? _Jv_SuspendThread would take a 
> _Jv_ThreadDesc_t argument, and use _Jv_GetPlatformThreadID() on it. 
> GC_suspend_thread() etc would be defined in gc.h with whatever the 
> correct argument type is depending on the platform.

Yeah, that should work. I think this is a combination of nos. 3 and 4 on 
my list:

3) Abstract what the GC wants in posix-threads.cc 
(_Jv_Thread_t->data->thread) and win32-threads.cc 
(_Jv_Thread_t->data->thread_obj). Still have to do #1, though...

4) Something else I've missed

:-)

Thanks!
Keith

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [RFA] Boehm GC support addition for debugging
  2006-06-21 14:43       ` Keith Seitz
@ 2006-06-21 16:28         ` Bryce McKinlay
  0 siblings, 0 replies; 16+ messages in thread
From: Bryce McKinlay @ 2006-06-21 16:28 UTC (permalink / raw)
  To: Keith Seitz; +Cc: Java Patch List

[-- Attachment #1: Type: text/plain, Size: 434 bytes --]

Keith Seitz wrote:
> Bryce McKinlay wrote:
>
>> Would the following work? _Jv_SuspendThread would take a 
>> _Jv_ThreadDesc_t argument, and use _Jv_GetPlatformThreadID() on it. 
>> GC_suspend_thread() etc would be defined in gc.h with whatever the 
>> correct argument type is depending on the platform.
>
> Yeah, that should work. I think this is a combination of nos. 3 and 4 
> on my list:

I've checked this in to trunk.

Bryce



[-- Attachment #2: libgcj-thread-id.patch --]
[-- Type: text/x-patch, Size: 1337 bytes --]

2006-06-21  Bryce McKinlay  <mckinlay@redhat.com>

        * include/win32-threads.h (_Jv_ThreadDesc_t): New typedef.
        (_Jv_GetPlatformThreadID): New function.
        * include/posix-threads.h (_Jv_ThreadDesc_t): New typedef.
        (_Jv_GetPlatformThreadID): New function.

Index: win32-threads.h
===================================================================
--- win32-threads.h	(revision 114472)
+++ win32-threads.h	(working copy)
@@ -72,6 +72,15 @@
 
 typedef void _Jv_ThreadStartFunc (java::lang::Thread *);
 
+// Type identifying a win32 thread.
+typedef HANDLE _Jv_ThreadDesc_t;
+
+inline _Jv_ThreadDesc_t
+_Jv_GetPlatformThreadID(_Jv_Thread_t *t)
+{
+  return t->handle;
+}
+
 //
 // Condition variables.
 //
Index: posix-threads.h
===================================================================
--- posix-threads.h	(revision 114472)
+++ posix-threads.h	(working copy)
@@ -47,7 +47,6 @@
 
 typedef void _Jv_ThreadStartFunc (java::lang::Thread *);
 
-
 // Condition Variables used to implement wait/notify/sleep/interrupt.
 typedef struct
 {
@@ -82,6 +81,15 @@
   return (mu->owner != pthread_self());
 }
 
+// Type identifying a POSIX thread.
+typedef pthread_t _Jv_ThreadDesc_t;
+
+inline _Jv_ThreadDesc_t
+_Jv_GetPlatformThreadID(_Jv_Thread_t *t)
+{
+  return t->thread;
+}
+
 //
 // Condition variables.
 //

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [RFA] Boehm GC support addition for debugging
  2006-06-20 19:28     ` Bryce McKinlay
@ 2006-06-21 20:58       ` Keith Seitz
  0 siblings, 0 replies; 16+ messages in thread
From: Keith Seitz @ 2006-06-21 20:58 UTC (permalink / raw)
  To: Bryce McKinlay; +Cc: Java Patch List

[-- Attachment #1: Type: text/plain, Size: 1050 bytes --]

Bryce McKinlay wrote:
> Keith Seitz wrote:
>> Hmm. Okay, well, so should I consider the GC patch "approved" _for 
>> now_ (or at least until it is a problem for me)?
> 
> Yes, it is approved for libgcj trunk.

Okay, I have committed this. I made one small change to accommodate the 
recent _Jv_ThreadDesc_t patch you committed: I've eliminated 
GC_suspend_self. GC_suspend_thread will check for this case itself. 
[That saves me from having to do a pthread_self in otherwise generic code.]

Thank you for all your help with this.

Keith

ChangeLog

2006-06-21  Keith Seitz  <keiths@redhat.com>

         * pthread_stop_world.c (GC_suspend_handler): Redirect to suspension
         routine if signal is received and thread is flagged SUSPENDED.
         (suspend_self): New function.
         (GC_suspend_thread): New function.
         (GC_resume_thread): New function.
         * include/gc.h (GC_suspend_thread): Declare.
         (GC_resumet_thread): Declare.
         * include/private/pthread_support.h (SUSPENDED): New GC_thread
         flag.

[-- Attachment #2: gc-suspend.patch --]
[-- Type: text/x-patch, Size: 3723 bytes --]

Index: include/gc.h
===================================================================
--- include/gc.h	(revision 114865)
+++ include/gc.h	(working copy)
@@ -1040,4 +1040,14 @@
     }  /* end of extern "C" */
 #endif
 
+/* External thread suspension support. These functions do not implement
+ * suspension counts or any other higher-level abstraction. Threads which
+ * have been suspended numerous times will resume with the very first call
+ * to GC_resume_thread.
+ */
+#if defined(GC_PTHREADS) && !defined(GC_SOLARIS_THREADS) \
+  && !defined(GC_WIN32_THREADS) && !defined(GC_DARWIN_THREADS)
+GC_API void GC_suspend_thread GC_PROTO((pthread_t));
+GC_API void GC_resume_thread GC_PROTO((pthread_t));
+#endif
 #endif /* _GC_H */
Index: include/private/pthread_support.h
===================================================================
--- include/private/pthread_support.h	(revision 114865)
+++ include/private/pthread_support.h	(working copy)
@@ -33,6 +33,7 @@
 #	define FINISHED 1   	/* Thread has exited.	*/
 #	define DETACHED 2	/* Thread is intended to be detached.	*/
 #	define MAIN_THREAD 4	/* True for the original thread only.	*/
+#       define SUSPENDED 8      /* True if thread was suspended externally */
     short thread_blocked;	/* Protected by GC lock.		*/
     				/* Treated as a boolean value.  If set,	*/
     				/* thread will acquire GC lock before	*/
Index: pthread_stop_world.c
===================================================================
--- pthread_stop_world.c	(revision 114865)
+++ pthread_stop_world.c	(working copy)
@@ -13,6 +13,8 @@
   /* Doesn't exist on HP/UX 11.11. */
 #endif
 
+void suspend_self();
+
 #if DEBUG_THREADS
 
 #ifndef NSIG
@@ -127,9 +129,14 @@
 
 void GC_suspend_handler(int sig)
 {
-  int old_errno = errno;
-  GC_with_callee_saves_pushed(GC_suspend_handler_inner, (ptr_t)(word)sig);
-  errno = old_errno;
+  GC_thread me = GC_lookup_thread (pthread_self());
+  if (me -> flags & SUSPENDED)
+    suspend_self();
+  else {
+    int old_errno = errno;
+    GC_with_callee_saves_pushed(GC_suspend_handler_inner, (ptr_t)(word)sig);
+    errno = old_errno;
+  }
 }
 
 #else
@@ -137,9 +144,14 @@
 /* in the signal handler frame.						*/
 void GC_suspend_handler(int sig)
 {
-  int old_errno = errno;
-  GC_suspend_handler_inner((ptr_t)(word)sig);
-  errno = old_errno;
+  GC_thread me = GC_lookup_thread(pthread_self());
+  if (me -> flags & SUSPENDED)
+    suspend_self();
+  else {
+    int old_errno = errno;
+    GC_suspend_handler_inner((ptr_t)(word)sig);
+    errno = old_errno;
+  }
 }
 #endif
 
@@ -430,6 +442,47 @@
     GC_stopping_thread = 0;  /* debugging only */
 }
 
+void suspend_self() {
+  GC_thread me = GC_lookup_thread(pthread_self());
+  if (me == NULL)
+    ABORT("attempting to suspend unknown thread");
+
+  me -> flags |= SUSPENDED;
+  GC_start_blocking();
+  while (me -> flags & SUSPENDED)
+    GC_brief_async_signal_safe_sleep();
+  GC_end_blocking();
+}
+
+void GC_suspend_thread(pthread_t thread) {
+  if (thread == pthread_self())
+    suspend_self();
+  else {
+    int result;
+    GC_thread t = GC_lookup_thread(thread);
+    if (t == NULL)
+      ABORT("attempting to suspend unknown thread");
+
+    t -> flags |= SUSPENDED;
+    result = pthread_kill (t -> id, SIG_SUSPEND);
+    switch (result) {
+    case ESRCH:
+    case 0:
+      break;
+    default:
+      ABORT("pthread_kill failed");
+    }
+  }
+}
+
+void GC_resume_thread(pthread_t thread) {
+  GC_thread t = GC_lookup_thread(thread);
+  if (t == NULL)
+    ABORT("attempting to resume unknown thread");
+
+  t -> flags &= ~SUSPENDED;
+}
+
 /* Caller holds allocation lock, and has held it continuously since	*/
 /* the world stopped.							*/
 void GC_start_world()

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [RFA] Boehm GC support addition for debugging
  2006-06-20 19:13 ` Bryce McKinlay
  2006-06-20 19:23   ` Keith Seitz
  2006-06-20 19:51   ` Keith Seitz
@ 2006-06-21 21:11   ` Keith Seitz
  2006-06-26 16:01     ` Bryce McKinlay
  2 siblings, 1 reply; 16+ messages in thread
From: Keith Seitz @ 2006-06-21 21:11 UTC (permalink / raw)
  To: Bryce McKinlay; +Cc: Java Patch List

[-- Attachment #1: Type: text/plain, Size: 595 bytes --]

Bryce McKinlay wrote:

> nogc.cc should also be updated.

I've updated the patch to include this nogc.cc and include/no-gc.h for 
final approval.

Keith

ChangeLog
2006-06-21  Keith Seitz  <keiths@redhat.com>

         * include/no-gc.h (_Jv_SuspendThread): Declare.
         (_Jv_ResumeThread): Likewise.
         * include/boehm-gc.h (_Jv_SuspendThread): Declare.
         (_Jv_ResumeThread): Likewise.
         * nogc.cc (_Jv_SuspendThread): New function.
         (_Jv_ResumeThread): Likewise.
         * boehm-gc.cc (_Jv_SuspendThread): New function.
         (_Jv_ResumeThread): Likewise.

[-- Attachment #2: gcs-suspend.patch --]
[-- Type: text/x-patch, Size: 2501 bytes --]

Index: include/no-gc.h
===================================================================
--- include/no-gc.h	(revision 114859)
+++ include/no-gc.h	(working copy)
@@ -1,7 +1,7 @@
 // -*- c++ -*-
 // no-gc.h - Defines for no garbage collector.
 
-/* Copyright (C) 1998, 1999  Free Software Foundation
+/* Copyright (C) 1998, 1999, 2006  Free Software Foundation
 
    This file is part of libgcj.
 
@@ -12,6 +12,10 @@
 #ifndef __JV_NO_GC__
 #define __JV_NO_GC__
 
-// Nothing.
+// Suspend the given thread. This includes suspending the calling thread.
+extern "C" void _Jv_SuspendThread (_Jv_Thread_t*);
 
+// Resume a suspended thread.
+extern "C" void _Jv_ResumeThread (_Jv_Thread_t*);
+
 #endif /* __JV_NO_GC__ */
Index: include/boehm-gc.h
===================================================================
--- include/boehm-gc.h	(revision 114859)
+++ include/boehm-gc.h	(working copy)
@@ -1,7 +1,7 @@
 // -*- c++ -*-
 // boehm-gc.h - Defines for Boehm collector.
 
-/* Copyright (C) 1998, 1999, 2002, 2004  Free Software Foundation
+/* Copyright (C) 1998, 1999, 2002, 2004, 2006  Free Software Foundation
 
    This file is part of libgcj.
 
@@ -83,4 +83,10 @@
 // _Jv_AllocBytes (jsize size) should go here, too.  But clients don't
 // usually include this header.
 
+// Suspend the given thread. This includes suspending the calling thread.
+extern "C" void _Jv_SuspendThread (_Jv_Thread_t*);
+
+// Resume a suspended thread.
+extern "C" void _Jv_ResumeThread (_Jv_Thread_t*);
+
 #endif /* __JV_BOEHM_GC__ */
Index: nogc.cc
===================================================================
--- nogc.cc	(revision 114617)
+++ nogc.cc	(working copy)
@@ -1,6 +1,6 @@
 // nogc.cc - Implement null garbage collector.
 
-/* Copyright (C) 1998, 1999, 2000, 2001, 2002  Free Software Foundation
+/* Copyright (C) 1998, 1999, 2000, 2001, 2002, 2006  Free Software Foundation
 
    This file is part of libgcj.
 
@@ -165,3 +165,13 @@
   return result;
 }
 #endif /* JV_HASH_SYNCHRONIZATION */
+
+void
+_Jv_SuspendThread (_Jv_Thread_t* thread)
+{
+}
+
+void
+_Jv_ResumeThread (_Jv_Thread_t* thread)
+{
+}
Index: boehm.cc
===================================================================
--- boehm.cc	(revision 114617)
+++ boehm.cc	(working copy)
@@ -673,3 +673,14 @@
 #endif
 }
 
+void
+_Jv_SuspendThread (_Jv_Thread_t* thread)
+{
+  GC_suspend_thread (_Jv_GetPlatformThreadID (thread));
+}
+
+void
+_Jv_ResumeThread (_Jv_Thread_t* thread)
+{
+  GC_resume_thread (_Jv_GetPlatformThreadID (thread));
+}

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [RFA] Boehm GC support addition for debugging
  2006-06-21 21:11   ` Keith Seitz
@ 2006-06-26 16:01     ` Bryce McKinlay
  2006-06-26 16:32       ` Keith Seitz
  2006-06-27  5:01       ` Andrew Pinski
  0 siblings, 2 replies; 16+ messages in thread
From: Bryce McKinlay @ 2006-06-26 16:01 UTC (permalink / raw)
  To: Keith Seitz; +Cc: Java Patch List

Keith Seitz wrote:
> I've updated the patch to include this nogc.cc and include/no-gc.h for 
> final approval.

> -// Nothing.
> +// Suspend the given thread. This includes suspending the calling thread.
> +extern "C" void _Jv_SuspendThread (_Jv_Thread_t*);
>   

Just a coding style nit: for consistency with the rest of the code, 
there should be a space between the argument type and the '*', and not 
between the '*' and the argument name.

Otherwise, this is fine.

Bryce

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [RFA] Boehm GC support addition for debugging
  2006-06-26 16:01     ` Bryce McKinlay
@ 2006-06-26 16:32       ` Keith Seitz
  2006-06-27  5:01       ` Andrew Pinski
  1 sibling, 0 replies; 16+ messages in thread
From: Keith Seitz @ 2006-06-26 16:32 UTC (permalink / raw)
  To: Bryce McKinlay; +Cc: Java Patch List

Bryce McKinlay wrote:

> Just a coding style nit: for consistency with the rest of the code, 
> there should be a space between the argument type and the '*', and not 
> between the '*' and the argument name.

Grr. Some habits die hard. I've corrected them in this patch, and I will 
audit anything I've been doing lately to change them, too.

> Otherwise, this is fine.

Committed. Thanks.

Keith

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [RFA] Boehm GC support addition for debugging
  2006-06-26 16:01     ` Bryce McKinlay
  2006-06-26 16:32       ` Keith Seitz
@ 2006-06-27  5:01       ` Andrew Pinski
  2006-06-27  5:07         ` Andrew Pinski
  1 sibling, 1 reply; 16+ messages in thread
From: Andrew Pinski @ 2006-06-27  5:01 UTC (permalink / raw)
  To: Bryce McKinlay, gcc mailing list; +Cc: Keith Seitz, Java Patch List


On Jun 26, 2006, at 9:01 AM, Bryce McKinlay wrote:

> Otherwise, this is fine.

No it is not, it broke bootstrap on powerpc-darwin:
/Users/regress/tbox/svn-gcc/libjava/boehm.cc: In function 'void  
_Jv_SuspendThread(_Jv_Thread_t*)':
/Users/regress/tbox/svn-gcc/libjava/boehm.cc:679: error:  
'GC_suspend_thread' was not declared in this scope
/Users/regress/tbox/svn-gcc/libjava/boehm.cc: In function 'void  
_Jv_ResumeThread(_Jv_Thread_t*)':
/Users/regress/tbox/svn-gcc/libjava/boehm.cc:685: error:  
'GC_resume_thread' was not declared in this scope

This is the fourth patch to libjava in the last two weeks which have  
caused a bootstrap
failure on powerpc-darwin (and sometimes on other targets) so maybe  
it might be time to step
back for a second before approving/applying patches.


Thanks,
Andrew Pinski

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [RFA] Boehm GC support addition for debugging
  2006-06-27  5:01       ` Andrew Pinski
@ 2006-06-27  5:07         ` Andrew Pinski
  2006-06-27  5:20           ` Andreas Tobler
  0 siblings, 1 reply; 16+ messages in thread
From: Andrew Pinski @ 2006-06-27  5:07 UTC (permalink / raw)
  To: Andrew Pinski
  Cc: Bryce McKinlay, gcc mailing list, Keith Seitz, Java Patch List


On Jun 26, 2006, at 10:02 PM, Andrew Pinski wrote:

>
> On Jun 26, 2006, at 9:01 AM, Bryce McKinlay wrote:
>
>> Otherwise, this is fine.
>
> No it is not, it broke bootstrap on powerpc-darwin:

Did you look when the function would have been declared.
 From gc.h:

#if defined(GC_PTHREADS) && !defined(GC_SOLARIS_THREADS) \
   && !defined(GC_WIN32_THREADS) && !defined(GC_DARWIN_THREADS)
GC_API void GC_suspend_thread GC_PROTO((pthread_t));
GC_API void GC_resume_thread GC_PROTO((pthread_t));
#endif

So this is not just Darwin but also Windows and maybe solaris too.

-- Pinski

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [RFA] Boehm GC support addition for debugging
  2006-06-27  5:07         ` Andrew Pinski
@ 2006-06-27  5:20           ` Andreas Tobler
  2006-06-27 16:05             ` Marco Trudel
  0 siblings, 1 reply; 16+ messages in thread
From: Andreas Tobler @ 2006-06-27  5:20 UTC (permalink / raw)
  To: Andrew Pinski, Bryce McKinlay, Keith Seitz
  Cc: gcc mailing list, Java Patch List

Andrew Pinski wrote:
> 
> On Jun 26, 2006, at 10:02 PM, Andrew Pinski wrote:
> 
>>
>> On Jun 26, 2006, at 9:01 AM, Bryce McKinlay wrote:
>>
>>> Otherwise, this is fine.
>>
>> No it is not, it broke bootstrap on powerpc-darwin:
> 
> Did you look when the function would have been declared.
>  From gc.h:
> 
> #if defined(GC_PTHREADS) && !defined(GC_SOLARIS_THREADS) \
>   && !defined(GC_WIN32_THREADS) && !defined(GC_DARWIN_THREADS)
> GC_API void GC_suspend_thread GC_PROTO((pthread_t));
> GC_API void GC_resume_thread GC_PROTO((pthread_t));
> #endif
> 
> So this is not just Darwin but also Windows and maybe solaris too.

Solaris too. :(

I'd wish being a bit more careful with such patches.
Everyone can ask for RFT and I'll happily test on my farm as time permits.

Andreas

P.S, Me being a _spare_ time contributor....

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [RFA] Boehm GC support addition for debugging
  2006-06-27  5:20           ` Andreas Tobler
@ 2006-06-27 16:05             ` Marco Trudel
  0 siblings, 0 replies; 16+ messages in thread
From: Marco Trudel @ 2006-06-27 16:05 UTC (permalink / raw)
  To: Andreas Tobler
  Cc: Andrew Pinski, Bryce McKinlay, Keith Seitz, gcc mailing list,
	Java Patch List

I already sent a patch for that to the java-patches list...


Andreas Tobler wrote:
> Andrew Pinski wrote:
>>
>> On Jun 26, 2006, at 10:02 PM, Andrew Pinski wrote:
>>
>>>
>>> On Jun 26, 2006, at 9:01 AM, Bryce McKinlay wrote:
>>>
>>>> Otherwise, this is fine.
>>>
>>> No it is not, it broke bootstrap on powerpc-darwin:
>>
>> Did you look when the function would have been declared.
>>  From gc.h:
>>
>> #if defined(GC_PTHREADS) && !defined(GC_SOLARIS_THREADS) \
>>   && !defined(GC_WIN32_THREADS) && !defined(GC_DARWIN_THREADS)
>> GC_API void GC_suspend_thread GC_PROTO((pthread_t));
>> GC_API void GC_resume_thread GC_PROTO((pthread_t));
>> #endif
>>
>> So this is not just Darwin but also Windows and maybe solaris too.
> 
> Solaris too. :(
> 
> I'd wish being a bit more careful with such patches.
> Everyone can ask for RFT and I'll happily test on my farm as time permits.
> 
> Andreas
> 
> P.S, Me being a _spare_ time contributor....
> 
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2006-06-27 16:05 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2006-06-19 16:23 [RFA] Boehm GC support addition for debugging Keith Seitz
2006-06-20 19:13 ` Bryce McKinlay
2006-06-20 19:23   ` Keith Seitz
2006-06-20 19:28     ` Bryce McKinlay
2006-06-21 20:58       ` Keith Seitz
2006-06-20 19:51   ` Keith Seitz
2006-06-21  0:05     ` Bryce McKinlay
2006-06-21 14:43       ` Keith Seitz
2006-06-21 16:28         ` Bryce McKinlay
2006-06-21 21:11   ` Keith Seitz
2006-06-26 16:01     ` Bryce McKinlay
2006-06-26 16:32       ` Keith Seitz
2006-06-27  5:01       ` Andrew Pinski
2006-06-27  5:07         ` Andrew Pinski
2006-06-27  5:20           ` Andreas Tobler
2006-06-27 16:05             ` Marco Trudel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).