Index: hbase-common/src/main/java/org/apache/hadoop/hbase/KeyValue.java =================================================================== --- hbase-common/src/main/java/org/apache/hadoop/hbase/KeyValue.java (revision 1417289) +++ hbase-common/src/main/java/org/apache/hadoop/hbase/KeyValue.java (working copy) @@ -248,9 +248,6 @@ private int offset = 0; private int length = 0; - // the row cached - private volatile byte [] rowCache = null; - /** * @return True if a delete type, a {@link KeyValue.Type#Delete} or * a {KeyValue.Type#DeleteFamily} or a {@link KeyValue.Type#DeleteColumn} @@ -1240,7 +1237,6 @@ int tsOffset = getTimestampOffset(); System.arraycopy(now, 0, this.bytes, tsOffset, Bytes.SIZEOF_LONG); // clear cache or else getTimestamp() possibly returns an old value - timestampCache = -1L; return true; } return false; @@ -1312,29 +1308,20 @@ * @return Row in a new byte array. */ public byte [] getRow() { - if (rowCache == null) { - int o = getRowOffset(); - short l = getRowLength(); - // initialize and copy the data into a local variable - // in case multiple threads race here. - byte local[] = new byte[l]; - System.arraycopy(getBuffer(), o, local, 0, l); - rowCache = local; // volatile assign - } - return rowCache; + int o = getRowOffset(); + short l = getRowLength(); + byte result[] = new byte[l]; + System.arraycopy(getBuffer(), o, result, 0, l); + return result; } /** * * @return Timestamp */ - private long timestampCache = -1; @Override public long getTimestamp() { - if (timestampCache == -1) { - timestampCache = getTimestamp(getKeyLength()); - } - return timestampCache; + return getTimestamp(getKeyLength()); } /** @@ -2639,12 +2626,11 @@ public long heapSize() { int sum = 0; sum += ClassSize.OBJECT;// the KeyValue object itself - sum += 2 * ClassSize.REFERENCE;// 2 * pointers to "bytes" and "rowCache" byte[] - sum += 2 * ClassSize.align(ClassSize.ARRAY);// 2 * "bytes" and "rowCache" byte[] + sum += ClassSize.REFERENCE;// pointer to "bytes" + sum += ClassSize.align(ClassSize.ARRAY);// "bytes" sum += ClassSize.align(length);// number of bytes of data in the "bytes" array - //ignore the data in the rowCache because it is cleared for inactive memstore KeyValues sum += 3 * Bytes.SIZEOF_INT;// offset, length, keyLength - sum += 2 * Bytes.SIZEOF_LONG;// timestampCache, memstoreTS + sum += Bytes.SIZEOF_LONG;// memstoreTS return ClassSize.align(sum); } @@ -2652,10 +2638,8 @@ // and it expects the length of the KeyValue to be explicitly passed // to it. public void readFields(int length, final DataInput in) throws IOException { - this.rowCache = null; this.length = length; this.offset = 0; - this.timestampCache = -1; this.keyLength = 0; this.bytes = new byte[this.length]; in.readFully(this.bytes, 0, this.length); Index: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java =================================================================== --- hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java (revision 1417289) +++ hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java (working copy) @@ -3536,17 +3536,25 @@ rpcCall.throwExceptionIfCallerDisconnected(); } - byte [] currentRow = peekRow(); - if (isStopRow(currentRow)) { + KeyValue current = this.storeHeap.peek(); + byte[] currentRow = null; + int offset = 0; + short length = 0; + if (current != null) { + currentRow = current.getBuffer(); + offset = current.getRowOffset(); + length = current.getRowLength(); + } + if (isStopRow(currentRow, offset, length)) { if (filter != null && filter.hasFilterRow()) { filter.filterRow(results); } return false; - } else if (filterRowKey(currentRow)) { - nextRow(currentRow); + } else if (filterRowKey(currentRow, offset, length)) { + nextRow(currentRow, offset, length); } else { - byte [] nextRow; + KeyValue nextKv; do { this.storeHeap.next(results, limit - results.size(), metric); if (limit > 0 && results.size() == limit) { @@ -3556,9 +3564,10 @@ } return true; // we are expecting more yes, but also limited to how many we can return. } - } while (Bytes.equals(currentRow, nextRow = peekRow())); + nextKv = this.storeHeap.peek(); + } while (nextKv != null && nextKv.matchingRow(currentRow, offset, length)); - final boolean stopRow = isStopRow(nextRow); + final boolean stopRow = nextKv == null || isStopRow(nextKv.getBuffer(), nextKv.getRowOffset(), nextKv.getRowLength()); // now that we have an entire row, lets process with a filters: @@ -3573,7 +3582,7 @@ // the reasons for calling this method are: // 1. reset the filters. // 2. provide a hook to fast forward the row (used by subclasses) - nextRow(currentRow); + nextRow(currentRow, offset, length); // This row was totally filtered out, if this is NOT the last row, // we should continue on. @@ -3585,33 +3594,25 @@ } } - private boolean filterRow() { + private boolean filterRowKey(byte[] row, int offset, short length) { return filter != null - && filter.filterRow(); + && filter.filterRowKey(row, offset, length); } - private boolean filterRowKey(byte[] row) { - return filter != null - && filter.filterRowKey(row, 0, row.length); - } - protected void nextRow(byte [] currentRow) throws IOException { - while (Bytes.equals(currentRow, peekRow())) { - this.storeHeap.next(MOCKED_LIST); + protected void nextRow(byte [] currentRow, int offset, short length) throws IOException { + KeyValue next; + while((next = this.storeHeap.peek()) != null && next.matchingRow(currentRow, offset, length)) { + this.storeHeap.next(MOCKED_LIST); } results.clear(); resetFilters(); } - private byte[] peekRow() { - KeyValue kv = this.storeHeap.peek(); - return kv == null ? null : kv.getRow(); - } - - private boolean isStopRow(byte [] currentRow) { + private boolean isStopRow(byte [] currentRow, int offset, short length) { return currentRow == null || (stopRow != null && comparator.compareRows(stopRow, 0, stopRow.length, - currentRow, 0, currentRow.length) <= isScan); + currentRow, offset, length) <= isScan); } @Override Index: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanQueryMatcher.java =================================================================== --- hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanQueryMatcher.java (revision 1417289) +++ hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanQueryMatcher.java (working copy) @@ -83,6 +83,8 @@ /* row is not private for tests */ /** Row the query is on */ byte [] row; + int rowOffset; + short rowLength; /** * Oldest put in any of the involved store files @@ -223,7 +225,7 @@ short rowLength = Bytes.toShort(bytes, offset, Bytes.SIZEOF_SHORT); offset += Bytes.SIZEOF_SHORT; - int ret = this.rowComparator.compareRows(row, 0, row.length, + int ret = this.rowComparator.compareRows(row, this.rowOffset, this.rowLength, bytes, offset, rowLength); if (ret <= -1) { return MatchCode.DONE; @@ -386,8 +388,10 @@ * Set current row * @param row */ - public void setRow(byte [] row) { + public void setRow(byte [] row, int offset, short length) { this.row = row; + this.rowOffset = offset; + this.rowLength = length; reset(); } Index: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java =================================================================== --- hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java (revision 1417289) +++ hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java (working copy) @@ -53,7 +53,6 @@ private int storeLimit = -1; private int storeOffset = 0; - private String metricNamePrefix; // Used to indicate that the scanner has closed (see HBASE-1107) // Doesnt need to be volatile because it's always accessed via synchronized methods private boolean closing = false; @@ -328,9 +327,11 @@ // only call setRow if the row changes; avoids confusing the query matcher // if scanning intra-row - if ((matcher.row == null) || !peeked.matchingRow(matcher.row)) { - this.countPerRow = 0; - matcher.setRow(peeked.getRow()); + byte[] row = peeked.getBuffer(); + int offset = peeked.getRowOffset(); + short length = peeked.getRowLength(); + if ((matcher.row == null) || !Bytes.equals(row, offset, length, matcher.row, matcher.rowOffset, matcher.rowLength)) { + matcher.setRow(row, offset, length); } KeyValue kv; @@ -529,10 +530,13 @@ if (kv == null) { kv = lastTopKey; } - if ((matcher.row == null) || !kv.matchingRow(matcher.row)) { + byte[] row = kv.getBuffer(); + int offset = kv.getRowOffset(); + short length = kv.getRowLength(); + if ((matcher.row == null) || !Bytes.equals(row, offset, length, matcher.row, matcher.rowOffset, matcher.rowLength)) { this.countPerRow = 0; matcher.reset(); - matcher.setRow(kv.getRow()); + matcher.setRow(row, offset, length); } } Index: hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestQueryMatcher.java =================================================================== --- hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestQueryMatcher.java (revision 1417289) +++ hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestQueryMatcher.java (working copy) @@ -112,7 +112,8 @@ memstore.add(new KeyValue(row2, fam1, col1, data)); List actual = new ArrayList(); - qm.setRow(memstore.get(0).getRow()); + KeyValue k = memstore.get(0); + qm.setRow(k.getBuffer(), k.getRowOffset(), k.getRowLength()); for (KeyValue kv : memstore){ actual.add(qm.match(kv)); @@ -157,7 +158,8 @@ List actual = new ArrayList(); - qm.setRow(memstore.get(0).getRow()); + KeyValue k = memstore.get(0); + qm.setRow(k.getBuffer(), k.getRowOffset(), k.getRowLength()); for(KeyValue kv : memstore) { actual.add(qm.match(kv)); @@ -209,7 +211,8 @@ new KeyValue(row2, fam1, col1, now-10, data) }; - qm.setRow(kvs[0].getRow()); + KeyValue k = kvs[0]; + qm.setRow(k.getBuffer(), k.getRowOffset(), k.getRowLength()); List actual = new ArrayList(kvs.length); for (KeyValue kv : kvs) { @@ -261,7 +264,8 @@ new KeyValue(row1, fam2, col5, now-10000, data), new KeyValue(row2, fam1, col1, now-10, data) }; - qm.setRow(kvs[0].getRow()); + KeyValue k = kvs[0]; + qm.setRow(k.getBuffer(), k.getRowOffset(), k.getRowLength()); List actual = new ArrayList(kvs.length);