Re[2]: [Gc] Fix for Win32 PARALLEL_MARK

Ivan Maidanski ivmai at mail.ru
Wed Nov 12 02:27:15 PST 2008


Hi!

"Boehm, Hans" <hans.boehm at hp.com> wrote:
> Thanks.  This looks very promising, and it will be really nice to have this working reliably.  I checked in the preceding PARALLEL_MARK patch, this patch, and diff25 for test.c.  I Also changed the declaration of n_tests in test.c to have type AO_t (a necessary change, I think, though it may have been in one of your other patches).
>
> I don't think this breaks anything, but it doesn't quite seem to be working for me either:
>
> 1) I think we need a definition of AO_ASSUME_WINDOWS98 if we're on Windows and PARALLEL_MARK is defined.  Otherwise we get a correct complaint that CAS isn't available.

-DAO_ASSUME_WINDOWS98 should be added to VC++ build scripts only. I don't think it's correct to add it to some header file.

>
> 2) I'm seeing a failure on win64 with PARALLEL_MARK that I haven't debugged.  Is that still expected?

I've never expected it. Are You talking about "Unexpected heap growth"? If not, tell me the failure detailed info.

>
> Should configure.ac be updated to handle --enable-parallel-mark for cygwin?

Yes. It works (but not tested too much). The algorithm is really same for pthread_support.

>
> More below:
>
> > -----Original Message-----
> > From: gc-bounces at napali.hpl.hp.com
> > [mailto:gc-bounces at napali.hpl.hp.com] On Behalf Of Ivan Maidanski
> > Sent: Sunday, October 26, 2008 8:17 AM
> > To: gc at napali.hpl.hp.com
> > Subject: [Gc] Fix for Win32 PARALLEL_MARK
> >
> > Hi!
> >
> > This patch is for my previous patch introducing parallel
> > marking on Win32 and Cygwin. My previous implementation of
> > Win32 GC_wait_marker() turns (frequently when GC_MARKERS >
> > ncpu) into simple yielding (thus uselessly consuming CPU time
> > outside garbage collections).
> >
> > Now I'm using PulseEvent and atomic NT SignalObjectAndWait()
> > (on Windows 98/Me parallel marking is set disabled but it's
> > ok since multicore CPUs are not supported by these OS, AFAIK).
> >
> > Mark lock spinning is disabled by default (unless
> > MARKLOCK_USES_SPIN) but it works ok. More careful
> > measurements should be done.
> >
> > Tested with MinGW and VC++ 32-bit on dual core CPU.
> >
> > Still some questions remain:
> > - should we scan markers in GC_get_next_stack()?;
> I think so.  This is used to skip thread stacks.  And we certainly don't want to mark the marker thread stacks.

Ok. I'll do it.

>
> > - should we use .._full() version (or weaker) of CAS in
> > GC_acquire_mark_lock()?
> I think you want to use _acquire versions whenever this is used to acquire a lock, and _release in GC_release_mark_lock.  I did not study the logic there carefully enough to claim that it is correct.  But I also didn't immediately see any problems.  Mark_mutex_event is set up so that signaling it is remembered, even if there are currently no waiters, right?

At present, I don't use CAS anymore. As for inc/dec, their release/acquire versions are redirected to full version in libatomics for msvc/gcc x86/amd64, so let me leave it as is.

>
> One minor issue is that I would like to see corrected is that the statistics variables should also be accessed through AO_ operations.  Otherwise it's very hard to tell whether they're trustworthy.  You can easily get into situations in which such counters are off by a factor of two or more due to contention.  I've been bitten by this.  (...)

I'll do it here and for pthread_support too.

>
> Hans
>
> >
> > PS. This patch also adjusts ..._printf() actual params
> > (according to their format specifiers) for "win32_threads.c" file.
> >
> > Bye.

Bye.


More information about the Gc mailing list