RFR(XXS): 8227338: templateInterpreter.cpp: copy_table() needs to be safer

[Kim Barrett]

> On Jul 9, 2019, at 9:19 AM, Daniel D. Daugherty <daniel.daugherty at oracle.com> wrote:
> On 7/8/19 7:08 PM, Kim Barrett wrote:
>> src/hotspot/share/interpreter/templateInterpreter.cpp
>>  286     while (size-- > 0) *to++ = *from++;
>> 
>> [pre-existing]
>> 
>> This ought to be using Copy::disjoint_words.  That's even more obvious
>> in conjunction with the change to use Copy::disjoint_words_atomic in
>> the non-safepoint case.
> 
> I can make that change. Is there a specific advantage/reason that you
> have in mind here?

Mostly a “if we’re going to have these kinds of utility APIs because we think they
are useful, then we really ought to use them” argument.  One might benefit from
some highly tuned memcpy-like thing provided by the per-platform implementation
of Copy::disjoint_words.  Of course, the code is sufficiently simple that a compiler
has a reason chance of making the appropriate transformation even without us
telling it.  The non-performance reason is a named operation is easier to read and
understand than the corresponding explicit loop.

>> src/hotspot/share/interpreter/templateInterpreter.cpp
>>  284   if (SafepointSynchronize::is_at_safepoint()) {
>> 
>> I wonder how much benefit we really get from having distinct safepoint
>> and non-safepoint cases, rather than just unconditionally using
>> Copy::disjoint_words_atomic.
> 
> Sorry, I don't know the answer to that. My intention was to use
> Copy::disjoint_words_atomic() only in the case where I knew that
> I needed it so no potential impact on existing uses at a safepoint.

Yeah, I just wasn’t sure how performance critical this copy is.  Hm, I see that it might
affect the time to get out of a safepoint, so potentially getting a highly tuned
platform-specific memcpy operation in the safepoint case might indeed be worthwhile.
So okay.