JDK-8249001 : Add Copy::disjoint_bytes
  • Type: Enhancement
  • Component: hotspot
  • Sub-Component: runtime
  • Affected Version: 16
  • Priority: P4
  • Status: Closed
  • Resolution: Won't Fix
  • Submitted: 2020-07-07
  • Updated: 2024-01-16
  • Resolved: 2024-01-10
Related Reports
Blocks :  
JDK-8214976 - Warn about uses of functions replaced for portability
Relates :  
JDK-8219010 - Copy class should use templates
Relates :  
JDK-8301718 - Copy should be using memcpy/memset/memmove for everything bytes or non-atomic
Relates :  
JDK-8142349 - memset_with_concurrent_readers should be moved to copy.hpp
Description
We currently don't have Copy::disjoint_bytes. The lack of this function leads us to use bare memcpy in many places instead of using Copy, effectively denormalizing the use of Copy. Folks forget it even exists, because bare memcpy (and even the occasional bare memmove instead of Copy::conjoint_bytes) is so common in our code.

Also change uses of bare memcpy to use the new function.  Also change bare memmove's to Copy::conjoint_bytes.

We should also have an _atomic variant, to go with the existing Copy::conjoint_jbytes_atomic.
Comments
Regarding atomic variants, we need(ed) one even for byte-sized sets, due to SPARC BIS introducing intermediate unexpected (zero) values. And there's a completeness argument, to avoid having some operations via one group (Copy::) and another via a different group (libc or whatever).
16-01-2024
Runtime Triage: This is not on our current list of priorities. We will consider this feature if we receive additional customer requirements.
10-01-2024
We can look at outlawing use of memcpy even if the rest of JDK-8214976 doesn't go ahead.
09-02-2022
See [~kbarrett] comment above. Replacing memcpy with this function will not prevent people from backsliding into memcpy, so it doesn't seem worth doing just to have a layer on memcpy. Depending on code reviews isn't a reliable way to prevent it.
08-02-2022
> This is blocked by JDK-8214976 Why?
07-02-2022
This is blocked by JDK-8214976. If [~dholmes] objects to that change, this should be closed as WNF.
07-02-2022
I still want a verb.
06-01-2022
The conjoint/disjoint terminology is fairly standard data structure terminology. The problem with the plain copy_bytes is that you don't know by looking at it whether it supports conjoint or only disjoint. I'd rather we just keep the existing technically correct names.
06-01-2022
n-1 comment. Ok. That helps. My biggest problem with Copy::disjoint_bytes is that I've never liked the names of these functions. The name Copy::conjoint_bytes is like Huh? when I see it. memcpy is like of course I know what that does. And I don't have any idea why we have our own versions. std::memcpy - cppreference.comhttps://en.cppreference.com › cpp › string › byte › me... Apr 6, 2021 — std::memcpy is meant to be the fastest library routine for memory-to-memory copy. It is usually more efficient than std::strcpy, which must scan ... ‎Memcpy, memcpy_s · ‎Std::memmove · ‎Std::memset I can see having an atomic versions if needed for larger sizes than bytes. Or maybe we always want to have atomic versions? Maybe that's why we want to have our very own interface? I still don't like these names. How about these instead? Copy::copy_bytes/words/jshort etc. (assume disjoint like memcpy and can assert that). Copy::copy_overlapping_bytes/words/jshort etc ? "copy" is redundant with the name of the function but it is still nice to start the name with a verb. edit: there are a lot of these conjoint named functions.
05-01-2022
[~coleenp] JDK-8214976 should provide a solution to the backsliding usage problem.
05-01-2022
$ cd src/hotspot/share ; grep -r memcpy | wc -l 146 It's less than I thought. There are 14 memmoves in shared. Even if we replaced all of these with Copy::disjoint_bytes (ugh I've never liked these names), there's no way to make new changes to the code not use memcpy without having one of our HotSpoty rules to enforce while reading every code review that goes by. And try to explain why to new people. With solaris and the non-atomic version gone, I don't see the motivation to do this. Convince me otherwise.
04-01-2022
The risks of the non-atomic version(s) should be documented, so coders can properly trade off performance and correctness. Using the word "bytes" makes it more clear (than "memcpy") that address units larger than bytes might be temporarily corrupted by race conditions or similar effects.
08-07-2020