RFR: 8168914: Crash in ClassLoaderData/JNIHandleBlock::oops_do during concurrent marking (original) (raw)

Kim Barrett kim.barrett at oracle.com
Thu Feb 16 17:55:55 UTC 2017

On Feb 16, 2017, at 8:18 AM, Erik Helin <erik.helin at oracle.com> wrote:

Hi Kim, thanks for reviewing! On 02/16/2017 03:41 AM, Kim Barrett wrote: ------------------------------------------------------------------------------ /src/share/vm/classfile/classLoaderData.cpp 138 const juint size = OrderAccess::loadacquire(&c->size);

I think all but the first chunk have a constant size of the maximum size, so no need for acquire except for the first. Please see my reply to my David. I agree that this can be optimized, but I'm not sure if it is worth it (the code will not become harder to understand)?

I knew when I made the suggestion that it might increase the raw footprint of the source code. But I agree with David’s sentiment that removing dynamically unnecessary load_acquires helps readability.

------------------------------------------------------------------------------ /src/share/vm/classfile/classLoaderData.cpp 173 VerifyContainsOopClosure cl(op); 174 oopsdo(&cl);

We don't care that this iterates over the entire list even if found early? Please see my reply to Thomas. We don't care :)

That’s what I guessed, hence making it a question. Okay.

------------------------------------------------------------------------------ /src/share/vm/classfile/classLoaderData.cpp 626 return (jobject) handles.add(h()); and 631 ((oop) h) = NULL;

Having spent a bunch of time recently dealing with and cleaning up a bunch of places that break the jobject abstraction (though by no means all), I'm not happy to see new ones. As Coleen said, there's discussion about how to refer to indirect pointers to oops in runtime code. jobject has been used as an answer for that in some places, but they might need to be changed. Hiding the conversions as casts will just make that harder. I'm thinking some named conversion functions are in order, instead of bare casts. Hmm, I would not like to introduce too much of Coleen's work in this patch. Do you have something minimal in mind that you think improves the situation? Or should we just note this place in the code and come back to it in 10?

Some sort of “convert_x_to_y” functions in JNIHandles. I added parts of one direction in 8166188, though that change has run into problems because more testing uncovered more abstraction breakage.

Thanks, Erik

More information about the hotspot-dev mailing list