[jira] [Commented] (HADOOP-10169) remove the unnecessary synchronized in JvmMetrics class

2013-12-21 Thread Hudson (JIRA)

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

Hudson commented on HADOOP-10169:
-

FAILURE: Integrated in Hadoop-Hdfs-trunk #1619 (See 
[https://builds.apache.org/job/Hadoop-Hdfs-trunk/1619/])
HADOOP-10169. Remove the unnecessary synchronized in JvmMetrics class. 
Contributed by Liang Xie. (jing9: 
http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1552820)
* /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
* 
/hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/source/JvmMetrics.java


> remove the unnecessary  synchronized in JvmMetrics class
> 
>
> Key: HADOOP-10169
> URL: https://issues.apache.org/jira/browse/HADOOP-10169
> Project: Hadoop Common
>  Issue Type: Improvement
>  Components: metrics
>Affects Versions: 3.0.0, 2.2.0
>Reporter: Liang Xie
>Assignee: Liang Xie
>Priority: Minor
> Fix For: 2.4.0
>
> Attachments: HADOOP-10169-v2.txt, HADOOP-10169-v3.txt, 
> HADOOP-10169-v4.txt, HADOOP-10169.txt
>
>
> When i looked into a HBase JvmMetric impl, just found this synchronized seems 
> not essential.



--
This message was sent by Atlassian JIRA
(v6.1.5#6160)


[jira] [Commented] (HADOOP-10169) remove the unnecessary synchronized in JvmMetrics class

2013-12-21 Thread Hudson (JIRA)

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

Hudson commented on HADOOP-10169:
-

FAILURE: Integrated in Hadoop-Mapreduce-trunk #1645 (See 
[https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1645/])
HADOOP-10169. Remove the unnecessary synchronized in JvmMetrics class. 
Contributed by Liang Xie. (jing9: 
http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1552820)
* /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
* 
/hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/source/JvmMetrics.java


> remove the unnecessary  synchronized in JvmMetrics class
> 
>
> Key: HADOOP-10169
> URL: https://issues.apache.org/jira/browse/HADOOP-10169
> Project: Hadoop Common
>  Issue Type: Improvement
>  Components: metrics
>Affects Versions: 3.0.0, 2.2.0
>Reporter: Liang Xie
>Assignee: Liang Xie
>Priority: Minor
> Fix For: 2.4.0
>
> Attachments: HADOOP-10169-v2.txt, HADOOP-10169-v3.txt, 
> HADOOP-10169-v4.txt, HADOOP-10169.txt
>
>
> When i looked into a HBase JvmMetric impl, just found this synchronized seems 
> not essential.



--
This message was sent by Atlassian JIRA
(v6.1.5#6160)


[jira] [Commented] (HADOOP-10169) remove the unnecessary synchronized in JvmMetrics class

2013-12-21 Thread Hudson (JIRA)

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

Hudson commented on HADOOP-10169:
-

FAILURE: Integrated in Hadoop-Yarn-trunk #428 (See 
[https://builds.apache.org/job/Hadoop-Yarn-trunk/428/])
HADOOP-10169. Remove the unnecessary synchronized in JvmMetrics class. 
Contributed by Liang Xie. (jing9: 
http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1552820)
* /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
* 
/hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/source/JvmMetrics.java


> remove the unnecessary  synchronized in JvmMetrics class
> 
>
> Key: HADOOP-10169
> URL: https://issues.apache.org/jira/browse/HADOOP-10169
> Project: Hadoop Common
>  Issue Type: Improvement
>  Components: metrics
>Affects Versions: 3.0.0, 2.2.0
>Reporter: Liang Xie
>Assignee: Liang Xie
>Priority: Minor
> Fix For: 2.4.0
>
> Attachments: HADOOP-10169-v2.txt, HADOOP-10169-v3.txt, 
> HADOOP-10169-v4.txt, HADOOP-10169.txt
>
>
> When i looked into a HBase JvmMetric impl, just found this synchronized seems 
> not essential.



--
This message was sent by Atlassian JIRA
(v6.1.5#6160)


[jira] [Commented] (HADOOP-10169) remove the unnecessary synchronized in JvmMetrics class

2013-12-21 Thread Hudson (JIRA)

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

Hudson commented on HADOOP-10169:
-

SUCCESS: Integrated in Hadoop-trunk-Commit #4924 (See 
[https://builds.apache.org/job/Hadoop-trunk-Commit/4924/])
HADOOP-10169. Remove the unnecessary synchronized in JvmMetrics class. 
Contributed by Liang Xie. (jing9: 
http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1552820)
* /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
* 
/hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/source/JvmMetrics.java


> remove the unnecessary  synchronized in JvmMetrics class
> 
>
> Key: HADOOP-10169
> URL: https://issues.apache.org/jira/browse/HADOOP-10169
> Project: Hadoop Common
>  Issue Type: Improvement
>  Components: metrics
>Affects Versions: 3.0.0, 2.2.0
>Reporter: Liang Xie
>Assignee: Liang Xie
>Priority: Minor
> Fix For: 2.4.0
>
> Attachments: HADOOP-10169-v2.txt, HADOOP-10169-v3.txt, 
> HADOOP-10169-v4.txt, HADOOP-10169.txt
>
>
> When i looked into a HBase JvmMetric impl, just found this synchronized seems 
> not essential.



--
This message was sent by Atlassian JIRA
(v6.1.5#6160)


[jira] [Commented] (HADOOP-10169) remove the unnecessary synchronized in JvmMetrics class

2013-12-20 Thread Jing Zhao (JIRA)

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

Jing Zhao commented on HADOOP-10169:


+1 for the latest patch. Thanks for cleaning the code, Liang Xie! And thanks to 
Jason for the review! I will commit it shortly.

> remove the unnecessary  synchronized in JvmMetrics class
> 
>
> Key: HADOOP-10169
> URL: https://issues.apache.org/jira/browse/HADOOP-10169
> Project: Hadoop Common
>  Issue Type: Improvement
>  Components: metrics
>Affects Versions: 3.0.0, 2.2.0
>Reporter: Liang Xie
>Assignee: Liang Xie
>Priority: Minor
> Attachments: HADOOP-10169-v2.txt, HADOOP-10169-v3.txt, 
> HADOOP-10169-v4.txt, HADOOP-10169.txt
>
>
> When i looked into a HBase JvmMetric impl, just found this synchronized seems 
> not essential.



--
This message was sent by Atlassian JIRA
(v6.1.4#6159)


[jira] [Commented] (HADOOP-10169) remove the unnecessary synchronized in JvmMetrics class

2013-12-19 Thread Hadoop QA (JIRA)

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

Hadoop QA commented on HADOOP-10169:


{color:red}-1 overall{color}.  Here are the results of testing the latest 
attachment 
  http://issues.apache.org/jira/secure/attachment/12619729/HADOOP-10169-v4.txt
  against trunk revision .

{color:green}+1 @author{color}.  The patch does not contain any @author 
tags.

{color:red}-1 tests included{color}.  The patch doesn't appear to include 
any new or modified tests.
Please justify why no new tests are needed for this 
patch.
Also please list what manual steps were performed to 
verify this patch.

{color:green}+1 javac{color}.  The applied patch does not increase the 
total number of javac compiler warnings.

{color:green}+1 javadoc{color}.  The javadoc tool did not generate any 
warning messages.

{color:green}+1 eclipse:eclipse{color}.  The patch built with 
eclipse:eclipse.

{color:green}+1 findbugs{color}.  The patch does not introduce any new 
Findbugs (version 1.3.9) warnings.

{color:green}+1 release audit{color}.  The applied patch does not increase 
the total number of release audit warnings.

{color:green}+1 core tests{color}.  The patch passed unit tests in 
hadoop-common-project/hadoop-common.

{color:green}+1 contrib tests{color}.  The patch passed contrib unit tests.

Test results: 
https://builds.apache.org/job/PreCommit-HADOOP-Build/3375//testReport/
Console output: 
https://builds.apache.org/job/PreCommit-HADOOP-Build/3375//console

This message is automatically generated.

> remove the unnecessary  synchronized in JvmMetrics class
> 
>
> Key: HADOOP-10169
> URL: https://issues.apache.org/jira/browse/HADOOP-10169
> Project: Hadoop Common
>  Issue Type: Improvement
>  Components: metrics
>Affects Versions: 3.0.0, 2.2.0
>Reporter: Liang Xie
>Assignee: Liang Xie
>Priority: Minor
> Attachments: HADOOP-10169-v2.txt, HADOOP-10169-v3.txt, 
> HADOOP-10169-v4.txt, HADOOP-10169.txt
>
>
> When i looked into a HBase JvmMetric impl, just found this synchronized seems 
> not essential.



--
This message was sent by Atlassian JIRA
(v6.1.4#6159)


[jira] [Commented] (HADOOP-10169) remove the unnecessary synchronized in JvmMetrics class

2013-12-19 Thread Liang Xie (JIRA)

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

Liang Xie commented on HADOOP-10169:


Yes, you are right, now attached v4 patch.  Thanks for review, i really 
appreciate and enjoy it:) and have learned a lot from those detail code/styles.

> remove the unnecessary  synchronized in JvmMetrics class
> 
>
> Key: HADOOP-10169
> URL: https://issues.apache.org/jira/browse/HADOOP-10169
> Project: Hadoop Common
>  Issue Type: Improvement
>  Components: metrics
>Affects Versions: 3.0.0, 2.2.0
>Reporter: Liang Xie
>Assignee: Liang Xie
>Priority: Minor
> Attachments: HADOOP-10169-v2.txt, HADOOP-10169-v3.txt, 
> HADOOP-10169-v4.txt, HADOOP-10169.txt
>
>
> When i looked into a HBase JvmMetric impl, just found this synchronized seems 
> not essential.



--
This message was sent by Atlassian JIRA
(v6.1.4#6159)


[jira] [Commented] (HADOOP-10169) remove the unnecessary synchronized in JvmMetrics class

2013-12-19 Thread Jason Lowe (JIRA)

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

Jason Lowe commented on HADOOP-10169:
-

Thanks for updating the patch.  One last nit is that this code:

{code}
  if (gcInfo == null) {
  gcInfo = new MetricsInfo[2];
  
  MetricsInfo[] previousGcInfo = gcInfoCache.putIfAbsent(gcName, gcInfo);
  if (previousGcInfo == null) {
return gcInfo;
  }
  return previousGcInfo;
  }
  return gcInfo;
{code}

can be simplified to the following so there aren't so many return statements to 
track:

{code}
  if (gcInfo == null) {
  gcInfo = new MetricsInfo[2];
  
  MetricsInfo[] previousGcInfo = gcInfoCache.putIfAbsent(gcName, gcInfo);
  if (previousGcInfo != null) {
gcInfo = previousGcInfo;
  }
  }
  return gcInfo;
{code}

> remove the unnecessary  synchronized in JvmMetrics class
> 
>
> Key: HADOOP-10169
> URL: https://issues.apache.org/jira/browse/HADOOP-10169
> Project: Hadoop Common
>  Issue Type: Improvement
>  Components: metrics
>Affects Versions: 3.0.0, 2.2.0
>Reporter: Liang Xie
>Assignee: Liang Xie
>Priority: Minor
> Attachments: HADOOP-10169-v2.txt, HADOOP-10169-v3.txt, 
> HADOOP-10169.txt
>
>
> When i looked into a HBase JvmMetric impl, just found this synchronized seems 
> not essential.



--
This message was sent by Atlassian JIRA
(v6.1.4#6159)


[jira] [Commented] (HADOOP-10169) remove the unnecessary synchronized in JvmMetrics class

2013-12-18 Thread Hadoop QA (JIRA)

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

Hadoop QA commented on HADOOP-10169:


{color:red}-1 overall{color}.  Here are the results of testing the latest 
attachment 
  http://issues.apache.org/jira/secure/attachment/12619459/HADOOP-10169-v3.txt
  against trunk revision .

{color:green}+1 @author{color}.  The patch does not contain any @author 
tags.

{color:red}-1 tests included{color}.  The patch doesn't appear to include 
any new or modified tests.
Please justify why no new tests are needed for this 
patch.
Also please list what manual steps were performed to 
verify this patch.

{color:green}+1 javac{color}.  The applied patch does not increase the 
total number of javac compiler warnings.

{color:green}+1 javadoc{color}.  The javadoc tool did not generate any 
warning messages.

{color:green}+1 eclipse:eclipse{color}.  The patch built with 
eclipse:eclipse.

{color:green}+1 findbugs{color}.  The patch does not introduce any new 
Findbugs (version 1.3.9) warnings.

{color:green}+1 release audit{color}.  The applied patch does not increase 
the total number of release audit warnings.

{color:green}+1 core tests{color}.  The patch passed unit tests in 
hadoop-common-project/hadoop-common.

{color:green}+1 contrib tests{color}.  The patch passed contrib unit tests.

Test results: 
https://builds.apache.org/job/PreCommit-HADOOP-Build/3370//testReport/
Console output: 
https://builds.apache.org/job/PreCommit-HADOOP-Build/3370//console

This message is automatically generated.

> remove the unnecessary  synchronized in JvmMetrics class
> 
>
> Key: HADOOP-10169
> URL: https://issues.apache.org/jira/browse/HADOOP-10169
> Project: Hadoop Common
>  Issue Type: Improvement
>  Components: metrics
>Affects Versions: 3.0.0, 2.2.0
>Reporter: Liang Xie
>Assignee: Liang Xie
>Priority: Minor
> Attachments: HADOOP-10169-v2.txt, HADOOP-10169-v3.txt, 
> HADOOP-10169.txt
>
>
> When i looked into a HBase JvmMetric impl, just found this synchronized seems 
> not essential.



--
This message was sent by Atlassian JIRA
(v6.1.4#6159)


[jira] [Commented] (HADOOP-10169) remove the unnecessary synchronized in JvmMetrics class

2013-12-18 Thread Liang Xie (JIRA)

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

Liang Xie commented on HADOOP-10169:


Thanks [~jlowe] for your nice reply, yes, you are correct!  Attached v3 with an 
IDE formatting now:)

> remove the unnecessary  synchronized in JvmMetrics class
> 
>
> Key: HADOOP-10169
> URL: https://issues.apache.org/jira/browse/HADOOP-10169
> Project: Hadoop Common
>  Issue Type: Improvement
>  Components: metrics
>Affects Versions: 3.0.0, 2.2.0
>Reporter: Liang Xie
>Assignee: Liang Xie
>Priority: Minor
> Attachments: HADOOP-10169-v2.txt, HADOOP-10169-v3.txt, 
> HADOOP-10169.txt
>
>
> When i looked into a HBase JvmMetric impl, just found this synchronized seems 
> not essential.



--
This message was sent by Atlassian JIRA
(v6.1.4#6159)


[jira] [Commented] (HADOOP-10169) remove the unnecessary synchronized in JvmMetrics class

2013-12-18 Thread Jason Lowe (JIRA)

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

Jason Lowe commented on HADOOP-10169:
-

Doh, in my haste I misread the code, so my pseudo-code isn't relevant.  We're 
not initializing the map just once overall, we're initializing it once per key. 
 As you point out, we can be initializing a new key as we're busy accessing an 
old key, and something like a ConcurrentMap is more appropriate.

However in general we cannot just replace HashMap with ConcurrentHashMap, 
remove the synchronized keywords, and expect it to work properly in all cases.  
There's now a race in getGcInfo where thread A comes along, sees there isn't an 
entry in the map for key K, and starts creating an empty MetricsInfo for it.  
Meanwhile thread B comes along, also sees there isn't an entry for key K, 
creates an empty MetricsInfo, puts it in the map, updates the MetricsInfo with 
new metrics, and continues on.  Thread A then wakes back up and pokes the empty 
MetricsInfo into the map for key K, causing data loss of the metrics computed 
by thread B.  The gcInfoCache.put needs to be gcInfoCache.putIfAbsent, and if 
putIfAbsent returns a value then we need to return that instead of the empty 
metrics info.

Couple of other nits on the patch: the com.google.common.collect.Maps import is 
no longer necessary, and the patch includes an unrelated whitespace change that 
pushes one of the modified lines over 80 columns.

> remove the unnecessary  synchronized in JvmMetrics class
> 
>
> Key: HADOOP-10169
> URL: https://issues.apache.org/jira/browse/HADOOP-10169
> Project: Hadoop Common
>  Issue Type: Improvement
>  Components: metrics
>Affects Versions: 3.0.0, 2.2.0
>Reporter: Liang Xie
>Assignee: Liang Xie
>Priority: Minor
> Attachments: HADOOP-10169-v2.txt, HADOOP-10169.txt
>
>
> When i looked into a HBase JvmMetric impl, just found this synchronized seems 
> not essential.



--
This message was sent by Atlassian JIRA
(v6.1.4#6159)


[jira] [Commented] (HADOOP-10169) remove the unnecessary synchronized in JvmMetrics class

2013-12-17 Thread Hadoop QA (JIRA)

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

Hadoop QA commented on HADOOP-10169:


{color:red}-1 overall{color}.  Here are the results of testing the latest 
attachment 
  http://issues.apache.org/jira/secure/attachment/12619238/HADOOP-10169-v2.txt
  against trunk revision .

{color:green}+1 @author{color}.  The patch does not contain any @author 
tags.

{color:red}-1 tests included{color}.  The patch doesn't appear to include 
any new or modified tests.
Please justify why no new tests are needed for this 
patch.
Also please list what manual steps were performed to 
verify this patch.

{color:green}+1 javac{color}.  The applied patch does not increase the 
total number of javac compiler warnings.

{color:green}+1 javadoc{color}.  The javadoc tool did not generate any 
warning messages.

{color:green}+1 eclipse:eclipse{color}.  The patch built with 
eclipse:eclipse.

{color:green}+1 findbugs{color}.  The patch does not introduce any new 
Findbugs (version 1.3.9) warnings.

{color:green}+1 release audit{color}.  The applied patch does not increase 
the total number of release audit warnings.

{color:green}+1 core tests{color}.  The patch passed unit tests in 
hadoop-common-project/hadoop-common.

{color:green}+1 contrib tests{color}.  The patch passed contrib unit tests.

Test results: 
https://builds.apache.org/job/PreCommit-HADOOP-Build/3366//testReport/
Console output: 
https://builds.apache.org/job/PreCommit-HADOOP-Build/3366//console

This message is automatically generated.

> remove the unnecessary  synchronized in JvmMetrics class
> 
>
> Key: HADOOP-10169
> URL: https://issues.apache.org/jira/browse/HADOOP-10169
> Project: Hadoop Common
>  Issue Type: Improvement
>  Components: metrics
>Affects Versions: 3.0.0, 2.2.0
>Reporter: Liang Xie
>Assignee: Liang Xie
>Priority: Minor
> Attachments: HADOOP-10169-v2.txt, HADOOP-10169.txt
>
>
> When i looked into a HBase JvmMetric impl, just found this synchronized seems 
> not essential.



--
This message was sent by Atlassian JIRA
(v6.1.4#6159)


[jira] [Commented] (HADOOP-10169) remove the unnecessary synchronized in JvmMetrics class

2013-12-17 Thread Liang Xie (JIRA)

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

Liang Xie commented on HADOOP-10169:


[~jingzhao], [~jlowe], thanks for your review, yes, seems my original thought 
has some flaw:)
I just attached another small v2 patch, could you help to review again?  thanks 
in advance! I think it's enough to use the ConcurrentHashMap to replace the big 
Synchronized, and it should be OK for all the read-only requests once 
initialization done.

[~jlowe], thanks for your detailed pseudo-code, IMHO, it seems a bit tricky 
here to work correctly maybe, since the getGcInfo() is invoked by a loop in 
getGcUsage(), then if the first time in loop initialized successful, then the 
latter ones will be omitted:)

> remove the unnecessary  synchronized in JvmMetrics class
> 
>
> Key: HADOOP-10169
> URL: https://issues.apache.org/jira/browse/HADOOP-10169
> Project: Hadoop Common
>  Issue Type: Improvement
>  Components: metrics
>Affects Versions: 3.0.0, 2.2.0
>Reporter: Liang Xie
>Assignee: Liang Xie
>Priority: Minor
> Attachments: HADOOP-10169-v2.txt, HADOOP-10169.txt
>
>
> When i looked into a HBase JvmMetric impl, just found this synchronized seems 
> not essential.



--
This message was sent by Atlassian JIRA
(v6.1.4#6159)


[jira] [Commented] (HADOOP-10169) remove the unnecessary synchronized in JvmMetrics class

2013-12-17 Thread Jason Lowe (JIRA)

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

Jason Lowe commented on HADOOP-10169:
-

As Jing pointed out, the function needs to be synchronized as written.  It's 
true that once initialized the code will no longer modify the map, but there's 
a race during the initialization itself.  If two or more threads call this 
function before it's initialized then they can both attempt to put() at the 
same time.  Alternatively, a thread could be late to the party and be calling 
get() just as the initializing thread is calling put() which is also a thread 
safety violation.

One possible way to avoid the synchronization bottleneck is to use a volatile 
boolean to indicate whether it's initialized so the vast majority of callers 
don't have to grab a lock.  Something like this pseudo-code:

{code}
volatile boolean isInitialized = false;

  if (!isInitialized) {
synchronized (gcInfoCache) {
  if (!isInitialized) {
.
gcInfoCache.put(...);
  }
}
isInitialized = true;
  }
  return gcInfoCache.get(...);
{code}

> remove the unnecessary  synchronized in JvmMetrics class
> 
>
> Key: HADOOP-10169
> URL: https://issues.apache.org/jira/browse/HADOOP-10169
> Project: Hadoop Common
>  Issue Type: Improvement
>  Components: metrics
>Affects Versions: 3.0.0, 2.2.0
>Reporter: Liang Xie
>Assignee: Liang Xie
>Priority: Minor
> Attachments: HADOOP-10169.txt
>
>
> When i looked into a HBase JvmMetric impl, just found this synchronized seems 
> not essential.



--
This message was sent by Atlassian JIRA
(v6.1.4#6159)


[jira] [Commented] (HADOOP-10169) remove the unnecessary synchronized in JvmMetrics class

2013-12-17 Thread Liang Xie (JIRA)

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

Liang Xie commented on HADOOP-10169:


The map gcInfoCache will be filled up once the first getMetrics() be invoked, 
seems then it'll be keep unchanged always.
Then during almost of all the life cycle, the gcInfoCache will be read only 
without any write. Do we still need the synchronized?
Am i missing sth ?  thanks for taking a look at it:)

> remove the unnecessary  synchronized in JvmMetrics class
> 
>
> Key: HADOOP-10169
> URL: https://issues.apache.org/jira/browse/HADOOP-10169
> Project: Hadoop Common
>  Issue Type: Improvement
>  Components: metrics
>Affects Versions: 3.0.0, 2.2.0
>Reporter: Liang Xie
>Assignee: Liang Xie
>Priority: Minor
> Attachments: HADOOP-10169.txt
>
>
> When i looked into a HBase JvmMetric impl, just found this synchronized seems 
> not essential.



--
This message was sent by Atlassian JIRA
(v6.1.4#6159)


[jira] [Commented] (HADOOP-10169) remove the unnecessary synchronized in JvmMetrics class

2013-12-16 Thread Jing Zhao (JIRA)

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

Jing Zhao commented on HADOOP-10169:


Is it possible that multiple threads call getMetrics(MetricsCollector, boolean) 
around the same time, which causes getGcInfo to be called concurrently? In that 
case, we still need the synchronized here?

> remove the unnecessary  synchronized in JvmMetrics class
> 
>
> Key: HADOOP-10169
> URL: https://issues.apache.org/jira/browse/HADOOP-10169
> Project: Hadoop Common
>  Issue Type: Improvement
>  Components: metrics
>Affects Versions: 3.0.0, 2.2.0
>Reporter: Liang Xie
>Assignee: Liang Xie
>Priority: Minor
> Attachments: HADOOP-10169.txt
>
>
> When i looked into a HBase JvmMetric impl, just found this synchronized seems 
> not essential.



--
This message was sent by Atlassian JIRA
(v6.1.4#6159)


[jira] [Commented] (HADOOP-10169) remove the unnecessary synchronized in JvmMetrics class

2013-12-16 Thread Hadoop QA (JIRA)

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

Hadoop QA commented on HADOOP-10169:


{color:red}-1 overall{color}.  Here are the results of testing the latest 
attachment 
  http://issues.apache.org/jira/secure/attachment/12619040/HADOOP-10169.txt
  against trunk revision .

{color:green}+1 @author{color}.  The patch does not contain any @author 
tags.

{color:red}-1 tests included{color}.  The patch doesn't appear to include 
any new or modified tests.
Please justify why no new tests are needed for this 
patch.
Also please list what manual steps were performed to 
verify this patch.

{color:green}+1 javac{color}.  The applied patch does not increase the 
total number of javac compiler warnings.

{color:green}+1 javadoc{color}.  The javadoc tool did not generate any 
warning messages.

{color:green}+1 eclipse:eclipse{color}.  The patch built with 
eclipse:eclipse.

{color:green}+1 findbugs{color}.  The patch does not introduce any new 
Findbugs (version 1.3.9) warnings.

{color:green}+1 release audit{color}.  The applied patch does not increase 
the total number of release audit warnings.

{color:green}+1 core tests{color}.  The patch passed unit tests in 
hadoop-common-project/hadoop-common.

{color:green}+1 contrib tests{color}.  The patch passed contrib unit tests.

Test results: 
https://builds.apache.org/job/PreCommit-HADOOP-Build/3363//testReport/
Console output: 
https://builds.apache.org/job/PreCommit-HADOOP-Build/3363//console

This message is automatically generated.

> remove the unnecessary  synchronized in JvmMetrics class
> 
>
> Key: HADOOP-10169
> URL: https://issues.apache.org/jira/browse/HADOOP-10169
> Project: Hadoop Common
>  Issue Type: Improvement
>  Components: metrics
>Affects Versions: 3.0.0, 2.2.0
>Reporter: Liang Xie
>Assignee: Liang Xie
>Priority: Minor
> Attachments: HADOOP-10169.txt
>
>
> When i looked into a HBase JvmMetric impl, just found this synchronized seems 
> not essential.



--
This message was sent by Atlassian JIRA
(v6.1.4#6159)