bbeaudreault commented on code in PR #5016:
URL: https://github.com/apache/hbase/pull/5016#discussion_r1100233880
##########
hbase-common/src/main/java/org/apache/hadoop/hbase/io/TagCompressionContext.java:
##########
@@ -107,15 +107,15 @@ public void uncompressTags(InputStream src, byte[] dest,
int offset, int length)
throws IOException {
int endOffset = offset + length;
while (offset < endOffset) {
- byte status = (byte) src.read();
+ byte status = StreamUtils.readByte(src);
if (status == Dictionary.NOT_IN_DICTIONARY) {
int tagLen = StreamUtils.readRawVarint32(src);
offset = Bytes.putAsShort(dest, offset, tagLen);
IOUtils.readFully(src, dest, offset, tagLen);
- tagDict.addEntry(dest, offset, tagLen);
+ tagDict.findEntry(dest, offset, tagLen);
Review Comment:
Do you think this change could be expensive? In the normal case, the entry
will not exist in the dict. But now we're adding an extra map lookup for every
call. Granted o(1), but involves cpu for hashcode, allocating lookup key, etc.
I wonder if we could trigger findEntry only if context has been reset?
Otherwise use addEntry for first pass?
May not be a big issue, just checking
--
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]
