[
https://issues.apache.org/jira/browse/HDFS-14541?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16865764#comment-16865764
]
Íñigo Goiri commented on HDFS-14541:
------------------------------------
Thanks [~leosun08] for the patch.
Handling these scenarios through exception is always a bad idea; thanks for
finding this.
As we are cleaning this up, I would get rid of the {{while(true)}} and make it
{{while(!evictable.isEmpty())}}.
For the other while loop, similarly: {{while (evictable.size() +
evictableMmapped.isEmpty() > maxTotalSize)}}.
I have to say that this class in general could use some cleanup.
I'm sure we can do better than have a map that saves both longs and
{{ShortCircuitReplica}}.
For the code in lines 541-551 I would extend the comment.
Probably we could extend the javadoc explaining the ideas.
I'm not that familiar with this code so... how is the test coverage in this
area?
Are we covering the corner cases here?
> ShortCircuitReplica#unref cost about 6% cpu and 6% heap allocation because of
> the frequent thrown NoSuchElementException in our HBase benchmark
> ------------------------------------------------------------------------------------------------------------------------------------------------
>
> Key: HDFS-14541
> URL: https://issues.apache.org/jira/browse/HDFS-14541
> Project: Hadoop HDFS
> Issue Type: Bug
> Reporter: Zheng Hu
> Assignee: Lisheng Sun
> Priority: Major
> Attachments: HDFS-14541.000.patch, async-prof-pid-94152-alloc-2.svg,
> async-prof-pid-94152-cpu-1.svg
>
>
> Our XiaoMi HBase team are evaluating the performence improvement of
> HBASE-21879, and we have few CPU flame graph & heap flame graph by using
> async-profiler, and find that there're some performence issues in DFSClient
> .
> See the attached two flame graphs, we can conclude that the try catch block
> in ShortCircuitCache#trimEvictionMaps has some serious perf problem , we
> should remove the try catch from DFSClient.
> {code}
> /**
> * Trim the eviction lists.
> */
> private void trimEvictionMaps() {
> long now = Time.monotonicNow();
> demoteOldEvictableMmaped(now);
> while (true) {
> long evictableSize = evictable.size();
> long evictableMmappedSize = evictableMmapped.size();
> if (evictableSize + evictableMmappedSize <= maxTotalSize) {
> return;
> }
> ShortCircuitReplica replica;
> try {
> if (evictableSize == 0) {
> replica = (ShortCircuitReplica)evictableMmapped.get(evictableMmapped
> .firstKey());
> } else {
> replica = (ShortCircuitReplica)evictable.get(evictable.firstKey());
> }
> } catch (NoSuchElementException e) {
> break;
> }
> if (LOG.isTraceEnabled()) {
> LOG.trace(this + ": trimEvictionMaps is purging " + replica +
> StringUtils.getStackTrace(Thread.currentThread()));
> }
> purge(replica);
> }
> }
> {code}
> Our Xiaomi HDFS Team member [~leosun08] will prepare patch for this issue.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
