Skip to content

HDDS-9611. Avoid unnecessary buffer conversion.#5542

Merged
adoroszlai merged 4 commits intoapache:masterfrom
szetszwo:HDDS-9611
Nov 6, 2023
Merged

HDDS-9611. Avoid unnecessary buffer conversion.#5542
adoroszlai merged 4 commits intoapache:masterfrom
szetszwo:HDDS-9611

Conversation

@szetszwo
Copy link
Contributor

@szetszwo szetszwo commented Nov 2, 2023

What changes were proposed in this pull request?

In KeyValueHandler.validateChunkChecksumData(..), it converts ChunkBuffer to ByteString, which will be converted back to ChunkBuffer.

Also includes code cleanup:

  • avoid passing buffer capacity as long;
  • check for integer overflow

All the changes were discovered in HDDS-9536.

What is the link to the Apache JIRA

HDDS-9611

How was this patch tested?

Updating existing tests.

@szetszwo szetszwo changed the title iHDDS-9611. Avoid unnecessary buffer conversion. HDDS-9611. Avoid unnecessary buffer conversion. Nov 2, 2023
@szetszwo szetszwo requested a review from duongkame November 2, 2023 22:46
Copy link
Contributor

@adoroszlai adoroszlai left a comment

Choose a reason for hiding this comment

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

Thanks @szetszwo for the patch. LGTM, but there are some conflicts due to recent commit e4ff5fa.

final ByteBuffer[] buffers = BufferUtils.assignByteBuffers(len,
bufferCapacity);
readMethod.accept(buffers);
return ChunkBuffer.wrap(Lists.newArrayList(buffers));
Copy link
Contributor

Choose a reason for hiding this comment

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

ChunkBuffer doesn't change the list's size, and it creates a defensive copy anyway. So I think we could use Arrays.asList() here.

@szetszwo
Copy link
Contributor Author

szetszwo commented Nov 4, 2023

@adoroszlai , Thanks for reviewing this! Sync'ed with the master branch and using Arrays.asList() now.

@szetszwo szetszwo requested a review from adoroszlai November 6, 2023 06:14
@adoroszlai adoroszlai merged commit 9f67f4b into apache:master Nov 6, 2023
@szetszwo
Copy link
Contributor Author

szetszwo commented Nov 7, 2023

@adoroszlai , thanks for merging this!

ibrusentsev pushed a commit to ibrusentsev/ozone that referenced this pull request Nov 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants