Skip to content

HDDS-10480. Avoid proto2 ByteString.toByteArray() calls. - #6342

Merged
szetszwo merged 2 commits into
apache:masterfrom
szetszwo:HDDS-10480
Mar 8, 2024
Merged

HDDS-10480. Avoid proto2 ByteString.toByteArray() calls.#6342
szetszwo merged 2 commits into
apache:masterfrom
szetszwo:HDDS-10480

Conversation

@szetszwo

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

Some proto2 ByteString.toByteArray() calls can be removed in order to avoid buffer copying.

What is the link to the Apache JIRA

HDDS-10480

How was this patch tested?

By existing tests.

@szetszwo
szetszwo requested a review from duongkameMarch 7, 2024 17:13

final SCMRatisResponseProto responseProto = SCMRatisResponseProto
.parseFrom(response);
final SCMRatisResponseProto responseProto = SCMRatisResponseProto.parseFrom(response.toByteArray());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can use the below to avoid creating new buffer too.

final SCMRatisResponseProto responseProto = SCMRatisResponseProto.parseFrom(response.newInput());

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

@duongkame , thanks for reviewing this!

The method parseFrom(InputStream) will call new CodedInputStream(InputStream), which creates a byte[] in the CodedInputStream constructor. Since it is InputStream, it probably will copy at least two times.

@szetszwo
szetszwo requested a review from duongkameMarch 8, 2024 00:34

@duongkameduongkame left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the patch @szetszwo . LGTM.

@szetszwo

Copy link
Copy Markdown
ContributorAuthor

@duongkame , thanks a lot for reviewing this!

@szetszwo
szetszwo merged commit d94aadc into apache:masterMar 8, 2024
xichen01 pushed a commit to xichen01/ozone that referenced this pull request Oct 16, 2024
Sign up for freeto 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

@szetszwo@duongkame