[jira] [Commented] (HBASE-19684) BlockCacheKey toString Performance
[
https://issues.apache.org/jira/browse/HBASE-19684?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16316317#comment-16316317
]
Hudson commented on HBASE-19684:
FAILURE: Integrated in Jenkins build HBase-Trunk_matrix #4363 (See
[https://builds.apache.org/job/HBase-Trunk_matrix/4363/])
HBASE-19684 BlockCacheKey toString Performance (zhangduo: rev
afc2cdbaffca6cb1f8495451a0b0940ad1fc4be0)
* (edit)
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/BlockCacheKey.java
> BlockCacheKey toString Performance
> --
>
> Key: HBASE-19684
> URL: https://issues.apache.org/jira/browse/HBASE-19684
> Project: HBase
> Issue Type: Improvement
> Components: hbase
>Affects Versions: 3.0.0
>Reporter: BELUGA BEHR
>Assignee: BELUGA BEHR
>Priority: Trivial
> Fix For: 3.0.0, 1.3.2, 1.4.1, 1.5.0, 1.2.7, 2.0.0-beta-2
>
> Attachments: HBASE-19684.1.patch, HBASE-19684.2.patch
>
>
> {code:titile=BlockCacheKey.java}
> @Override
> public String toString() {
> return String.format("%s_%d", hfileName, offset);
> }
> {code}
> I found through bench-marking that the following code is 10x faster.
> {code:titi\le=BlockCacheKey.java}
> @Override
> public String toString() {
> return hfileName.concat("_").concat(Long.toString(offset));
> }
> {code}
> Normally it wouldn't matter for a _toString()_ method, but this is comes into
> play because {{MemcachedBlockCache}} uses it.
> {code:title=MemcachedBlockCache.java}
> @Override
> public void cacheBlock(BlockCacheKey cacheKey, Cacheable buf) {
> if (buf instanceof HFileBlock) {
> client.add(cacheKey.toString(), MAX_SIZE, (HFileBlock) buf, tc);
> } else {
> if (LOG.isDebugEnabled()) {
> LOG.debug("MemcachedBlockCache can not cache Cacheable's of type "
> + buf.getClass().toString());
> }
> }
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (HBASE-19684) BlockCacheKey toString Performance
[
https://issues.apache.org/jira/browse/HBASE-19684?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16316039#comment-16316039
]
Hudson commented on HBASE-19684:
SUCCESS: Integrated in Jenkins build HBase-1.3-IT #331 (See
[https://builds.apache.org/job/HBase-1.3-IT/331/])
HBASE-19684 BlockCacheKey toString Performance (zhangduo: rev
21dd7ac3227efd33409851fe0a9dff057bed537a)
* (edit)
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/BlockCacheKey.java
> BlockCacheKey toString Performance
> --
>
> Key: HBASE-19684
> URL: https://issues.apache.org/jira/browse/HBASE-19684
> Project: HBase
> Issue Type: Improvement
> Components: hbase
>Affects Versions: 3.0.0
>Reporter: BELUGA BEHR
>Assignee: BELUGA BEHR
>Priority: Trivial
> Fix For: 3.0.0, 1.3.2, 1.4.1, 1.5.0, 1.2.7, 2.0.0-beta-2
>
> Attachments: HBASE-19684.1.patch, HBASE-19684.2.patch
>
>
> {code:titile=BlockCacheKey.java}
> @Override
> public String toString() {
> return String.format("%s_%d", hfileName, offset);
> }
> {code}
> I found through bench-marking that the following code is 10x faster.
> {code:titi\le=BlockCacheKey.java}
> @Override
> public String toString() {
> return hfileName.concat("_").concat(Long.toString(offset));
> }
> {code}
> Normally it wouldn't matter for a _toString()_ method, but this is comes into
> play because {{MemcachedBlockCache}} uses it.
> {code:title=MemcachedBlockCache.java}
> @Override
> public void cacheBlock(BlockCacheKey cacheKey, Cacheable buf) {
> if (buf instanceof HFileBlock) {
> client.add(cacheKey.toString(), MAX_SIZE, (HFileBlock) buf, tc);
> } else {
> if (LOG.isDebugEnabled()) {
> LOG.debug("MemcachedBlockCache can not cache Cacheable's of type "
> + buf.getClass().toString());
> }
> }
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (HBASE-19684) BlockCacheKey toString Performance
[
https://issues.apache.org/jira/browse/HBASE-19684?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16316038#comment-16316038
]
Hudson commented on HBASE-19684:
SUCCESS: Integrated in Jenkins build HBase-1.2-IT #1055 (See
[https://builds.apache.org/job/HBase-1.2-IT/1055/])
HBASE-19684 BlockCacheKey toString Performance (zhangduo: rev
8509be660cbefecf0be9c03108c84327fcb07894)
* (edit)
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/BlockCacheKey.java
> BlockCacheKey toString Performance
> --
>
> Key: HBASE-19684
> URL: https://issues.apache.org/jira/browse/HBASE-19684
> Project: HBase
> Issue Type: Improvement
> Components: hbase
>Affects Versions: 3.0.0
>Reporter: BELUGA BEHR
>Assignee: BELUGA BEHR
>Priority: Trivial
> Fix For: 3.0.0, 1.3.2, 1.4.1, 1.5.0, 1.2.7, 2.0.0-beta-2
>
> Attachments: HBASE-19684.1.patch, HBASE-19684.2.patch
>
>
> {code:titile=BlockCacheKey.java}
> @Override
> public String toString() {
> return String.format("%s_%d", hfileName, offset);
> }
> {code}
> I found through bench-marking that the following code is 10x faster.
> {code:titi\le=BlockCacheKey.java}
> @Override
> public String toString() {
> return hfileName.concat("_").concat(Long.toString(offset));
> }
> {code}
> Normally it wouldn't matter for a _toString()_ method, but this is comes into
> play because {{MemcachedBlockCache}} uses it.
> {code:title=MemcachedBlockCache.java}
> @Override
> public void cacheBlock(BlockCacheKey cacheKey, Cacheable buf) {
> if (buf instanceof HFileBlock) {
> client.add(cacheKey.toString(), MAX_SIZE, (HFileBlock) buf, tc);
> } else {
> if (LOG.isDebugEnabled()) {
> LOG.debug("MemcachedBlockCache can not cache Cacheable's of type "
> + buf.getClass().toString());
> }
> }
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (HBASE-19684) BlockCacheKey toString Performance
[
https://issues.apache.org/jira/browse/HBASE-19684?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16315923#comment-16315923
]
Duo Zhang commented on HBASE-19684:
---
Can not push to asf git...
{noformat}
fatal: unable to access 'https://git-wip-us.apache.org/repos/asf/hbase.git/':
The requested URL returned error: 500
{noformat}
Will try again later.
> BlockCacheKey toString Performance
> --
>
> Key: HBASE-19684
> URL: https://issues.apache.org/jira/browse/HBASE-19684
> Project: HBase
> Issue Type: Improvement
> Components: hbase
>Affects Versions: 3.0.0
>Reporter: BELUGA BEHR
>Assignee: BELUGA BEHR
>Priority: Trivial
> Attachments: HBASE-19684.1.patch, HBASE-19684.2.patch
>
>
> {code:titile=BlockCacheKey.java}
> @Override
> public String toString() {
> return String.format("%s_%d", hfileName, offset);
> }
> {code}
> I found through bench-marking that the following code is 10x faster.
> {code:titi\le=BlockCacheKey.java}
> @Override
> public String toString() {
> return hfileName.concat("_").concat(Long.toString(offset));
> }
> {code}
> Normally it wouldn't matter for a _toString()_ method, but this is comes into
> play because {{MemcachedBlockCache}} uses it.
> {code:title=MemcachedBlockCache.java}
> @Override
> public void cacheBlock(BlockCacheKey cacheKey, Cacheable buf) {
> if (buf instanceof HFileBlock) {
> client.add(cacheKey.toString(), MAX_SIZE, (HFileBlock) buf, tc);
> } else {
> if (LOG.isDebugEnabled()) {
> LOG.debug("MemcachedBlockCache can not cache Cacheable's of type "
> + buf.getClass().toString());
> }
> }
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (HBASE-19684) BlockCacheKey toString Performance
[
https://issues.apache.org/jira/browse/HBASE-19684?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16315861#comment-16315861
]
Duo Zhang commented on HBASE-19684:
---
+1 on using the simple '+' operator. Will commit later if no objections.
> BlockCacheKey toString Performance
> --
>
> Key: HBASE-19684
> URL: https://issues.apache.org/jira/browse/HBASE-19684
> Project: HBase
> Issue Type: Improvement
> Components: hbase
>Affects Versions: 3.0.0
>Reporter: BELUGA BEHR
>Assignee: BELUGA BEHR
>Priority: Trivial
> Attachments: HBASE-19684.1.patch, HBASE-19684.2.patch
>
>
> {code:titile=BlockCacheKey.java}
> @Override
> public String toString() {
> return String.format("%s_%d", hfileName, offset);
> }
> {code}
> I found through bench-marking that the following code is 10x faster.
> {code:titi\le=BlockCacheKey.java}
> @Override
> public String toString() {
> return hfileName.concat("_").concat(Long.toString(offset));
> }
> {code}
> Normally it wouldn't matter for a _toString()_ method, but this is comes into
> play because {{MemcachedBlockCache}} uses it.
> {code:title=MemcachedBlockCache.java}
> @Override
> public void cacheBlock(BlockCacheKey cacheKey, Cacheable buf) {
> if (buf instanceof HFileBlock) {
> client.add(cacheKey.toString(), MAX_SIZE, (HFileBlock) buf, tc);
> } else {
> if (LOG.isDebugEnabled()) {
> LOG.debug("MemcachedBlockCache can not cache Cacheable's of type "
> + buf.getClass().toString());
> }
> }
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (HBASE-19684) BlockCacheKey toString Performance
[
https://issues.apache.org/jira/browse/HBASE-19684?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16313313#comment-16313313
]
BELUGA BEHR commented on HBASE-19684:
-
[~appy] and [~mdrob] I think we have put the bow on this patch? Can we commit
the latest patch with the easy to read, implicit, StringBuilder?
Thanks.
> BlockCacheKey toString Performance
> --
>
> Key: HBASE-19684
> URL: https://issues.apache.org/jira/browse/HBASE-19684
> Project: HBase
> Issue Type: Improvement
> Components: hbase
>Affects Versions: 3.0.0
>Reporter: BELUGA BEHR
>Assignee: BELUGA BEHR
>Priority: Trivial
> Attachments: HBASE-19684.1.patch, HBASE-19684.2.patch
>
>
> {code:titile=BlockCacheKey.java}
> @Override
> public String toString() {
> return String.format("%s_%d", hfileName, offset);
> }
> {code}
> I found through bench-marking that the following code is 10x faster.
> {code:titi\le=BlockCacheKey.java}
> @Override
> public String toString() {
> return hfileName.concat("_").concat(Long.toString(offset));
> }
> {code}
> Normally it wouldn't matter for a _toString()_ method, but this is comes into
> play because {{MemcachedBlockCache}} uses it.
> {code:title=MemcachedBlockCache.java}
> @Override
> public void cacheBlock(BlockCacheKey cacheKey, Cacheable buf) {
> if (buf instanceof HFileBlock) {
> client.add(cacheKey.toString(), MAX_SIZE, (HFileBlock) buf, tc);
> } else {
> if (LOG.isDebugEnabled()) {
> LOG.debug("MemcachedBlockCache can not cache Cacheable's of type "
> + buf.getClass().toString());
> }
> }
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (HBASE-19684) BlockCacheKey toString Performance
[
https://issues.apache.org/jira/browse/HBASE-19684?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16310410#comment-16310410
]
Mike Drob commented on HBASE-19684:
---
Yea, looking at the bytecode we can confirm that concat operator uses a
StringBuilder under the hood. It compiles to essentially the same code, except
without an initial buffer size like you noted.
> BlockCacheKey toString Performance
> --
>
> Key: HBASE-19684
> URL: https://issues.apache.org/jira/browse/HBASE-19684
> Project: HBase
> Issue Type: Improvement
> Components: hbase
>Affects Versions: 3.0.0
>Reporter: BELUGA BEHR
>Assignee: BELUGA BEHR
>Priority: Trivial
> Attachments: HBASE-19684.1.patch, HBASE-19684.2.patch
>
>
> {code:titile=BlockCacheKey.java}
> @Override
> public String toString() {
> return String.format("%s_%d", hfileName, offset);
> }
> {code}
> I found through bench-marking that the following code is 10x faster.
> {code:titi\le=BlockCacheKey.java}
> @Override
> public String toString() {
> return hfileName.concat("_").concat(Long.toString(offset));
> }
> {code}
> Normally it wouldn't matter for a _toString()_ method, but this is comes into
> play because {{MemcachedBlockCache}} uses it.
> {code:title=MemcachedBlockCache.java}
> @Override
> public void cacheBlock(BlockCacheKey cacheKey, Cacheable buf) {
> if (buf instanceof HFileBlock) {
> client.add(cacheKey.toString(), MAX_SIZE, (HFileBlock) buf, tc);
> } else {
> if (LOG.isDebugEnabled()) {
> LOG.debug("MemcachedBlockCache can not cache Cacheable's of type "
> + buf.getClass().toString());
> }
> }
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (HBASE-19684) BlockCacheKey toString Performance
[
https://issues.apache.org/jira/browse/HBASE-19684?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16310405#comment-16310405
]
BELUGA BEHR commented on HBASE-19684:
-
I'd probably just keep for the current patch as it is most readable. And we're
not talking about random data here. We're talking about positive offsets, so
it would have to be one really large offset to take up 15 characters. You
won't in practice see much of this buffer-copying happen.
> BlockCacheKey toString Performance
> --
>
> Key: HBASE-19684
> URL: https://issues.apache.org/jira/browse/HBASE-19684
> Project: HBase
> Issue Type: Improvement
> Components: hbase
>Affects Versions: 3.0.0
>Reporter: BELUGA BEHR
>Assignee: BELUGA BEHR
>Priority: Trivial
> Attachments: HBASE-19684.1.patch, HBASE-19684.2.patch
>
>
> {code:titile=BlockCacheKey.java}
> @Override
> public String toString() {
> return String.format("%s_%d", hfileName, offset);
> }
> {code}
> I found through bench-marking that the following code is 10x faster.
> {code:titi\le=BlockCacheKey.java}
> @Override
> public String toString() {
> return hfileName.concat("_").concat(Long.toString(offset));
> }
> {code}
> Normally it wouldn't matter for a _toString()_ method, but this is comes into
> play because {{MemcachedBlockCache}} uses it.
> {code:title=MemcachedBlockCache.java}
> @Override
> public void cacheBlock(BlockCacheKey cacheKey, Cacheable buf) {
> if (buf instanceof HFileBlock) {
> client.add(cacheKey.toString(), MAX_SIZE, (HFileBlock) buf, tc);
> } else {
> if (LOG.isDebugEnabled()) {
> LOG.debug("MemcachedBlockCache can not cache Cacheable's of type "
> + buf.getClass().toString());
> }
> }
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (HBASE-19684) BlockCacheKey toString Performance
[
https://issues.apache.org/jira/browse/HBASE-19684?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16310399#comment-16310399
]
BELUGA BEHR commented on HBASE-19684:
-
Just let me know which version you would like... or go ahead and make this one
liner yourself. :)
> BlockCacheKey toString Performance
> --
>
> Key: HBASE-19684
> URL: https://issues.apache.org/jira/browse/HBASE-19684
> Project: HBase
> Issue Type: Improvement
> Components: hbase
>Affects Versions: 3.0.0
>Reporter: BELUGA BEHR
>Assignee: BELUGA BEHR
>Priority: Trivial
> Attachments: HBASE-19684.1.patch, HBASE-19684.2.patch
>
>
> {code:titile=BlockCacheKey.java}
> @Override
> public String toString() {
> return String.format("%s_%d", hfileName, offset);
> }
> {code}
> I found through bench-marking that the following code is 10x faster.
> {code:titi\le=BlockCacheKey.java}
> @Override
> public String toString() {
> return hfileName.concat("_").concat(Long.toString(offset));
> }
> {code}
> Normally it wouldn't matter for a _toString()_ method, but this is comes into
> play because {{MemcachedBlockCache}} uses it.
> {code:title=MemcachedBlockCache.java}
> @Override
> public void cacheBlock(BlockCacheKey cacheKey, Cacheable buf) {
> if (buf instanceof HFileBlock) {
> client.add(cacheKey.toString(), MAX_SIZE, (HFileBlock) buf, tc);
> } else {
> if (LOG.isDebugEnabled()) {
> LOG.debug("MemcachedBlockCache can not cache Cacheable's of type "
> + buf.getClass().toString());
> }
> }
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (HBASE-19684) BlockCacheKey toString Performance
[
https://issues.apache.org/jira/browse/HBASE-19684?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16310390#comment-16310390
]
BELUGA BEHR commented on HBASE-19684:
-
[~mdrob] If you want to play that game... HA! :)
{code}
@State(Scope.Thread)
public static class Data
{
Random r = new Random();
String a;
Long b;
@Setup(Level.Trial)
public void setup()
{
a = Long.toHexString(r.nextLong());
b = Long.MIN_VALUE + 1L;
}
}
@Benchmark
public String testStringBuilder(Data data)
{
return (new StringBuilder(data.a.length() +
21).append(data.a).append('_').append(data.b).toString());
}
@Benchmark
public String testConcatOperator(Data data)
{
return data.a + '_' + data.b;
}
{code}
{code}
# Run complete. Total time: 00:13:28
Benchmark Mode Cnt ScoreError
Units
BenchmarkStrings.testConcatOperator thrpt 200 7852314.219 ± 154534.037
ops/s
BenchmarkStrings.testStringBuilder thrpt 200 10360674.186 ± 22605.073
ops/s
{code}
Here we can see that the explicit StringBuilder is faster, as I would expect.
This is because I am testing, not with random Long values, but with a constant
Long value, which when turned into a String, has a long string length (20
characters). So in TestStringBuilder, I account for these 20 characters, plus
1 character for the underscore. Therefore, everything fits nicely into one,
pre-allocated, buffer.
{quote}
public StringBuilder(String str)
Constructs a string builder initialized to the contents of the specified
string. The initial capacity of the string builder is 16 plus the length of the
string argument.
{quote}
So, in the testConcatOperator method, the 16-byte buffer must be expanded
(which includes the normal operation of allocated a new buffer and copying the
old contents into it) to fit the 21 characters that are required to represent
this Long value + one underscore.
> BlockCacheKey toString Performance
> --
>
> Key: HBASE-19684
> URL: https://issues.apache.org/jira/browse/HBASE-19684
> Project: HBase
> Issue Type: Improvement
> Components: hbase
>Affects Versions: 3.0.0
>Reporter: BELUGA BEHR
>Assignee: BELUGA BEHR
>Priority: Trivial
> Attachments: HBASE-19684.1.patch, HBASE-19684.2.patch
>
>
> {code:titile=BlockCacheKey.java}
> @Override
> public String toString() {
> return String.format("%s_%d", hfileName, offset);
> }
> {code}
> I found through bench-marking that the following code is 10x faster.
> {code:titi\le=BlockCacheKey.java}
> @Override
> public String toString() {
> return hfileName.concat("_").concat(Long.toString(offset));
> }
> {code}
> Normally it wouldn't matter for a _toString()_ method, but this is comes into
> play because {{MemcachedBlockCache}} uses it.
> {code:title=MemcachedBlockCache.java}
> @Override
> public void cacheBlock(BlockCacheKey cacheKey, Cacheable buf) {
> if (buf instanceof HFileBlock) {
> client.add(cacheKey.toString(), MAX_SIZE, (HFileBlock) buf, tc);
> } else {
> if (LOG.isDebugEnabled()) {
> LOG.debug("MemcachedBlockCache can not cache Cacheable's of type "
> + buf.getClass().toString());
> }
> }
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (HBASE-19684) BlockCacheKey toString Performance
[
https://issues.apache.org/jira/browse/HBASE-19684?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16309134#comment-16309134
]
Hadoop QA commented on HBASE-19684:
---
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m
12s{color} | {color:blue} Docker mode activated. {color} |
|| || || || {color:brown} Prechecks {color} ||
| {color:blue}0{color} | {color:blue} findbugs {color} | {color:blue} 0m
0s{color} | {color:blue} Findbugs executables are not available. {color} |
| {color:green}+1{color} | {color:green} hbaseanti {color} | {color:green} 0m
0s{color} | {color:green} Patch does not have any anti-patterns. {color} |
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m
0s{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:red}-1{color} | {color:red} test4tests {color} | {color:red} 0m
0s{color} | {color:red} The patch doesn't appear to include any new or modified
tests. Please justify why no new tests are needed for this patch. Also please
list what manual steps were performed to verify this patch. {color} |
|| || || || {color:brown} master Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 4m
49s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m
44s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 1m
8s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} shadedjars {color} | {color:green} 5m
54s{color} | {color:green} branch has no errors when building our shaded
downstream artifacts. {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
33s{color} | {color:green} master passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 4m
58s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m
45s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 0m
45s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 1m
7s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m
0s{color} | {color:green} The patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} shadedjars {color} | {color:green} 4m
44s{color} | {color:green} patch has no errors when building our shaded
downstream artifacts. {color} |
| {color:green}+1{color} | {color:green} hadoopcheck {color} | {color:green}
20m 23s{color} | {color:green} Patch does not cause any errors with Hadoop
2.6.5 2.7.4 or 3.0.0. {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
34s{color} | {color:green} the patch passed {color} |
|| || || || {color:brown} Other Tests {color} ||
| {color:green}+1{color} | {color:green} unit {color} | {color:green}113m
59s{color} | {color:green} hbase-server in the patch passed. {color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m
19s{color} | {color:green} The patch does not generate ASF License warnings.
{color} |
| {color:black}{color} | {color:black} {color} | {color:black}154m 28s{color} |
{color:black} {color} |
\\
\\
|| Subsystem || Report/Notes ||
| Docker | Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hbase:eee3b01 |
| JIRA Issue | HBASE-19684 |
| JIRA Patch URL |
https://issues.apache.org/jira/secure/attachment/12904309/HBASE-19684.2.patch |
| Optional Tests | asflicense javac javadoc unit findbugs shadedjars
hadoopcheck hbaseanti checkstyle compile |
| uname | Linux e5c56e8fc7be 3.13.0-129-generic #178-Ubuntu SMP Fri Aug 11
12:48:20 UTC 2017 x86_64 GNU/Linux |
| Build tool | maven |
| Personality |
/home/jenkins/jenkins-slave/workspace/PreCommit-HBASE-Build@2/component/dev-support/hbase-personality.sh
|
| git revision | master / a47afc84cd |
| maven | version: Apache Maven 3.5.2
(138edd61fd100ec658bfa2d307c43b76940a5d7d; 2017-10-18T07:58:13Z) |
| Default Java | 1.8.0_151 |
| Test Results |
https://builds.apache.org/job/PreCommit-HBASE-Build/10853/testReport/ |
| modules | C: hbase-server U: hbase-server |
| Console output |
https://builds.apache.org/job/PreCommit-HBASE-Build/10853/console |
| Powered by | Apache Yetus 0.6.0 http://yetus.apache.org |
This message was automatically generated.
> BlockCacheKey toString Performance
> --
>
> Key:
[jira] [Commented] (HBASE-19684) BlockCacheKey toString Performance
[
https://issues.apache.org/jira/browse/HBASE-19684?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16309055#comment-16309055
]
Mike Drob commented on HBASE-19684:
---
FWIW, looks like StringBuilder may be the most performant approach.
{noformat}
BenchmarkMode CntScoreError Units
MyBenchmark.testConcatMethodthrpt 200 8531013.864 ± 125796.993 ops/s
MyBenchmark.testConcatOperator thrpt 200 7168184.764 ± 209635.694 ops/s
MyBenchmark.testStringBuilder thrpt 200 9801662.430 ± 185811.763 ops/s
MyBenchmark.testStringFormatthrpt 200 1021658.557 ± 13366.938 ops/s
{noformat}
{code}
public class MyBenchmark {
@State(Scope.Thread)
public static class Data {
Random r = new Random();
String a;
Long b;
@Setup(Level.Trial)
public void setup() {
a = Long.toHexString(r.nextLong());
b = r.nextLong();
}
}
@Benchmark
public String testStringFormat(Data data) {
return (String.format("%s_%d", data.a, data.b));
}
@Benchmark
public String testConcatMethod(Data data) {
return (data.a.concat("_").concat(Long.toString(data.b)));
}
@Benchmark
public String testStringBuilder(Data data) {
return (new StringBuilder(data.a.length() +
21).append(data.a).append('_').append(data.b).toString());
}
@Benchmark
public String testConcatOperator(Data data) {
return data.a + '_' + data.b;
}
}
{code}
> BlockCacheKey toString Performance
> --
>
> Key: HBASE-19684
> URL: https://issues.apache.org/jira/browse/HBASE-19684
> Project: HBase
> Issue Type: Improvement
> Components: hbase
>Affects Versions: 3.0.0
>Reporter: BELUGA BEHR
>Assignee: BELUGA BEHR
>Priority: Trivial
> Attachments: HBASE-19684.1.patch, HBASE-19684.2.patch
>
>
> {code:titile=BlockCacheKey.java}
> @Override
> public String toString() {
> return String.format("%s_%d", hfileName, offset);
> }
> {code}
> I found through bench-marking that the following code is 10x faster.
> {code:titi\le=BlockCacheKey.java}
> @Override
> public String toString() {
> return hfileName.concat("_").concat(Long.toString(offset));
> }
> {code}
> Normally it wouldn't matter for a _toString()_ method, but this is comes into
> play because {{MemcachedBlockCache}} uses it.
> {code:title=MemcachedBlockCache.java}
> @Override
> public void cacheBlock(BlockCacheKey cacheKey, Cacheable buf) {
> if (buf instanceof HFileBlock) {
> client.add(cacheKey.toString(), MAX_SIZE, (HFileBlock) buf, tc);
> } else {
> if (LOG.isDebugEnabled()) {
> LOG.debug("MemcachedBlockCache can not cache Cacheable's of type "
> + buf.getClass().toString());
> }
> }
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (HBASE-19684) BlockCacheKey toString Performance
[
https://issues.apache.org/jira/browse/HBASE-19684?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16309031#comment-16309031
]
Mike Drob commented on HBASE-19684:
---
My -1 was for v1 of the patch based on the dubious benchmarking and use of
concat method. Happy to see the change to {{+}} operator.
> BlockCacheKey toString Performance
> --
>
> Key: HBASE-19684
> URL: https://issues.apache.org/jira/browse/HBASE-19684
> Project: HBase
> Issue Type: Improvement
> Components: hbase
>Affects Versions: 3.0.0
>Reporter: BELUGA BEHR
>Assignee: BELUGA BEHR
>Priority: Trivial
> Attachments: HBASE-19684.1.patch, HBASE-19684.2.patch
>
>
> {code:titile=BlockCacheKey.java}
> @Override
> public String toString() {
> return String.format("%s_%d", hfileName, offset);
> }
> {code}
> I found through bench-marking that the following code is 10x faster.
> {code:titi\le=BlockCacheKey.java}
> @Override
> public String toString() {
> return hfileName.concat("_").concat(Long.toString(offset));
> }
> {code}
> Normally it wouldn't matter for a _toString()_ method, but this is comes into
> play because {{MemcachedBlockCache}} uses it.
> {code:title=MemcachedBlockCache.java}
> @Override
> public void cacheBlock(BlockCacheKey cacheKey, Cacheable buf) {
> if (buf instanceof HFileBlock) {
> client.add(cacheKey.toString(), MAX_SIZE, (HFileBlock) buf, tc);
> } else {
> if (LOG.isDebugEnabled()) {
> LOG.debug("MemcachedBlockCache can not cache Cacheable's of type "
> + buf.getClass().toString());
> }
> }
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (HBASE-19684) BlockCacheKey toString Performance
[
https://issues.apache.org/jira/browse/HBASE-19684?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16309026#comment-16309026
]
BELUGA BEHR commented on HBASE-19684:
-
[~mdrob] The current implementation is:
{code}
return String.format("%s_%d", hfileName, offset);
{code}
{code}
return this.hfileName + '_' + this.offset;
{code}
It's definitely faster than {{String#format}}. The bulk of the discussion here
was _StringBuilder_ v.s. _concat_. Please reconsider. :)
> BlockCacheKey toString Performance
> --
>
> Key: HBASE-19684
> URL: https://issues.apache.org/jira/browse/HBASE-19684
> Project: HBase
> Issue Type: Improvement
> Components: hbase
>Affects Versions: 3.0.0
>Reporter: BELUGA BEHR
>Assignee: BELUGA BEHR
>Priority: Trivial
> Attachments: HBASE-19684.1.patch, HBASE-19684.2.patch
>
>
> {code:titile=BlockCacheKey.java}
> @Override
> public String toString() {
> return String.format("%s_%d", hfileName, offset);
> }
> {code}
> I found through bench-marking that the following code is 10x faster.
> {code:titi\le=BlockCacheKey.java}
> @Override
> public String toString() {
> return hfileName.concat("_").concat(Long.toString(offset));
> }
> {code}
> Normally it wouldn't matter for a _toString()_ method, but this is comes into
> play because {{MemcachedBlockCache}} uses it.
> {code:title=MemcachedBlockCache.java}
> @Override
> public void cacheBlock(BlockCacheKey cacheKey, Cacheable buf) {
> if (buf instanceof HFileBlock) {
> client.add(cacheKey.toString(), MAX_SIZE, (HFileBlock) buf, tc);
> } else {
> if (LOG.isDebugEnabled()) {
> LOG.debug("MemcachedBlockCache can not cache Cacheable's of type "
> + buf.getClass().toString());
> }
> }
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (HBASE-19684) BlockCacheKey toString Performance
[
https://issues.apache.org/jira/browse/HBASE-19684?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16309025#comment-16309025
]
Mike Drob commented on HBASE-19684:
---
-1 to this patch as is.
We need to either improve performance or improve readability here, and I am not
convinced that the patch does either. Like Duo asked, please write a JMH
benchmark to prove the former...
> BlockCacheKey toString Performance
> --
>
> Key: HBASE-19684
> URL: https://issues.apache.org/jira/browse/HBASE-19684
> Project: HBase
> Issue Type: Improvement
> Components: hbase
>Affects Versions: 3.0.0
>Reporter: BELUGA BEHR
>Assignee: BELUGA BEHR
>Priority: Trivial
> Attachments: HBASE-19684.1.patch, HBASE-19684.2.patch
>
>
> {code:titile=BlockCacheKey.java}
> @Override
> public String toString() {
> return String.format("%s_%d", hfileName, offset);
> }
> {code}
> I found through bench-marking that the following code is 10x faster.
> {code:titi\le=BlockCacheKey.java}
> @Override
> public String toString() {
> return hfileName.concat("_").concat(Long.toString(offset));
> }
> {code}
> Normally it wouldn't matter for a _toString()_ method, but this is comes into
> play because {{MemcachedBlockCache}} uses it.
> {code:title=MemcachedBlockCache.java}
> @Override
> public void cacheBlock(BlockCacheKey cacheKey, Cacheable buf) {
> if (buf instanceof HFileBlock) {
> client.add(cacheKey.toString(), MAX_SIZE, (HFileBlock) buf, tc);
> } else {
> if (LOG.isDebugEnabled()) {
> LOG.debug("MemcachedBlockCache can not cache Cacheable's of type "
> + buf.getClass().toString());
> }
> }
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (HBASE-19684) BlockCacheKey toString Performance
[
https://issues.apache.org/jira/browse/HBASE-19684?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16309014#comment-16309014
]
BELUGA BEHR commented on HBASE-19684:
-
[~appy] Only special kind of people like you and I would consider this _fun_ :)
I just attached the StringBuilder version. Readability for the win.
> BlockCacheKey toString Performance
> --
>
> Key: HBASE-19684
> URL: https://issues.apache.org/jira/browse/HBASE-19684
> Project: HBase
> Issue Type: Improvement
> Components: hbase
>Affects Versions: 3.0.0
>Reporter: BELUGA BEHR
>Assignee: BELUGA BEHR
>Priority: Trivial
> Attachments: HBASE-19684.1.patch, HBASE-19684.2.patch
>
>
> {code:titile=BlockCacheKey.java}
> @Override
> public String toString() {
> return String.format("%s_%d", hfileName, offset);
> }
> {code}
> I found through bench-marking that the following code is 10x faster.
> {code:titi\le=BlockCacheKey.java}
> @Override
> public String toString() {
> return hfileName.concat("_").concat(Long.toString(offset));
> }
> {code}
> Normally it wouldn't matter for a _toString()_ method, but this is comes into
> play because {{MemcachedBlockCache}} uses it.
> {code:title=MemcachedBlockCache.java}
> @Override
> public void cacheBlock(BlockCacheKey cacheKey, Cacheable buf) {
> if (buf instanceof HFileBlock) {
> client.add(cacheKey.toString(), MAX_SIZE, (HFileBlock) buf, tc);
> } else {
> if (LOG.isDebugEnabled()) {
> LOG.debug("MemcachedBlockCache can not cache Cacheable's of type "
> + buf.getClass().toString());
> }
> }
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (HBASE-19684) BlockCacheKey toString Performance
[
https://issues.apache.org/jira/browse/HBASE-19684?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16309013#comment-16309013
]
Duo Zhang commented on HBASE-19684:
---
https://stackoverflow.com/a/46485284
This answer shows that '+' is faster than concat in java8 with jmh...
> BlockCacheKey toString Performance
> --
>
> Key: HBASE-19684
> URL: https://issues.apache.org/jira/browse/HBASE-19684
> Project: HBase
> Issue Type: Improvement
> Components: hbase
>Affects Versions: 3.0.0
>Reporter: BELUGA BEHR
>Assignee: BELUGA BEHR
>Priority: Trivial
> Attachments: HBASE-19684.1.patch, HBASE-19684.2.patch
>
>
> {code:titile=BlockCacheKey.java}
> @Override
> public String toString() {
> return String.format("%s_%d", hfileName, offset);
> }
> {code}
> I found through bench-marking that the following code is 10x faster.
> {code:titi\le=BlockCacheKey.java}
> @Override
> public String toString() {
> return hfileName.concat("_").concat(Long.toString(offset));
> }
> {code}
> Normally it wouldn't matter for a _toString()_ method, but this is comes into
> play because {{MemcachedBlockCache}} uses it.
> {code:title=MemcachedBlockCache.java}
> @Override
> public void cacheBlock(BlockCacheKey cacheKey, Cacheable buf) {
> if (buf instanceof HFileBlock) {
> client.add(cacheKey.toString(), MAX_SIZE, (HFileBlock) buf, tc);
> } else {
> if (LOG.isDebugEnabled()) {
> LOG.debug("MemcachedBlockCache can not cache Cacheable's of type "
> + buf.getClass().toString());
> }
> }
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (HBASE-19684) BlockCacheKey toString Performance
[
https://issues.apache.org/jira/browse/HBASE-19684?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16309012#comment-16309012
]
Appy commented on HBASE-19684:
--
I would also admit that at this point, the sole purpose of doing the exercise
should be *fun*. Otherwise, one will just easily google for the known industry
standards and use known results. :)
> BlockCacheKey toString Performance
> --
>
> Key: HBASE-19684
> URL: https://issues.apache.org/jira/browse/HBASE-19684
> Project: HBase
> Issue Type: Improvement
> Components: hbase
>Affects Versions: 3.0.0
>Reporter: BELUGA BEHR
>Assignee: BELUGA BEHR
>Priority: Trivial
> Attachments: HBASE-19684.1.patch
>
>
> {code:titile=BlockCacheKey.java}
> @Override
> public String toString() {
> return String.format("%s_%d", hfileName, offset);
> }
> {code}
> I found through bench-marking that the following code is 10x faster.
> {code:titi\le=BlockCacheKey.java}
> @Override
> public String toString() {
> return hfileName.concat("_").concat(Long.toString(offset));
> }
> {code}
> Normally it wouldn't matter for a _toString()_ method, but this is comes into
> play because {{MemcachedBlockCache}} uses it.
> {code:title=MemcachedBlockCache.java}
> @Override
> public void cacheBlock(BlockCacheKey cacheKey, Cacheable buf) {
> if (buf instanceof HFileBlock) {
> client.add(cacheKey.toString(), MAX_SIZE, (HFileBlock) buf, tc);
> } else {
> if (LOG.isDebugEnabled()) {
> LOG.debug("MemcachedBlockCache can not cache Cacheable's of type "
> + buf.getClass().toString());
> }
> }
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (HBASE-19684) BlockCacheKey toString Performance
[
https://issues.apache.org/jira/browse/HBASE-19684?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16309010#comment-16309010
]
BELUGA BEHR commented on HBASE-19684:
-
[~appy] "c" is an ArrayList that is pre-allocated with a 100K array (not part
of the timing) just to avoid the optimization you pointed out.
> BlockCacheKey toString Performance
> --
>
> Key: HBASE-19684
> URL: https://issues.apache.org/jira/browse/HBASE-19684
> Project: HBase
> Issue Type: Improvement
> Components: hbase
>Affects Versions: 3.0.0
>Reporter: BELUGA BEHR
>Assignee: BELUGA BEHR
>Priority: Trivial
> Attachments: HBASE-19684.1.patch
>
>
> {code:titile=BlockCacheKey.java}
> @Override
> public String toString() {
> return String.format("%s_%d", hfileName, offset);
> }
> {code}
> I found through bench-marking that the following code is 10x faster.
> {code:titi\le=BlockCacheKey.java}
> @Override
> public String toString() {
> return hfileName.concat("_").concat(Long.toString(offset));
> }
> {code}
> Normally it wouldn't matter for a _toString()_ method, but this is comes into
> play because {{MemcachedBlockCache}} uses it.
> {code:title=MemcachedBlockCache.java}
> @Override
> public void cacheBlock(BlockCacheKey cacheKey, Cacheable buf) {
> if (buf instanceof HFileBlock) {
> client.add(cacheKey.toString(), MAX_SIZE, (HFileBlock) buf, tc);
> } else {
> if (LOG.isDebugEnabled()) {
> LOG.debug("MemcachedBlockCache can not cache Cacheable's of type "
> + buf.getClass().toString());
> }
> }
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (HBASE-19684) BlockCacheKey toString Performance
[
https://issues.apache.org/jira/browse/HBASE-19684?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16309004#comment-16309004
]
Appy commented on HBASE-19684:
--
Also, for correctness purpose, maybe you should use a new String c everytime?
Otherwise, is it possible that reallocation of c, once it grows very big (100k
iterations afterall!) may shadow real thing you are trying to measure.
What is c anyways? Why not assign it to random variable and use it for
something after the loop (otherwise java might optimize away the unused
variable, does it?).
> BlockCacheKey toString Performance
> --
>
> Key: HBASE-19684
> URL: https://issues.apache.org/jira/browse/HBASE-19684
> Project: HBase
> Issue Type: Improvement
> Components: hbase
>Affects Versions: 3.0.0
>Reporter: BELUGA BEHR
>Assignee: BELUGA BEHR
>Priority: Trivial
> Attachments: HBASE-19684.1.patch
>
>
> {code:titile=BlockCacheKey.java}
> @Override
> public String toString() {
> return String.format("%s_%d", hfileName, offset);
> }
> {code}
> I found through bench-marking that the following code is 10x faster.
> {code:titi\le=BlockCacheKey.java}
> @Override
> public String toString() {
> return hfileName.concat("_").concat(Long.toString(offset));
> }
> {code}
> Normally it wouldn't matter for a _toString()_ method, but this is comes into
> play because {{MemcachedBlockCache}} uses it.
> {code:title=MemcachedBlockCache.java}
> @Override
> public void cacheBlock(BlockCacheKey cacheKey, Cacheable buf) {
> if (buf instanceof HFileBlock) {
> client.add(cacheKey.toString(), MAX_SIZE, (HFileBlock) buf, tc);
> } else {
> if (LOG.isDebugEnabled()) {
> LOG.debug("MemcachedBlockCache can not cache Cacheable's of type "
> + buf.getClass().toString());
> }
> }
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (HBASE-19684) BlockCacheKey toString Performance
[
https://issues.apache.org/jira/browse/HBASE-19684?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16309000#comment-16309000
]
BELUGA BEHR commented on HBASE-19684:
-
[~Apache9] There isn't really, but this patch is already here :) I can update
or you can too since it's so trivial.
> BlockCacheKey toString Performance
> --
>
> Key: HBASE-19684
> URL: https://issues.apache.org/jira/browse/HBASE-19684
> Project: HBase
> Issue Type: Improvement
> Components: hbase
>Affects Versions: 3.0.0
>Reporter: BELUGA BEHR
>Assignee: BELUGA BEHR
>Priority: Trivial
> Attachments: HBASE-19684.1.patch
>
>
> {code:titile=BlockCacheKey.java}
> @Override
> public String toString() {
> return String.format("%s_%d", hfileName, offset);
> }
> {code}
> I found through bench-marking that the following code is 10x faster.
> {code:titi\le=BlockCacheKey.java}
> @Override
> public String toString() {
> return hfileName.concat("_").concat(Long.toString(offset));
> }
> {code}
> Normally it wouldn't matter for a _toString()_ method, but this is comes into
> play because {{MemcachedBlockCache}} uses it.
> {code:title=MemcachedBlockCache.java}
> @Override
> public void cacheBlock(BlockCacheKey cacheKey, Cacheable buf) {
> if (buf instanceof HFileBlock) {
> client.add(cacheKey.toString(), MAX_SIZE, (HFileBlock) buf, tc);
> } else {
> if (LOG.isDebugEnabled()) {
> LOG.debug("MemcachedBlockCache can not cache Cacheable's of type "
> + buf.getClass().toString());
> }
> }
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (HBASE-19684) BlockCacheKey toString Performance
[
https://issues.apache.org/jira/browse/HBASE-19684?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16308973#comment-16308973
]
Duo Zhang commented on HBASE-19684:
---
Could you please run a micro benchmark using jmh? A slow String.format is
expected, but I do not think there will be much difference between simple '+'
and concat...
Thanks.
> BlockCacheKey toString Performance
> --
>
> Key: HBASE-19684
> URL: https://issues.apache.org/jira/browse/HBASE-19684
> Project: HBase
> Issue Type: Improvement
> Components: hbase
>Affects Versions: 3.0.0
>Reporter: BELUGA BEHR
>Assignee: BELUGA BEHR
>Priority: Trivial
> Attachments: HBASE-19684.1.patch
>
>
> {code:titile=BlockCacheKey.java}
> @Override
> public String toString() {
> return String.format("%s_%d", hfileName, offset);
> }
> {code}
> I found through bench-marking that the following code is 10x faster.
> {code:titi\le=BlockCacheKey.java}
> @Override
> public String toString() {
> return hfileName.concat("_").concat(Long.toString(offset));
> }
> {code}
> Normally it wouldn't matter for a _toString()_ method, but this is comes into
> play because {{MemcachedBlockCache}} uses it.
> {code:title=MemcachedBlockCache.java}
> @Override
> public void cacheBlock(BlockCacheKey cacheKey, Cacheable buf) {
> if (buf instanceof HFileBlock) {
> client.add(cacheKey.toString(), MAX_SIZE, (HFileBlock) buf, tc);
> } else {
> if (LOG.isDebugEnabled()) {
> LOG.debug("MemcachedBlockCache can not cache Cacheable's of type "
> + buf.getClass().toString());
> }
> }
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (HBASE-19684) BlockCacheKey toString Performance
[
https://issues.apache.org/jira/browse/HBASE-19684?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16308832#comment-16308832
]
BELUGA BEHR commented on HBASE-19684:
-
[~appy] All I'm doing is creating a list of 100K string with either the current
implementation or with
{code}
for (int i = 0; i < 10; i++) {
// Test 1: c.add(String.format("%s_%d", a, b));
// Test 2: c.add(a.concat("_").concat(Long.toString(b)));
// Test 3: c.add(new StringBuilder(a.length() +
21).append(a).append('_').append(b).toString());
// Test 4: c.add(a + '_' + b);
}
{code}
The current code with _String.format_ takes about 320ms to complete the _concat
implementation takes about 29ms to complete. As you point out, the absolute
magnitude is small, but measurable. However, 100K items into and out of the
cache is not that much so the savings, over time, accrues.
Please consider patch as is.
> BlockCacheKey toString Performance
> --
>
> Key: HBASE-19684
> URL: https://issues.apache.org/jira/browse/HBASE-19684
> Project: HBase
> Issue Type: Improvement
> Components: hbase
>Affects Versions: 3.0.0
>Reporter: BELUGA BEHR
>Assignee: BELUGA BEHR
>Priority: Trivial
> Attachments: HBASE-19684.1.patch
>
>
> {code:titile=BlockCacheKey.java}
> @Override
> public String toString() {
> return String.format("%s_%d", hfileName, offset);
> }
> {code}
> I found through bench-marking that the following code is 10x faster.
> {code:titi\le=BlockCacheKey.java}
> @Override
> public String toString() {
> return hfileName.concat("_").concat(Long.toString(offset));
> }
> {code}
> Normally it wouldn't matter for a _toString()_ method, but this is comes into
> play because {{MemcachedBlockCache}} uses it.
> {code:title=MemcachedBlockCache.java}
> @Override
> public void cacheBlock(BlockCacheKey cacheKey, Cacheable buf) {
> if (buf instanceof HFileBlock) {
> client.add(cacheKey.toString(), MAX_SIZE, (HFileBlock) buf, tc);
> } else {
> if (LOG.isDebugEnabled()) {
> LOG.debug("MemcachedBlockCache can not cache Cacheable's of type "
> + buf.getClass().toString());
> }
> }
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (HBASE-19684) BlockCacheKey toString Performance
[
https://issues.apache.org/jira/browse/HBASE-19684?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16308507#comment-16308507
]
Appy commented on HBASE-19684:
--
i like the way of reasoning! Please add that nice testing/finding of your to
the commit message if this gets in.
A question though, how did you benchmark...a word or two to understand the
method would be good.
10x faster is good, but sometimes absolute magnitude is so small that it's not
even worth it. Additionally, it's debug log, so doesn't really matter in this
case. Would prefer readability.
But i'm happy to get it in if just for the sake of your 'right way' - testing,
benchmarking, and reasoning to support one's work.
> BlockCacheKey toString Performance
> --
>
> Key: HBASE-19684
> URL: https://issues.apache.org/jira/browse/HBASE-19684
> Project: HBase
> Issue Type: Improvement
> Components: hbase
>Affects Versions: 3.0.0
>Reporter: BELUGA BEHR
>Assignee: BELUGA BEHR
>Priority: Trivial
> Attachments: HBASE-19684.1.patch
>
>
> {code:titile=BlockCacheKey.java}
> @Override
> public String toString() {
> return String.format("%s_%d", hfileName, offset);
> }
> {code}
> I found through bench-marking that the following code is 10x faster.
> {code:titi\le=BlockCacheKey.java}
> @Override
> public String toString() {
> return hfileName.concat("_").concat(Long.toString(offset));
> }
> {code}
> Normally it wouldn't matter for a _toString()_ method, but this is comes into
> play because {{MemcachedBlockCache}} uses it.
> {code:title=MemcachedBlockCache.java}
> @Override
> public void cacheBlock(BlockCacheKey cacheKey, Cacheable buf) {
> if (buf instanceof HFileBlock) {
> client.add(cacheKey.toString(), MAX_SIZE, (HFileBlock) buf, tc);
> } else {
> if (LOG.isDebugEnabled()) {
> LOG.debug("MemcachedBlockCache can not cache Cacheable's of type "
> + buf.getClass().toString());
> }
> }
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (HBASE-19684) BlockCacheKey toString Performance
[
https://issues.apache.org/jira/browse/HBASE-19684?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16308291#comment-16308291
]
Hadoop QA commented on HBASE-19684:
---
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 1m
50s{color} | {color:blue} Docker mode activated. {color} |
|| || || || {color:brown} Prechecks {color} ||
| {color:blue}0{color} | {color:blue} findbugs {color} | {color:blue} 0m
0s{color} | {color:blue} Findbugs executables are not available. {color} |
| {color:green}+1{color} | {color:green} hbaseanti {color} | {color:green} 0m
0s{color} | {color:green} Patch does not have any anti-patterns. {color} |
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m
0s{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:red}-1{color} | {color:red} test4tests {color} | {color:red} 0m
0s{color} | {color:red} The patch doesn't appear to include any new or modified
tests. Please justify why no new tests are needed for this patch. Also please
list what manual steps were performed to verify this patch. {color} |
|| || || || {color:brown} master Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 4m
30s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m
41s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 1m
2s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} shadedjars {color} | {color:green} 5m
37s{color} | {color:green} branch has no errors when building our shaded
downstream artifacts. {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
27s{color} | {color:green} master passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 4m
35s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m
41s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 0m
41s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 1m
2s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m
0s{color} | {color:green} The patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} shadedjars {color} | {color:green} 4m
33s{color} | {color:green} patch has no errors when building our shaded
downstream artifacts. {color} |
| {color:green}+1{color} | {color:green} hadoopcheck {color} | {color:green}
19m 4s{color} | {color:green} Patch does not cause any errors with Hadoop
2.6.5 2.7.4 or 3.0.0. {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
26s{color} | {color:green} the patch passed {color} |
|| || || || {color:brown} Other Tests {color} ||
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 97m
0s{color} | {color:green} hbase-server in the patch passed. {color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m
17s{color} | {color:green} The patch does not generate ASF License warnings.
{color} |
| {color:black}{color} | {color:black} {color} | {color:black}136m 22s{color} |
{color:black} {color} |
\\
\\
|| Subsystem || Report/Notes ||
| Docker | Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hbase:eee3b01 |
| JIRA Issue | HBASE-19684 |
| JIRA Patch URL |
https://issues.apache.org/jira/secure/attachment/12904139/HBASE-19684.1.patch |
| Optional Tests | asflicense javac javadoc unit findbugs shadedjars
hadoopcheck hbaseanti checkstyle compile |
| uname | Linux d2ba628616e1 3.13.0-133-generic #182-Ubuntu SMP Tue Sep 19
15:49:21 UTC 2017 x86_64 GNU/Linux |
| Build tool | maven |
| Personality |
/home/jenkins/jenkins-slave/workspace/PreCommit-HBASE-Build/component/dev-support/hbase-personality.sh
|
| git revision | master / 9ac95419dc |
| maven | version: Apache Maven 3.5.2
(138edd61fd100ec658bfa2d307c43b76940a5d7d; 2017-10-18T07:58:13Z) |
| Default Java | 1.8.0_151 |
| Test Results |
https://builds.apache.org/job/PreCommit-HBASE-Build/10846/testReport/ |
| modules | C: hbase-server U: hbase-server |
| Console output |
https://builds.apache.org/job/PreCommit-HBASE-Build/10846/console |
| Powered by | Apache Yetus 0.6.0 http://yetus.apache.org |
This message was automatically generated.
> BlockCacheKey toString Performance
> --
>
> Key:
[jira] [Commented] (HBASE-19684) BlockCacheKey toString Performance
[
https://issues.apache.org/jira/browse/HBASE-19684?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16308086#comment-16308086
]
BELUGA BEHR commented on HBASE-19684:
-
[~Apache9] Thanks for the review. :)
While I admit that it is easier to read an implementation with StringBuilder,
but in my micro-bench-marking, I found that the concat implementation was
faster. This was a little surprising, but StringBuilder class has to do some
additional work around ensuring the size of its internal buffer, including null
checks, even if it doesn't change the size of the buffer. In addition, the
default buffer size for StringBuffer is 16 and it is possible that a Long could
be [up to 19 characters
long|http://grepcode.com/file/repository.grepcode.com/java/root/jdk/openjdk/8u40-b25/java/lang/Long.java#Long.stringSize%28long%29],
so it may have to resize the buffer and copy contents, etc. The concat
implementation is therefore more predictable in performance, as it will always
perform the same steps regardless of sizes.
> BlockCacheKey toString Performance
> --
>
> Key: HBASE-19684
> URL: https://issues.apache.org/jira/browse/HBASE-19684
> Project: HBase
> Issue Type: Improvement
> Components: hbase
>Affects Versions: 3.0.0
>Reporter: BELUGA BEHR
>Assignee: BELUGA BEHR
>Priority: Trivial
> Attachments: HBASE-19684.1.patch
>
>
> {code:titile=BlockCacheKey.java}
> @Override
> public String toString() {
> return String.format("%s_%d", hfileName, offset);
> }
> {code}
> I found through bench-marking that the following code is 10x faster.
> {code:titi\le=BlockCacheKey.java}
> @Override
> public String toString() {
> return hfileName.concat("_").concat(Long.toString(offset));
> }
> {code}
> Normally it wouldn't matter for a _toString()_ method, but this is comes into
> play because {{MemcachedBlockCache}} uses it.
> {code:title=MemcachedBlockCache.java}
> @Override
> public void cacheBlock(BlockCacheKey cacheKey, Cacheable buf) {
> if (buf instanceof HFileBlock) {
> client.add(cacheKey.toString(), MAX_SIZE, (HFileBlock) buf, tc);
> } else {
> if (LOG.isDebugEnabled()) {
> LOG.debug("MemcachedBlockCache can not cache Cacheable's of type "
> + buf.getClass().toString());
> }
> }
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (HBASE-19684) BlockCacheKey toString Performance
[
https://issues.apache.org/jira/browse/HBASE-19684?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16307586#comment-16307586
]
Duo Zhang commented on HBASE-19684:
---
Will it be more faster just with hfileName + "_" + offset? It will use
StringBuilder so will not lead to multiple String creation.
> BlockCacheKey toString Performance
> --
>
> Key: HBASE-19684
> URL: https://issues.apache.org/jira/browse/HBASE-19684
> Project: HBase
> Issue Type: Improvement
> Components: hbase
>Affects Versions: 3.0.0
>Reporter: BELUGA BEHR
>Priority: Trivial
> Attachments: HBASE-19684.1.patch
>
>
> {code:titile=BlockCacheKey.java}
> @Override
> public String toString() {
> return String.format("%s_%d", hfileName, offset);
> }
> {code}
> I found through bench-marking that the following code is 10x faster.
> {code:titi\le=BlockCacheKey.java}
> @Override
> public String toString() {
> return hfileName.concat("_").concat(Long.toString(offset));
> }
> {code}
> Normally it wouldn't matter for a _toString()_ method, but this is comes into
> play because {{MemcachedBlockCache}} uses it.
> {code:title=MemcachedBlockCache.java}
> @Override
> public void cacheBlock(BlockCacheKey cacheKey, Cacheable buf) {
> if (buf instanceof HFileBlock) {
> client.add(cacheKey.toString(), MAX_SIZE, (HFileBlock) buf, tc);
> } else {
> if (LOG.isDebugEnabled()) {
> LOG.debug("MemcachedBlockCache can not cache Cacheable's of type "
> + buf.getClass().toString());
> }
> }
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
