RFR (M): 8168467: Use TaskEntry as task mark queue elements
thomas.schatzl at oracle.com
Fri Mar 10 13:01:18 UTC 2017
On Thu, 2017-03-09 at 16:28 -0500, Kim Barrett wrote:
> > On Mar 8, 2017, at 5:31 AM, Thomas Schatzl <thomas.schatzl at oracle.c
> > om> wrote:
> > Hi,
> > On Tue, 2017-03-07 at 19:11 -0500, Kim Barrett wrote:
> > >
> > > >
> > I prepared a patch for how this would look like:
> > http://cr.openjdk.java.net/~tschatzl/8168467/webrev.1_to_2/
> > http://cr.openjdk.java.net/~tschatzl/8168467/webrev.2/
> > I will probably need to add a comment now why we use an extra
> > assign method instead of the operator now :-)
> > As for the change, if you insist on having an extra assign method
> > just to avoid this pragma and decrease readability of the uses of
> > that code, let's use webrev.2 as the current version.
> I agree the assign functioning approach is kind of ugly.
> But you still haven't answered my question as to whether the
> nonvolatile operator is actually needed, e.g. just use the volatile
> operator always.
I guess I simply did not understand your question (or the motivation
for your question)...
> But I finally remembered this:
> (Sorry it took so long to dredged this up, but it's been maybe a
> dozen years since I tripped over it.)
> Discussion there suggests returning volatile& is problematic, which
> seems right. The callers (GenericTaskQueue's push and push_slow) need
> to const_cast away the volatile and then cast to void the result.
> Much simpler would be an operation that didn't return volatile&
> (either a void returning volatile operator= or a named function).
> This is all getting rather far afield from the change at hand though.
> I suggest going with webrev.1 and a cleanup RFE around volatile
> operator=. (I suggest no volatile operator=, instead a volatile
> assign function, and change taskqueue push to use that.)
> I think StarTask and maybe the various flavors of oop are related.
> Hm, why doesn't the oop class need similar warning suppression?
I found that if CHECK_UNHANDLED_OOPS is enabled (to compile the
relevant code in oopsHierarchy.hpp), there is a global pragma to
disable this warning.
> If you agree, then webrev.1 looks good to me.
Agree. Let's keep webrev.1.
More information about the hotspot-gc-dev