hbase-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Hiroshi Ikeda (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HBASE-10656) high-scale-lib's Counter depends on Oracle (Sun) JRE, and also has some bug
Date Mon, 10 Mar 2014 09:34:53 GMT

    [ https://issues.apache.org/jira/browse/HBASE-10656?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13925593#comment-13925593
] 

Hiroshi Ikeda commented on HBASE-10656:
---------------------------------------

bq. Am I reading your table correctly? It says your implementation consistently faster than
cliff click lib and AtomicInteger?

Yes, the table says my counter works well.

For increment, comparing to high-scale-lib, the newly created counter has just a bit advantage,
which is coming from simplicity of logic, and just meaningful in micro-bench, I think.

For getting a value, high-sacle-lib's counter forgets to skip pads to sum up values (almost
bug). Using a volatile instance variable to cache the sum may also causes overhead, but anyway,
high-scale-lib's counter gives a wrong value so that I don't want to use it.

{quote}
What happens if the below does no longer holds?

+ // ...The cache-line size is expected to be equal to or
+ // less than about 128 Bytes (= 64 Bits * 16).
{quote}

By the previously uploaded result of my performance test, for AtomicLong, using more threads
requires more time. It starts contention using 2 threads, with gradually increasing to 8 threads,
which causes full contention because my environment has 8 core.

The same overhead may be occurred for the newly created counter if the actual cache line is
larger than the size I expected.
It seems that only just contention with 2 threads may be relatively large. It is possible
that the performance drastically reduces.

However, in the first place, the effect of adding pads basically depends on the implementation
of JRE. I referred the source code of LongAdder and added pads like it, and I think it is
practical because the LongAdder is written by an expert, but logically VM may eliminate that
unused pads under the name of optimization.

Also adding pads can waste memory. The newly created counter uses about 16 long variables
for one cell, and supposing the internal array requires 8 length to avoid contention, it requires
the same memory as 128 long variables per a counter. I think it is already sufficiently done
to waste memory.

bq. Looking at LongAdder, its public domain so copying from it is fine (I was concerned about
license issues).

I just borrow a trick of padding and copy&paste names of pad variables (that are never
used and any names will go) from LongAdder. There are the other ideas in LongAdder, but it
depends on VM implementation in the end, and performance costs of the ticks didn't seem trivial
comparing to the main logic (just increment), so I just give priority to make my counter simple.
It may be required to revise if it is too rough.

>  high-scale-lib's Counter depends on Oracle (Sun) JRE, and also has some bug
> ----------------------------------------------------------------------------
>
>                 Key: HBASE-10656
>                 URL: https://issues.apache.org/jira/browse/HBASE-10656
>             Project: HBase
>          Issue Type: Bug
>            Reporter: Hiroshi Ikeda
>            Priority: Minor
>         Attachments: HBASE-10656-0.96.patch, HBASE-10656-trunk.patch, MyCounter.java,
MyCounter2.java, MyCounterTest.java, MyCounterTest.java, PerformanceTestApp.java, output.pdf,
output.txt
>
>
> Cliff's high-scale-lib's Counter is used in important classes (for example, HRegion)
in HBase, but Counter uses sun.misc.Unsafe, that is implementation detail of the Java standard
library and belongs to Oracle (Sun). That consequently makes HBase depend on the specific
JRE Implementation.
> To make matters worse, Counter has a bug and you may get wrong result if you mix a reading
method into your logic calling writing methods.
> In more detail, I think the bug is caused by reading an internal array field without
resolving memory caching, which is intentional the comment says, but storing the read result
into a volatile field. That field may be not changed after you can see the true values of
the array field, and also may be not changed after updating the "next" CAT instance's values
in some race condition when extending CAT instance chain.
> Anyway, it is possible that you create a new alternative class which only depends on
the standard library. I know Java8 provides its alternative, but HBase should support Java6
and Java7 for some time.



--
This message was sent by Atlassian JIRA
(v6.2#6252)

Mime
View raw message