jbewing commented on code in PR #5576:
URL: https://github.com/apache/hbase/pull/5576#discussion_r1439908082
##########
hbase-common/src/main/java/org/apache/hadoop/hbase/util/ByteBufferUtils.java:
##########
@@ -468,38 +468,75 @@ public static void writeVLong(ByteBuffer out, long i) {
}
}
- private interface ByteVisitor {
- byte get();
- }
-
- private static long readVLong(ByteVisitor visitor) {
- byte firstByte = visitor.get();
+ /**
+ * Similar to {@link WritableUtils#readVLong(java.io.DataInput)} but reads
from a
+ * {@link ByteBuff}.
+ */
+ public static long readVLong(ByteBuff buf) {
+ byte firstByte = buf.get();
int len = WritableUtils.decodeVIntSize(firstByte);
if (len == 1) {
return firstByte;
+ } else {
+ int remaining = len - 1;
+ long i = 0;
+ int offsetFromPos = 0;
+ if (remaining >= Bytes.SIZEOF_INT) {
+ // The int read has to be converted to unsigned long so the & op
+ i = (buf.getIntAfterPosition(offsetFromPos) & 0x00000000ffffffffL);
+ remaining -= Bytes.SIZEOF_INT;
+ offsetFromPos += Bytes.SIZEOF_INT;
+ }
+ if (remaining >= Bytes.SIZEOF_SHORT) {
+ short s = buf.getShortAfterPosition(offsetFromPos);
+ i = i << 16;
+ i = i | (s & 0xFFFF);
+ remaining -= Bytes.SIZEOF_SHORT;
+ offsetFromPos += Bytes.SIZEOF_SHORT;
+ }
+ for (int idx = 0; idx < remaining; idx++) {
+ byte b = buf.getByteAfterPosition(offsetFromPos + idx);
+ i = i << 8;
+ i = i | (b & 0xFF);
+ }
+ buf.skip(len - 1);
+ return WritableUtils.isNegativeVInt(firstByte) ? ~i : i;
}
- long i = 0;
- for (int idx = 0; idx < len - 1; idx++) {
- byte b = visitor.get();
- i = i << 8;
- i = i | (b & 0xFF);
- }
- return (WritableUtils.isNegativeVInt(firstByte) ? (i ^ -1L) : i);
}
/**
* Similar to {@link WritableUtils#readVLong(DataInput)} but reads from a
{@link ByteBuffer}.
*/
- public static long readVLong(ByteBuffer in) {
- return readVLong(in::get);
- }
-
- /**
- * Similar to {@link WritableUtils#readVLong(java.io.DataInput)} but reads
from a
- * {@link ByteBuff}.
- */
- public static long readVLong(ByteBuff in) {
- return readVLong(in::get);
+ public static long readVLong(ByteBuffer buf) {
+ byte firstByte = buf.get();
+ int len = WritableUtils.decodeVIntSize(firstByte);
+ if (len == 1) {
+ return firstByte;
+ } else {
+ int remaining = len - 1;
+ long i = 0;
+ int offsetFromPos = 0;
+ if (remaining >= Bytes.SIZEOF_INT) {
+ // The int read has to be converted to unsigned long so the & op
+ i = (buf.getInt(buf.position() + offsetFromPos) & 0x00000000ffffffffL);
Review Comment:
> why do we need to do this & here when the original impl in
[HBASE-14186](https://issues.apache.org/jira/browse/HBASE-14186) did not?
This is needed as the original implementation from
[HBASE-14186](https://issues.apache.org/jira/browse/HBASE-14186) has a small
bug which was fixed by
[HBASE-16624](https://issues.apache.org/jira/browse/HBASE-16624).
> Also fwiw, you can use getIntAfterPosition() (same for
getShortAfterPosition() and getByteAfterPosition() below), to avoid the
duplication of calling buf.position() + offset in each of these. This will also
have a minor perf benefit (until
[HBASE-27730](https://issues.apache.org/jira/browse/HBASE-27730) is
implemented) because it doesn't incur an extra checkRefCount() call).
This is the implementation for Java NIO ByteBuffer's which AFAIK don't have
the same `getIntAfterPosition` as HBase's ByteBuff class. These ports are
pretty direct translations from HBase ByteBuff -> Java ByteBuffer. Since these
are Java ByteBuffer's there shouldn't be any performance penalty for calling
`position()` since it's not gated by a RefCnt check
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
