Avoid some GCC 10.X warnings in HotSpot
kim.barrett at oracle.com
Tue Jul 7 23:29:17 UTC 2020
> On Jul 7, 2020, at 12:45 PM, John Rose <john.r.rose at oracle.com> wrote:
> On Jul 7, 2020, at 7:36 AM, Kim Barrett <kim.barrett at oracle.com> wrote:
>> Using memcpy instead of byte_at_put (and getting rid of byte_at_put)
>> seems like a good idea to me.
> We have had problems with memcpy in the long past, and I’m
> personally still nervous when I see a call to it. The problem is
> subtle, and escaped everybody’s notice the first time around,
> and I’d prefer not to make more bugs of the same kind.
> What problem? [… snipped long discussion …]
> All that said, I have no problem with memcpy being
> used (though I prefer a locally documented alias!) in
> the places where it is appearing in our source base.
> Those places are, yes, private to some construction
> process, or singly-threaded for some other reason,
> perhaps a safepoint. But I think you understand now
> that I am nervous when I see more and more uses
> of memcpy in our code, because I think that now
> it’s just a matter of time before someone concludes
> that memcpy is the new (old) best way to copy data,
> and the hard work of codifying our practices in
> Copy will start to be neglected. A new programmer
> in our code base could make the mistake of just
> reaching for memcpy instead of doing the work of
> deciding which kind of Copy to use. And that will
> eventually cause a difficult-to-detect bug. Let’s not.
> — John
[This is somewhat summarizing an off-email discussion John and I had.]
It shouldn't be surprising that we have uses of bare memcpy, since we
don't have Copy::disjoint_bytes. In the particular case at hand,
there's no issue of word tearing since we aren't copying words, and
we're not even doing an aligned copy. But I wouldn't object to, and in
fact would encourage, the use of Copy::disjoint_bytes there if it
existed. Not having that function effectively denormalizes Copy in
favor of bare memcpy, making it easy to forget that Copy even exists.
I think the split between bare memcpy and Copy::conjoint_words in
HotSpot currently is close to even. And there are a score or so bare
I think the places where word tearing is an issue ought to be using
one of the "atomic" Copy functions. (And it's not just word tearing
that can be an issue, as we found with SPARC BIS. You convinced me
some time ago that memset_with_concurrent_readers (my fault for that)
should have been an "atomic" Copy function. I'll probably deal with
that RFE soon; it's much easier now that SPARC has been removed.)
All of this is something of a digression from the change at hand. In
the absence of Copy::disjoint_bytes (which this change shouldn't be
worrying about), I think using memcpy is fine for this change.
More information about the hotspot-runtime-dev