This is the mail archive of the java-patches@gcc.gnu.org mailing list for the Java project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFC] GC: external thread suspension


Boehm, Hans wrote:

In general, this looks pretty good to me.  As you no doubt discovered,
this is tricky code.

Indeed! It is quite humbling.


Here are a few more comments:

Signal handler:

UNLOCK() is generally not asynch-signal-safe.  This needs a comment
stating that this only happens if it's directly called.  Is it better to
suspend oneself using something like the GC_start_blocking() support?
(Note that this is fixed and renamed GC_do_blocking() in 7.0, and maybe
hard to use in 6.7.)

Comment added. As for GC_start_blocking/GC_do_blocking, well, you're in a better position to comment about that. I didn't even know that existed until you mentioned it!

<thinking out loud>
Moving to GC_start/end_blocking would mean that I would have to do
something like:

void GC_suspend_self() {
  GC_start_blocking();
  while (suspended)
    sleep();
  GC_end_blocking();
}

In this context, I don't think we need to worry too much about the
loop's conditional and locking, since we are only reading state and I
don't really care if the debugger is delayed X ms because we had to loop one more time.


Of course we are just spinning... But I don't think it matters. [At least not in my case.]

If I am reading GC_suspend_all correctly, while GC_thread->thread_blocked is TRUE, the GC will not signal the thread. Therefore a blocked thread is never garbage collected. In my patch, this restriction does not exist. I don't think it matters, though. [Does it?]

So if the signal handler is called while thread_blocked is set, we know that it is a result of suspension.

This seems like it could be a whole lot cleaner. Provided I haven't made any other big logic mistakes. :-)
</thinking out loud>


What do others think? Despite the one restriction (the GC will not run on a blocked thread), I think I am beginning to like the GC_start/end_blocking approach. It seems much simpler than what I've come up with.

Redundant parens in while loop at end.  Shouldn't one of the && be an
||?  This doesn't look quite right.

Yeah, that's wrong. What I really want to say is "if we're externally suspended, exit loop if no longer externally suspended" and "if not externally suspended, do the normal GC thing." That translates into a condition that makes me nervous:

ext_suspended = me -> flags & SUSPENDED; // Yich!

    sigsuspend(&suspend_handler_mask);        /* Wait for signal */
    while ((ext_suspended && me -> flags & SUSPENDED)
           || (!ext_suspended && GC_world_is_stopped
               && GC_stop_count == my_stop_count)) {
        GC_brief_async_signal_safe_sleep();
#       if DEBUG_THREADS
          GC_err_printf0("Sleeping in signal handler");
#       endif
    }

What happens if a thread is suspended in the final loop, and multiple
GCs occur? Can it acknowledge the suspension?

No, it cannot, but it should already be in a state where the GC can run. send_suspend_signal will overlook externally suspended threads in the stop count. One time or another I managed to convince myself that this is okay. I don't remember much about it anymore, though.

suspend_thread:
If externally suspended, can it wake up asynchronously?
Add heading comment and/or rename, since this doesn't really suspend a
thread; it only suspends the signal.

Renamed to send_suspend_signal, which you mention below.


GC_stop_world_inner:
Rename to GC_stop_threads?  I don't like the current name, since that's
no longer always what it does.

Done.


I don't like the resume_thread vs GC_resume_thread naming.  How about
send_suspend_signal and send_resume_signal?

Renamed.


I'm aiming for #if's which are properly indented, but with the # in
column 1.
In gc6.7, that isn't always the case anyway, but I fixed it in 7.0, and
would
like to see new code follow that convention.

Done.


Keith


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]