Return-Path: X-Original-To: archive-asf-public-internal@cust-asf2.ponee.io Delivered-To: archive-asf-public-internal@cust-asf2.ponee.io Received: from cust-asf.ponee.io (cust-asf.ponee.io [163.172.22.183]) by cust-asf2.ponee.io (Postfix) with ESMTP id AAFEF20049E for ; Thu, 10 Aug 2017 18:04:05 +0200 (CEST) Received: by cust-asf.ponee.io (Postfix) id A9BEF16B90E; Thu, 10 Aug 2017 16:04:05 +0000 (UTC) Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by cust-asf.ponee.io (Postfix) with SMTP id F207216B90C for ; Thu, 10 Aug 2017 18:04:04 +0200 (CEST) Received: (qmail 82120 invoked by uid 500); 10 Aug 2017 16:04:04 -0000 Mailing-List: contact issues-help@hbase.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Delivered-To: mailing list issues@hbase.apache.org Received: (qmail 82109 invoked by uid 99); 10 Aug 2017 16:04:04 -0000 Received: from pnap-us-west-generic-nat.apache.org (HELO spamd4-us-west.apache.org) (209.188.14.142) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 10 Aug 2017 16:04:04 +0000 Received: from localhost (localhost [127.0.0.1]) by spamd4-us-west.apache.org (ASF Mail Server at spamd4-us-west.apache.org) with ESMTP id 9FAC8C04DC for ; Thu, 10 Aug 2017 16:04:03 +0000 (UTC) X-Virus-Scanned: Debian amavisd-new at spamd4-us-west.apache.org X-Spam-Flag: NO X-Spam-Score: -99.202 X-Spam-Level: X-Spam-Status: No, score=-99.202 tagged_above=-999 required=6.31 tests=[KAM_ASCII_DIVIDERS=0.8, RP_MATCHES_RCVD=-0.001, SPF_PASS=-0.001, USER_IN_WHITELIST=-100] autolearn=disabled Received: from mx1-lw-us.apache.org ([10.40.0.8]) by localhost (spamd4-us-west.apache.org [10.40.0.11]) (amavisd-new, port 10024) with ESMTP id Zg0PxubC67eM for ; Thu, 10 Aug 2017 16:04:02 +0000 (UTC) Received: from mailrelay1-us-west.apache.org (mailrelay1-us-west.apache.org [209.188.14.139]) by mx1-lw-us.apache.org (ASF Mail Server at mx1-lw-us.apache.org) with ESMTP id 306C05F6C2 for ; Thu, 10 Aug 2017 16:04:02 +0000 (UTC) Received: from jira-lw-us.apache.org (unknown [207.244.88.139]) by mailrelay1-us-west.apache.org (ASF Mail Server at mailrelay1-us-west.apache.org) with ESMTP id 3F49CE0051 for ; Thu, 10 Aug 2017 16:04:01 +0000 (UTC) Received: from jira-lw-us.apache.org (localhost [127.0.0.1]) by jira-lw-us.apache.org (ASF Mail Server at jira-lw-us.apache.org) with ESMTP id 5200E24162 for ; Thu, 10 Aug 2017 16:04:00 +0000 (UTC) Date: Thu, 10 Aug 2017 16:04:00 +0000 (UTC) From: "Xiang Li (JIRA)" To: issues@hbase.apache.org Message-ID: In-Reply-To: References:

Subject: [jira] [Updated] (HBASE-18555) Remove redundant familyMap.put() from Put#addColumn() and addImmutable() MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-JIRA-FingerPrint: 30527f35849b9dde25b450d4833f0394 archived-at: Thu, 10 Aug 2017 16:04:05 -0000 [ https://issues.apache.org/jira/browse/HBASE-18555?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Xiang Li updated HBASE-18555: ----------------------------- Description: In Put#addColumn() and addImmutable(), after getting the cell list of the given family and add the cell into the list, the code puts (key=family, value=list) into familyMap. In addColumn(), it is like {code} List list = getCellList(family); KeyValue kv = createPutKeyValue(family, qualifier, ts, value); list.add(kv); familyMap.put(CellUtil.cloneFamily(kv), list); // <-- here return this; {code} In addImmutable(), it is like {code} List list = getCellList(family); KeyValue kv = createPutKeyValue(family, qualifier, ts, value, tag); list.add(kv); familyMap.put(family, list); // <-- here return this; {code} I think those put() for Map only take effect when getCellList(family) returns a new allocated ArrayList. When the list for a family already exist, put() for Map will update the value to the reference of the list, but actually, the reference of the list is not changed. Those put() do not do any harm in terms of the correctness when they are here. But it could be removed to improve the performance. familyMap searches for key and set the new value and return the old value. Those operation take some time but actually are not needed. The put() could be moved into Mutation#getCellList(family) was: In Put#addColumn() and addImmutable(), after getting cell list of the given family and add the cell into the list, the code will put (key=family, value=list) into family map. In addColumn(), it is like {code} List list = getCellList(family); KeyValue kv = createPutKeyValue(family, qualifier, ts, value); list.add(kv); familyMap.put(CellUtil.cloneFamily(kv), list); // <-- here return this; {code} In addImmutable(), it is like {code} List list = getCellList(family); KeyValue kv = createPutKeyValue(family, qualifier, ts, value, tag); list.add(kv); familyMap.put(family, list); // <-- here return this; {code} I think put() for Map only take effect when getCellList(family) returns a new allocated ArrayList. When the list for a family already exist, put() for Map will update the value to the reference of the list, but actually, the reference of the list is not changed. Those put() do not do any harm in terms of the correctness when they are here. But it could be removed to improve the performance, as familyMap is a TreeMap, in put(), it searches for key and set the new value and return the old value. Those operation take some time but actually are not needed. The put() could be moved into Mutation#getCellList(family) > Remove redundant familyMap.put() from Put#addColumn() and addImmutable() > ------------------------------------------------------------------------ > > Key: HBASE-18555 > URL: https://issues.apache.org/jira/browse/HBASE-18555 > Project: HBase > Issue Type: Improvement > Components: Client > Reporter: Xiang Li > Assignee: Xiang Li > Priority: Minor > > In Put#addColumn() and addImmutable(), after getting the cell list of the given family and add the cell into the list, the code puts (key=family, value=list) into familyMap. > In addColumn(), it is like > {code} > List list = getCellList(family); > KeyValue kv = createPutKeyValue(family, qualifier, ts, value); > list.add(kv); > familyMap.put(CellUtil.cloneFamily(kv), list); // <-- here > return this; > {code} > In addImmutable(), it is like > {code} > List list = getCellList(family); > KeyValue kv = createPutKeyValue(family, qualifier, ts, value, tag); > list.add(kv); > familyMap.put(family, list); // <-- here > return this; > {code} > I think those put() for Map only take effect when getCellList(family) returns a new allocated ArrayList. When the list for a family already exist, put() for Map will update the value to the reference of the list, but actually, the reference of the list is not changed. > Those put() do not do any harm in terms of the correctness when they are here. But it could be removed to improve the performance. familyMap searches for key and set the new value and return the old value. Those operation take some time but actually are not needed. > The put() could be moved into Mutation#getCellList(family) -- This message was sent by Atlassian JIRA (v6.4.14#64029)