-
Notifications
You must be signed in to change notification settings - Fork 4k
protobuf, api, core, netty: zero copy into protobuf #7330
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 26 commits
9936df1
aa93e1d
454e224
3f47d5e
b3c9974
2ede525
3dca312
6249353
29dce67
22ea6c3
53e347c
27801fe
eb71a68
5d3c657
9fd8d3c
e3afe50
e2fdd07
033270b
0622d51
0e8caee
ba4e91b
69618b2
437857d
1363505
c46eb73
7b4e070
772b3ba
b1c99e5
692076c
10c13b8
f13c165
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| /* | ||
| * Copyright 2020 The gRPC Authors | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| package io.grpc; | ||
|
|
||
| import java.nio.ByteBuffer; | ||
| import javax.annotation.Nullable; | ||
|
|
||
| /** | ||
| * Extension to an {@link java.io.InputStream} whose content can be accessed as {@link | ||
| * ByteBuffer}s. | ||
| * | ||
| * <p>This can be used for optimizing the case for the consumer of a {@link ByteBuffer}-backed | ||
| * input stream supports efficient reading from {@link ByteBuffer}s directly. This turns the reader | ||
| * interface from an {@link java.io.InputStream} to {@link ByteBuffer}s, without copying the | ||
| * content to a byte array and read from it. | ||
| */ | ||
| // TODO(chengyuanzhang): add ExperimentalApi annotation. | ||
| public interface HasByteBuffer { | ||
|
|
||
| /** | ||
| * Indicates whether or not {@link #getByteBuffer} operation is supported. | ||
| */ | ||
| boolean getByteBufferSupported(); | ||
|
|
||
| /** | ||
| * Gets a {@link ByteBuffer} containing some bytes of the content next to be read, or {@code | ||
| * null} if has reached end of the content. The number of bytes contained in the returned buffer | ||
| * is implementation specific. Calling this method does not change the position of the input | ||
| * stream. The returned buffer's content should not be modified, but the position, limit, and | ||
| * mark may be changed. Operations for changing the position, limit, and mark of the returned | ||
| * buffer does not affect the position, limit, and mark of this input stream. This is an optional | ||
| * method, so callers should first check {@link #getByteBufferSupported}. | ||
| * | ||
| * @throws UnsupportedOperationException if this operation is not supported. | ||
| */ | ||
| @Nullable | ||
| ByteBuffer getByteBuffer(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To make the API cleaner how about having this return
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The semantics of returning |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,6 +20,7 @@ | |
| import java.io.IOException; | ||
| import java.io.OutputStream; | ||
| import java.nio.ByteBuffer; | ||
| import javax.annotation.Nullable; | ||
|
|
||
| /** | ||
| * Interface for an abstract byte buffer. Buffers are intended to be a read-only, except for the | ||
|
|
@@ -123,6 +124,39 @@ public interface ReadableBuffer extends Closeable { | |
| */ | ||
| int arrayOffset(); | ||
|
|
||
| /** | ||
| * Marks the current position in this buffer. A subsequent call to the {@link #reset} method | ||
| * repositions this stream at the last marked position so that subsequent reads re-read the same | ||
| * bytes. | ||
| */ | ||
| void mark(); | ||
|
|
||
| /** | ||
| * Repositions this buffer to the position at the time {@link #mark} was last called on this | ||
| * buffer. | ||
| */ | ||
| void reset(); | ||
|
|
||
| /** | ||
| * Indicates whether or not {@link #getByteBuffer} operation is supported for this buffer. | ||
| */ | ||
| boolean canUseByteBuffer(); | ||
|
||
|
|
||
| /** | ||
| * Gets a {@link ByteBuffer} that contains some bytes of the content next to be read, or {@code | ||
| * null} if this buffer has been exhausted. The number of bytes contained in the returned buffer | ||
| * is implementation specific. The position of this buffer is unchanged after calling this | ||
| * method. The returned buffer's content should not be modified, but the position, limit, and | ||
| * mark may be changed. Operations for changing the position, limit, and mark of the returned | ||
| * buffer does not affect the position, limit, and mark of this buffer. Buffers returned by this | ||
| * method have independent position, limit and mark. This is an optional method, so callers | ||
| * should first check {@link #canUseByteBuffer}. | ||
| * | ||
| * @throws UnsupportedOperationException the buffer does not support this method. | ||
| */ | ||
| @Nullable | ||
| ByteBuffer getByteBuffer(); | ||
|
|
||
| /** | ||
| * Closes this buffer and releases any resources. | ||
| */ | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not give this the same name as the equivalent added
ReadableBuffermethod (my suggestion would behasByteBuffer())?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't need to match what we use for
ReadableBufferright? We'd probably have a discussion on this API this Thursday and we usually do a vote for the name.hasByteBuffer1 vote now.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As well as for consistency, having it line up with
ReadableBufferwould permitReadableBuffers to themselves beInputStreams implementing this interface, which I think might allow for some other streamlining later.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe, for that reason.
My original thought was this
HasByteBufferinterface should only have thegetByteBuffermethod. InputStreams not supporting the operation should just not implement this interface. #7330 (comment) suggests combining two InputStream implementations into one by adding thisgetByteBufferSupportedmethod. This look ok, but making the methodhasByteBufferdoes make this API wired. I'd rather change thehasByteBuffermethod onReadableBufferinterface togetByteBufferSupported.