You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by "Ted Yu (JIRA)" <ji...@apache.org> on 2018/03/15 15:51:00 UTC

[jira] [Commented] (HBASE-20208) Review of SequenceIdAccounting.java

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

Ted Yu commented on HBASE-20208:
--------------------------------

{code}
397	      long min = Math.min(oldestFlushing.longValue(), oldestUnflushed.longValue());
{code}
hmm. With the change, there would be additional boxing / unboxing involved.

> Review of SequenceIdAccounting.java
> -----------------------------------
>
>                 Key: HBASE-20208
>                 URL: https://issues.apache.org/jira/browse/HBASE-20208
>             Project: HBase
>          Issue Type: Improvement
>          Components: hbase
>    Affects Versions: 2.0.0
>            Reporter: BELUGA BEHR
>            Assignee: BELUGA BEHR
>            Priority: Minor
>         Attachments: HBASE-20208.1.patch
>
>
> # Fix checkstyle warnings
> # Use re-usable libraries where possible
> # Improve Map Access
> What got my attention on this class was:
> {code}
> for (Map.Entry<byte[], Long> e : sequenceids.entrySet()) {
>       long oldestFlushing = Long.MAX_VALUE;
>       long oldestUnflushed = Long.MAX_VALUE;
>       if (flushing != null && flushing.containsKey(e.getKey())) {
>         oldestFlushing = flushing.get(e.getKey());
>       }
>       if (unflushed != null && unflushed.containsKey(e.getKey())) {
>         oldestUnflushed = unflushed.get(e.getKey());
>       }
>       long min = Math.min(oldestFlushing, oldestUnflushed);
>       if (min <= e.getValue()) {
>         return false;
>       }
> {code}
> Here, the two maps are calling _containsKey_ and then _get_.  It is also calling {{e.getKey()}} repeatedly.
> I propose changing this so that {{e.getKey()}} is only called once and instead of looking up an entry with _containsKey_ and then a _get_, simply use _get_ once and check for a 'null' value to check for existence.  It saves two trips through the Map Collection on each loop.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)