[jira] [Commented] (HBASE-9935) Slight perf improvement: Avoid KeyValue.getRowLength() at some places
[
https://issues.apache.org/jira/browse/HBASE-9935?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13818068#comment-13818068
]
Lars Hofhansl commented on HBASE-9935:
--
There are some more cases like this in lazy seeking. Also some of the methods
in KeyValue do similar things.
> Slight perf improvement: Avoid KeyValue.getRowLength() at some places
> -
>
> Key: HBASE-9935
> URL: https://issues.apache.org/jira/browse/HBASE-9935
> Project: HBase
> Issue Type: Bug
>Reporter: Lars Hofhansl
>
> Here's an example:
> {code}
> KeyValue.createLastOnRow(
> kv.getBuffer(), kv.getRowOffset(), kv.getRowLength(),
> kv.getBuffer(), kv.getFamilyOffset(), kv.getFamilyLength(),
>kv.getBuffer(), kv.getQualifierOffset(), kv.getQualifierLength());
> {code}
> Looks harmless enough, but that actually recalculates the rowlength 5 times.
> And each time it needs to decode the rowlength again from the bytes of the KV.
--
This message was sent by Atlassian JIRA
(v6.1#6144)
[jira] [Commented] (HBASE-9935) Slight perf improvement: Avoid KeyValue.getRowLength() at some places
[
https://issues.apache.org/jira/browse/HBASE-9935?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13818117#comment-13818117
]
Jean-Marc Spaggiari commented on HBASE-9935:
Nice one again. We should take more attention when we call external methodes on
the same object multiple time and see if we are not better to do it once and
store the result.
One improvement we can do here, is to store the row lenght and invalidate it
only if it changes? Like
{code}
@Override
public short getRowLength() {
if (rowLength == null)
rowLength = Bytes.toShort(this.bytes, getKeyOffset());
return rowLength;
}
{code}
And then each time we modify this.offset or this.byte we invalidate
rowLenght... So that will optimize everywhere even if we missed it? Looking
very quickly at the KV code, I have not seen anywhere except in the constructor
where this is modified. So might be a quick and small optimization again...
> Slight perf improvement: Avoid KeyValue.getRowLength() at some places
> -
>
> Key: HBASE-9935
> URL: https://issues.apache.org/jira/browse/HBASE-9935
> Project: HBase
> Issue Type: Bug
>Reporter: Lars Hofhansl
>
> Here's an example:
> {code}
> KeyValue.createLastOnRow(
> kv.getBuffer(), kv.getRowOffset(), kv.getRowLength(),
> kv.getBuffer(), kv.getFamilyOffset(), kv.getFamilyLength(),
>kv.getBuffer(), kv.getQualifierOffset(), kv.getQualifierLength());
> {code}
> Looks harmless enough, but that actually recalculates the rowlength 5 times.
> And each time it needs to decode the rowlength again from the bytes of the KV.
--
This message was sent by Atlassian JIRA
(v6.1#6144)
[jira] [Commented] (HBASE-9935) Slight perf improvement: Avoid KeyValue.getRowLength() at some places
[
https://issues.apache.org/jira/browse/HBASE-9935?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13818222#comment-13818222
]
Andrew Purtell commented on HBASE-9935:
---
Nice, I've been perpetuating this antipattern in some recent changes. Let's go
look...
> Slight perf improvement: Avoid KeyValue.getRowLength() at some places
> -
>
> Key: HBASE-9935
> URL: https://issues.apache.org/jira/browse/HBASE-9935
> Project: HBase
> Issue Type: Bug
>Reporter: Lars Hofhansl
>
> Here's an example:
> {code}
> KeyValue.createLastOnRow(
> kv.getBuffer(), kv.getRowOffset(), kv.getRowLength(),
> kv.getBuffer(), kv.getFamilyOffset(), kv.getFamilyLength(),
>kv.getBuffer(), kv.getQualifierOffset(), kv.getQualifierLength());
> {code}
> Looks harmless enough, but that actually recalculates the rowlength 5 times.
> And each time it needs to decode the rowlength again from the bytes of the KV.
--
This message was sent by Atlassian JIRA
(v6.1#6144)
[jira] [Commented] (HBASE-9935) Slight perf improvement: Avoid KeyValue.getRowLength() at some places
[
https://issues.apache.org/jira/browse/HBASE-9935?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13818256#comment-13818256
]
Lars Hofhansl commented on HBASE-9935:
--
[~jmspaggi], caching stuff because of a profiler session is itself an anti
pattern. See HBASE-7279 where I removed a bunch of stuff that we cached. Now,
we never cached the rowLength before and it is only short, so it might be OK.
I also want rationalize all these static methods we have on KeyValue. The
particular one I quote in the description should be an object method - and
interestingly there *is* a method on KeyValue that does exactly that call
createLastOnRowCol. If we use these or similar methods everywhere we can hide
the KeyValue specific optimizations inside KeyValue while still not requiring a
cache of the rowLength.
> Slight perf improvement: Avoid KeyValue.getRowLength() at some places
> -
>
> Key: HBASE-9935
> URL: https://issues.apache.org/jira/browse/HBASE-9935
> Project: HBase
> Issue Type: Bug
>Reporter: Lars Hofhansl
>
> Here's an example:
> {code}
> KeyValue.createLastOnRow(
> kv.getBuffer(), kv.getRowOffset(), kv.getRowLength(),
> kv.getBuffer(), kv.getFamilyOffset(), kv.getFamilyLength(),
>kv.getBuffer(), kv.getQualifierOffset(), kv.getQualifierLength());
> {code}
> Looks harmless enough, but that actually recalculates the rowlength 5 times.
> And each time it needs to decode the rowlength again from the bytes of the KV.
--
This message was sent by Atlassian JIRA
(v6.1#6144)
[jira] [Commented] (HBASE-9935) Slight perf improvement: Avoid KeyValue.getRowLength() at some places
[
https://issues.apache.org/jira/browse/HBASE-9935?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13818272#comment-13818272
]
Lars Hofhansl commented on HBASE-9935:
--
Meant to put a :) after "anti pattern" above.
> Slight perf improvement: Avoid KeyValue.getRowLength() at some places
> -
>
> Key: HBASE-9935
> URL: https://issues.apache.org/jira/browse/HBASE-9935
> Project: HBase
> Issue Type: Bug
>Reporter: Lars Hofhansl
>
> Here's an example:
> {code}
> KeyValue.createLastOnRow(
> kv.getBuffer(), kv.getRowOffset(), kv.getRowLength(),
> kv.getBuffer(), kv.getFamilyOffset(), kv.getFamilyLength(),
>kv.getBuffer(), kv.getQualifierOffset(), kv.getQualifierLength());
> {code}
> Looks harmless enough, but that actually recalculates the rowlength 5 times.
> And each time it needs to decode the rowlength again from the bytes of the KV.
--
This message was sent by Atlassian JIRA
(v6.1#6144)
[jira] [Commented] (HBASE-9935) Slight perf improvement: Avoid KeyValue.getRowLength() at some places
[
https://issues.apache.org/jira/browse/HBASE-9935?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13818282#comment-13818282
]
Lars Hofhansl commented on HBASE-9935:
--
Looking at KeyValue. The overall static size is still 68 bytes. 4 of which are
actually to cache the keyLength already. Adding 2 bytes to that might well
worth the effort. (Right now I am trying what happens when I *remove* 4 byte
cache for the keyLength).
> Slight perf improvement: Avoid KeyValue.getRowLength() at some places
> -
>
> Key: HBASE-9935
> URL: https://issues.apache.org/jira/browse/HBASE-9935
> Project: HBase
> Issue Type: Bug
>Reporter: Lars Hofhansl
>
> Here's an example:
> {code}
> KeyValue.createLastOnRow(
> kv.getBuffer(), kv.getRowOffset(), kv.getRowLength(),
> kv.getBuffer(), kv.getFamilyOffset(), kv.getFamilyLength(),
>kv.getBuffer(), kv.getQualifierOffset(), kv.getQualifierLength());
> {code}
> Looks harmless enough, but that actually recalculates the rowlength 5 times.
> And each time it needs to decode the rowlength again from the bytes of the KV.
--
This message was sent by Atlassian JIRA
(v6.1#6144)
[jira] [Commented] (HBASE-9935) Slight perf improvement: Avoid KeyValue.getRowLength() at some places
[
https://issues.apache.org/jira/browse/HBASE-9935?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13818287#comment-13818287
]
Lars Hofhansl commented on HBASE-9935:
--
Interestingly, when I remove the keyLength caching I see
KeyValue.getKeyLength() prominently in the profiler, but I can't measure any
performance detriment.
When I switch to a sampling profiler, I see the getRowLength() or
getKeyLength() almost never sees a hit. So this caching (and even the reason
why I filed this jira) might just be a profiler anomaly.
Maybe somebody from Intel ([~apurtell], hint hint) could have a look? If the
keyLength caching does not buy us much, we should remove it and save the memory.
> Slight perf improvement: Avoid KeyValue.getRowLength() at some places
> -
>
> Key: HBASE-9935
> URL: https://issues.apache.org/jira/browse/HBASE-9935
> Project: HBase
> Issue Type: Bug
>Reporter: Lars Hofhansl
>
> Here's an example:
> {code}
> KeyValue.createLastOnRow(
> kv.getBuffer(), kv.getRowOffset(), kv.getRowLength(),
> kv.getBuffer(), kv.getFamilyOffset(), kv.getFamilyLength(),
>kv.getBuffer(), kv.getQualifierOffset(), kv.getQualifierLength());
> {code}
> Looks harmless enough, but that actually recalculates the rowlength 5 times.
> And each time it needs to decode the rowlength again from the bytes of the KV.
--
This message was sent by Atlassian JIRA
(v6.1#6144)
[jira] [Commented] (HBASE-9935) Slight perf improvement: Avoid KeyValue.getRowLength() at some places
[
https://issues.apache.org/jira/browse/HBASE-9935?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13818605#comment-13818605
]
Lars Hofhansl commented on HBASE-9935:
--
Did more some tests.
The steady state of this is even improved when I remove the keyLength caching
from KeyValue. I my case I do full scans through 25m KVs, so in each run the
extra keyLength cache produces 100mb of extra garbage to be collected.
I also observed a slight slowdown (mostly within the noise, though) in this
scenario when I do the 2 byte rowLength caching.
So my proposal is this:
# remove the keyLength caching
# look through the callers of getFamilyOffset/getFamilyLength, etc, and see
where we can optimize this while hiding all of this in the KeyValue class
# no cache for the rowLength
> Slight perf improvement: Avoid KeyValue.getRowLength() at some places
> -
>
> Key: HBASE-9935
> URL: https://issues.apache.org/jira/browse/HBASE-9935
> Project: HBase
> Issue Type: Bug
>Reporter: Lars Hofhansl
>
> Here's an example:
> {code}
> KeyValue.createLastOnRow(
> kv.getBuffer(), kv.getRowOffset(), kv.getRowLength(),
> kv.getBuffer(), kv.getFamilyOffset(), kv.getFamilyLength(),
>kv.getBuffer(), kv.getQualifierOffset(), kv.getQualifierLength());
> {code}
> Looks harmless enough, but that actually recalculates the rowlength 5 times.
> And each time it needs to decode the rowlength again from the bytes of the KV.
--
This message was sent by Atlassian JIRA
(v6.1#6144)
[jira] [Commented] (HBASE-9935) Slight perf improvement: Avoid KeyValue.getRowLength() at some places
[
https://issues.apache.org/jira/browse/HBASE-9935?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13818625#comment-13818625
]
Lars Hofhansl commented on HBASE-9935:
--
I tried a big'ish patch. Didn't see any improvements. The JVM is probably smart
enough to do the right thing anyway. Unless somebody has some new ideas, I'll
close this as invalid (or maybe just remove the keyLenght caching).
> Slight perf improvement: Avoid KeyValue.getRowLength() at some places
> -
>
> Key: HBASE-9935
> URL: https://issues.apache.org/jira/browse/HBASE-9935
> Project: HBase
> Issue Type: Bug
>Reporter: Lars Hofhansl
>
> Here's an example:
> {code}
> KeyValue.createLastOnRow(
> kv.getBuffer(), kv.getRowOffset(), kv.getRowLength(),
> kv.getBuffer(), kv.getFamilyOffset(), kv.getFamilyLength(),
>kv.getBuffer(), kv.getQualifierOffset(), kv.getQualifierLength());
> {code}
> Looks harmless enough, but that actually recalculates the rowlength 5 times.
> And each time it needs to decode the rowlength again from the bytes of the KV.
--
This message was sent by Atlassian JIRA
(v6.1#6144)
[jira] [Commented] (HBASE-9935) Slight perf improvement: Avoid KeyValue.getRowLength() at some places
[
https://issues.apache.org/jira/browse/HBASE-9935?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13818957#comment-13818957
]
Jean-Marc Spaggiari commented on HBASE-9935:
Might be good for the GC time. Also, nice small improvement for
KeyValue.ROW_OFFSET move. But as you said, performance change is most probably
in the noise.
Since you are doing modifications to KeyValue, can you add the implement
Cloneable since we have a clone() method? And the @Override keyword for equals
and hashCode? ;)
> Slight perf improvement: Avoid KeyValue.getRowLength() at some places
> -
>
> Key: HBASE-9935
> URL: https://issues.apache.org/jira/browse/HBASE-9935
> Project: HBase
> Issue Type: Bug
>Reporter: Lars Hofhansl
>Priority: Minor
> Attachments: 9935-0.94.txt
>
>
> Here's an example:
> {code}
> KeyValue.createLastOnRow(
> kv.getBuffer(), kv.getRowOffset(), kv.getRowLength(),
> kv.getBuffer(), kv.getFamilyOffset(), kv.getFamilyLength(),
>kv.getBuffer(), kv.getQualifierOffset(), kv.getQualifierLength());
> {code}
> Looks harmless enough, but that actually recalculates the rowlength 5 times.
> And each time it needs to decode the rowlength again from the bytes of the KV.
--
This message was sent by Atlassian JIRA
(v6.1#6144)
[jira] [Commented] (HBASE-9935) Slight perf improvement: Avoid KeyValue.getRowLength() at some places
[
https://issues.apache.org/jira/browse/HBASE-9935?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13819172#comment-13819172
]
Lars Hofhansl commented on HBASE-9935:
--
He yes can do :)
I'll probably file another issue for the proposed KV changes and close this one.
(will also do a test on a machine with more cores, where CPU cycles are cheaper
and memory bandwidth is more of a limited commodity)
> Slight perf improvement: Avoid KeyValue.getRowLength() at some places
> -
>
> Key: HBASE-9935
> URL: https://issues.apache.org/jira/browse/HBASE-9935
> Project: HBase
> Issue Type: Bug
>Reporter: Lars Hofhansl
>Priority: Minor
> Attachments: 9935-0.94.txt
>
>
> Here's an example:
> {code}
> KeyValue.createLastOnRow(
> kv.getBuffer(), kv.getRowOffset(), kv.getRowLength(),
> kv.getBuffer(), kv.getFamilyOffset(), kv.getFamilyLength(),
>kv.getBuffer(), kv.getQualifierOffset(), kv.getQualifierLength());
> {code}
> Looks harmless enough, but that actually recalculates the rowlength 5 times.
> And each time it needs to decode the rowlength again from the bytes of the KV.
--
This message was sent by Atlassian JIRA
(v6.1#6144)
[jira] [Commented] (HBASE-9935) Slight perf improvement: Avoid KeyValue.getRowLength() at some places
[
https://issues.apache.org/jira/browse/HBASE-9935?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13819408#comment-13819408
]
stack commented on HBASE-9935:
--
+1 on removing caching of a keylength that provides no discernible benefit (I
probably added it after a 'profiling' session -- or at least added an ancestor
of current KV length caching).
> Slight perf improvement: Avoid KeyValue.getRowLength() at some places
> -
>
> Key: HBASE-9935
> URL: https://issues.apache.org/jira/browse/HBASE-9935
> Project: HBase
> Issue Type: Bug
>Reporter: Lars Hofhansl
>Priority: Minor
> Attachments: 9935-0.94.txt
>
>
> Here's an example:
> {code}
> KeyValue.createLastOnRow(
> kv.getBuffer(), kv.getRowOffset(), kv.getRowLength(),
> kv.getBuffer(), kv.getFamilyOffset(), kv.getFamilyLength(),
>kv.getBuffer(), kv.getQualifierOffset(), kv.getQualifierLength());
> {code}
> Looks harmless enough, but that actually recalculates the rowlength 5 times.
> And each time it needs to decode the rowlength again from the bytes of the KV.
--
This message was sent by Atlassian JIRA
(v6.1#6144)
[jira] [Commented] (HBASE-9935) Slight perf improvement: Avoid KeyValue.getRowLength() at some places
[
https://issues.apache.org/jira/browse/HBASE-9935?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13819527#comment-13819527
]
Lars Hofhansl commented on HBASE-9935:
--
Lemme do a bit more testing. I'll report back as soon as I have done that. In
either case it won't be a big perf improvement.
> Slight perf improvement: Avoid KeyValue.getRowLength() at some places
> -
>
> Key: HBASE-9935
> URL: https://issues.apache.org/jira/browse/HBASE-9935
> Project: HBase
> Issue Type: Bug
>Reporter: Lars Hofhansl
>Priority: Minor
> Attachments: 9935-0.94.txt
>
>
> Here's an example:
> {code}
> KeyValue.createLastOnRow(
> kv.getBuffer(), kv.getRowOffset(), kv.getRowLength(),
> kv.getBuffer(), kv.getFamilyOffset(), kv.getFamilyLength(),
>kv.getBuffer(), kv.getQualifierOffset(), kv.getQualifierLength());
> {code}
> Looks harmless enough, but that actually recalculates the rowlength 5 times.
> And each time it needs to decode the rowlength again from the bytes of the KV.
--
This message was sent by Atlassian JIRA
(v6.1#6144)
[jira] [Commented] (HBASE-9935) Slight perf improvement: Avoid KeyValue.getRowLength() at some places
[
https://issues.apache.org/jira/browse/HBASE-9935?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13819682#comment-13819682
]
Lars Hofhansl commented on HBASE-9935:
--
Tried on a 6 core (12 hw threads) machine. Inconclusive as before. Variance of
these test is quite hight (+- 5% runtime).
I'd say we remove the caching and reclaim those 4 bytes.
This issue itself was dud (sorry about that, it's easy to get fooled by the
profiler :) ).
I'll close this and open a new one for the keyLength removal.
> Slight perf improvement: Avoid KeyValue.getRowLength() at some places
> -
>
> Key: HBASE-9935
> URL: https://issues.apache.org/jira/browse/HBASE-9935
> Project: HBase
> Issue Type: Bug
>Reporter: Lars Hofhansl
>Priority: Minor
> Attachments: 9935-0.94.txt
>
>
> Here's an example:
> {code}
> KeyValue.createLastOnRow(
> kv.getBuffer(), kv.getRowOffset(), kv.getRowLength(),
> kv.getBuffer(), kv.getFamilyOffset(), kv.getFamilyLength(),
>kv.getBuffer(), kv.getQualifierOffset(), kv.getQualifierLength());
> {code}
> Looks harmless enough, but that actually recalculates the rowlength 5 times.
> And each time it needs to decode the rowlength again from the bytes of the KV.
--
This message was sent by Atlassian JIRA
(v6.1#6144)
[jira] [Commented] (HBASE-9935) Slight perf improvement: Avoid KeyValue.getRowLength() at some places
[
https://issues.apache.org/jira/browse/HBASE-9935?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13819689#comment-13819689
]
Lars Hofhansl commented on HBASE-9935:
--
Filed HBASE-9956.
> Slight perf improvement: Avoid KeyValue.getRowLength() at some places
> -
>
> Key: HBASE-9935
> URL: https://issues.apache.org/jira/browse/HBASE-9935
> Project: HBase
> Issue Type: Bug
>Reporter: Lars Hofhansl
>Priority: Minor
> Attachments: 9935-0.94.txt
>
>
> Here's an example:
> {code}
> KeyValue.createLastOnRow(
> kv.getBuffer(), kv.getRowOffset(), kv.getRowLength(),
> kv.getBuffer(), kv.getFamilyOffset(), kv.getFamilyLength(),
>kv.getBuffer(), kv.getQualifierOffset(), kv.getQualifierLength());
> {code}
> Looks harmless enough, but that actually recalculates the rowlength 5 times.
> And each time it needs to decode the rowlength again from the bytes of the KV.
--
This message was sent by Atlassian JIRA
(v6.1#6144)
