[
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)
