bbeaudreault commented on code in PR #5576:
URL: https://github.com/apache/hbase/pull/5576#discussion_r1439753125
##########
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 did
not?
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 is implemented) because it
doesn't incur an extra checkRefCount() call). That might actually account for
some of the perf difference between this and readVLongTimestamp at the larger
values.
--
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]
