Skip to content

Conversation

robfrank
Copy link
Collaborator

What does this PR do?

removes the use sun.mis.Unsafe

Motivation

Unsafe will be removed

Related issues

#1933

Checklist

  • I have run the build using mvn clean package command
  • My unit tests cover both failure and success scenarios

@robfrank robfrank marked this pull request as draft January 25, 2025 07:26
Copy link

codacy-production bot commented Jan 25, 2025

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
-6.17%
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (7800b87) 69934 44795 64.05%
Head commit (bc5db5a) 15755 (-54179) 9120 (-35675) 57.89% (-6.17%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#1934) 0 0 ∅ (not applicable)

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

@lvca lvca self-requested a review January 25, 2025 22:56
@lvca lvca added this to the 25.2.1 milestone Jan 25, 2025
@lvca lvca added the enhancement New feature or request label Jan 25, 2025
@lvca lvca modified the milestones: 25.2.1, 25.3.1 Feb 27, 2025
@lvca lvca modified the milestones: 25.3.1, 25.3.2 Mar 7, 2025
@lvca lvca modified the milestones: 25.3.2, 25.4.1 Mar 24, 2025
@robfrank robfrank modified the milestones: 25.4.1, 25.5.1 Apr 22, 2025
@robfrank robfrank force-pushed the feature/1933-remove-unsafe branch from 2530f6a to 9d158fb Compare May 11, 2025 16:09
@robfrank robfrank modified the milestones: 25.5.1, 25.6.1 May 30, 2025
@lvca lvca modified the milestones: 25.6.1, 25.8.1 Jul 4, 2025
@lvca lvca modified the milestones: 25.8.1, 25.6.1 Jul 4, 2025
@lvca
Copy link
Contributor

lvca commented Jul 4, 2025

It looks good to me!

@robfrank robfrank force-pushed the feature/1933-remove-unsafe branch from bc5db5a to 6a0f578 Compare July 4, 2025 12:35
@lvca lvca modified the milestones: 25.6.1, 25.7.1 Jul 4, 2025
final long rw = theUnsafe.getLong(right, BYTE_ARRAY_BASE_OFFSET + (long) i);
if (lw != rw)
return false;
try (Arena arena = Arena.ofConfined()) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you're using MemorySegments, I think this code could be simplified (and likely doesn't need the Arena given these are heap allocated JVM arrays):

// assumes left and right are non-null and length is non-zero
if (left.length < length || right.length < length) return false;

MemorySegment leftSegment = MemorySegment.ofArray(left).asSlice(0, length);
MemorySegment rightSegment = MemorySegment.ofArray(right).asSlice(0, length);

// mismatch is optimized and should be faster than a for loop
return leftSegment.mismatch(rightSegment) == -1;

Compare needs to take into account the possibility that left or right is a prefix of the other, but would use the same general flow:

// assumes left and right are non-null
MemorySegment leftSegment = MemorySegment.ofArray(left);
MemorySegment rightSegment = MemorySegment.ofArray(right);

long index = leftSegment.mismatch(rightSegment);
if (index == -1) {
    return Integer.compare(left.length, right.length);
}

// index is either the byte offset which differs or the length of the shorter array
if (index >= left.length || index >= right.length) {
    return Integer.compare(left.length, right.length);
}

return Integer.compare(Byte.toUnsignedInt(left[(int)index]), Byte.toUnsignedInt(right[(int)index]));

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, thank you!

@robfrank robfrank modified the milestones: 25.7.1, 25.8.1 Aug 11, 2025
@lvca lvca removed this from the 25.8.1 milestone Sep 10, 2025
@lvca lvca added this to the 25.10.1 milestone Sep 10, 2025
@robfrank robfrank force-pushed the feature/1933-remove-unsafe branch from 6a0f578 to fb4c355 Compare October 10, 2025 19:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants