You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-dev@hadoop.apache.org by "Ron Bodkin (JIRA)" <ji...@apache.org> on 2007/02/13 20:10:05 UTC
[jira] Created: (HADOOP-1017) Optimization: Reduce Overhead from
ReflectionUtils.newInstance
Optimization: Reduce Overhead from ReflectionUtils.newInstance
--------------------------------------------------------------
Key: HADOOP-1017
URL: https://issues.apache.org/jira/browse/HADOOP-1017
Project: Hadoop
Issue Type: Improvement
Components: util
Reporter: Ron Bodkin
Attachments: ReflectionUtils.patch.txt
I found that a significant amount of time on my project was being spent in creating constructors for each row of data. I dramatically optimized this performance by creating a simple WeakHashMap to cache constructors by class. For example, in a sample job I find that ReflectionUtils.newInstance takes 200 ms (2% of total) with the cache enabled, but it uses 900 ms (6% of total) without the cache.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-1017) Optimization: Reduce Overhead from
ReflectionUtils.newInstance
Posted by "Ron Bodkin (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-1017?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12473794 ]
Ron Bodkin commented on HADOOP-1017:
------------------------------------
How about using a concurrent map if one is available (on Java 5+ or if the concurrent backport is on the classpath), but falling back to a synchronized map if one is not? I've implemented code like that before (in our environment we run single threaded Hadoop jobs so I wasn't aware of the need for thread safety).
You are right about the values having a reference back to the Class -I think making the map just a HashMap is probably the right approach, since in most programs there would be only a handful and the classes won't need to be gc'd anyhow. it would be possible to make the values SoftReferences instead to allow collecting Classes but to make the cache less likely to lose useful data.
The cache member certainly could be final and named CACHE, good idea.
> Optimization: Reduce Overhead from ReflectionUtils.newInstance
> --------------------------------------------------------------
>
> Key: HADOOP-1017
> URL: https://issues.apache.org/jira/browse/HADOOP-1017
> Project: Hadoop
> Issue Type: Improvement
> Components: util
> Reporter: Ron Bodkin
> Attachments: cacheCtor.patch, ReflectionUtils.patch.txt, TestReflectionUtils.java
>
>
> I found that a significant amount of time on my project was being spent in creating constructors for each row of data. I dramatically optimized this performance by creating a simple WeakHashMap to cache constructors by class. For example, in a sample job I find that ReflectionUtils.newInstance takes 200 ms (2% of total) with the cache enabled, but it uses 900 ms (6% of total) without the cache.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HADOOP-1017) Optimization: Reduce Overhead from
ReflectionUtils.newInstance
Posted by "Doug Cutting (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-1017?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Doug Cutting updated HADOOP-1017:
---------------------------------
Resolution: Fixed
Status: Resolved (was: Patch Available)
I just committed this. Thanks, Ron!
> Optimization: Reduce Overhead from ReflectionUtils.newInstance
> --------------------------------------------------------------
>
> Key: HADOOP-1017
> URL: https://issues.apache.org/jira/browse/HADOOP-1017
> Project: Hadoop
> Issue Type: Improvement
> Components: util
> Reporter: Ron Bodkin
> Fix For: 0.12.0
>
> Attachments: cacheCtor.patch, cacheCtor.patch, ReflectionUtils.patch.txt, TestReflectionUtils.java
>
>
> I found that a significant amount of time on my project was being spent in creating constructors for each row of data. I dramatically optimized this performance by creating a simple WeakHashMap to cache constructors by class. For example, in a sample job I find that ReflectionUtils.newInstance takes 200 ms (2% of total) with the cache enabled, but it uses 900 ms (6% of total) without the cache.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HADOOP-1017) Optimization: Reduce Overhead from
ReflectionUtils.newInstance
Posted by "Ron Bodkin (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-1017?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Ron Bodkin updated HADOOP-1017:
-------------------------------
Attachment: cacheCtor.patch
Here is the revised simplified patch following the contribution guidelines.
> Optimization: Reduce Overhead from ReflectionUtils.newInstance
> --------------------------------------------------------------
>
> Key: HADOOP-1017
> URL: https://issues.apache.org/jira/browse/HADOOP-1017
> Project: Hadoop
> Issue Type: Improvement
> Components: util
> Reporter: Ron Bodkin
> Attachments: cacheCtor.patch, ReflectionUtils.patch.txt, TestReflectionUtils.java
>
>
> I found that a significant amount of time on my project was being spent in creating constructors for each row of data. I dramatically optimized this performance by creating a simple WeakHashMap to cache constructors by class. For example, in a sample job I find that ReflectionUtils.newInstance takes 200 ms (2% of total) with the cache enabled, but it uses 900 ms (6% of total) without the cache.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-1017) Optimization: Reduce Overhead from
ReflectionUtils.newInstance
Posted by "Owen O'Malley (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-1017?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12473793 ]
Owen O'Malley commented on HADOOP-1017:
---------------------------------------
I'd vote for the synchronized map. The failure mode would just be that you look up the constructor a second time.
> Optimization: Reduce Overhead from ReflectionUtils.newInstance
> --------------------------------------------------------------
>
> Key: HADOOP-1017
> URL: https://issues.apache.org/jira/browse/HADOOP-1017
> Project: Hadoop
> Issue Type: Improvement
> Components: util
> Reporter: Ron Bodkin
> Attachments: cacheCtor.patch, ReflectionUtils.patch.txt, TestReflectionUtils.java
>
>
> I found that a significant amount of time on my project was being spent in creating constructors for each row of data. I dramatically optimized this performance by creating a simple WeakHashMap to cache constructors by class. For example, in a sample job I find that ReflectionUtils.newInstance takes 200 ms (2% of total) with the cache enabled, but it uses 900 ms (6% of total) without the cache.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HADOOP-1017) Optimization: Reduce Overhead from
ReflectionUtils.newInstance
Posted by "Doug Cutting (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-1017?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Doug Cutting updated HADOOP-1017:
---------------------------------
Fix Version/s: 0.12.0
Status: Patch Available (was: Open)
+1 Looks good to me.
> Optimization: Reduce Overhead from ReflectionUtils.newInstance
> --------------------------------------------------------------
>
> Key: HADOOP-1017
> URL: https://issues.apache.org/jira/browse/HADOOP-1017
> Project: Hadoop
> Issue Type: Improvement
> Components: util
> Reporter: Ron Bodkin
> Fix For: 0.12.0
>
> Attachments: cacheCtor.patch, cacheCtor.patch, ReflectionUtils.patch.txt, TestReflectionUtils.java
>
>
> I found that a significant amount of time on my project was being spent in creating constructors for each row of data. I dramatically optimized this performance by creating a simple WeakHashMap to cache constructors by class. For example, in a sample job I find that ReflectionUtils.newInstance takes 200 ms (2% of total) with the cache enabled, but it uses 900 ms (6% of total) without the cache.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-1017) Optimization: Reduce Overhead from
ReflectionUtils.newInstance
Posted by "Doug Cutting (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-1017?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12472919 ]
Doug Cutting commented on HADOOP-1017:
--------------------------------------
Package-private for access from unit tests sounds good. Yes, please submit another patch. The unit test can be included in the patch by using 'svn add <file>' before you use 'svn diff > my.patch' to create the patch, as described on http://wiki.apache.org/lucene-hadoop/HowToContribute. Thanks!
> Optimization: Reduce Overhead from ReflectionUtils.newInstance
> --------------------------------------------------------------
>
> Key: HADOOP-1017
> URL: https://issues.apache.org/jira/browse/HADOOP-1017
> Project: Hadoop
> Issue Type: Improvement
> Components: util
> Reporter: Ron Bodkin
> Attachments: ReflectionUtils.patch.txt, TestReflectionUtils.java
>
>
> I found that a significant amount of time on my project was being spent in creating constructors for each row of data. I dramatically optimized this performance by creating a simple WeakHashMap to cache constructors by class. For example, in a sample job I find that ReflectionUtils.newInstance takes 200 ms (2% of total) with the cache enabled, but it uses 900 ms (6% of total) without the cache.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HADOOP-1017) Optimization: Reduce Overhead from
ReflectionUtils.newInstance
Posted by "Ron Bodkin (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-1017?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Ron Bodkin updated HADOOP-1017:
-------------------------------
Attachment: cacheCtor.patch
Here is the revised patch that uses a concurrent map for thread safety and holds hard references instead.
> Optimization: Reduce Overhead from ReflectionUtils.newInstance
> --------------------------------------------------------------
>
> Key: HADOOP-1017
> URL: https://issues.apache.org/jira/browse/HADOOP-1017
> Project: Hadoop
> Issue Type: Improvement
> Components: util
> Reporter: Ron Bodkin
> Attachments: cacheCtor.patch, cacheCtor.patch, ReflectionUtils.patch.txt, TestReflectionUtils.java
>
>
> I found that a significant amount of time on my project was being spent in creating constructors for each row of data. I dramatically optimized this performance by creating a simple WeakHashMap to cache constructors by class. For example, in a sample job I find that ReflectionUtils.newInstance takes 200 ms (2% of total) with the cache enabled, but it uses 900 ms (6% of total) without the cache.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-1017) Optimization: Reduce Overhead from
ReflectionUtils.newInstance
Posted by "Doug Cutting (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-1017?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12473791 ]
Doug Cutting commented on HADOOP-1017:
--------------------------------------
A few more thoughts:
- should constructorCache be 'static final', and hence named CACHE, all-caps?
- the values of the WeakHashMap are of type Constructor, each which references its Class, the key. According to http://java.sun.com/j2se/1.5.0/docs/api/java/util/WeakHashMap.html, if values reference the keys, entries will never by collected. So we should either just use a HashMap, and not permit these to be collected, which is probably fine, or change the values to be WeakReferences to the constructors.
> Optimization: Reduce Overhead from ReflectionUtils.newInstance
> --------------------------------------------------------------
>
> Key: HADOOP-1017
> URL: https://issues.apache.org/jira/browse/HADOOP-1017
> Project: Hadoop
> Issue Type: Improvement
> Components: util
> Reporter: Ron Bodkin
> Attachments: cacheCtor.patch, ReflectionUtils.patch.txt, TestReflectionUtils.java
>
>
> I found that a significant amount of time on my project was being spent in creating constructors for each row of data. I dramatically optimized this performance by creating a simple WeakHashMap to cache constructors by class. For example, in a sample job I find that ReflectionUtils.newInstance takes 200 ms (2% of total) with the cache enabled, but it uses 900 ms (6% of total) without the cache.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-1017) Optimization: Reduce Overhead from
ReflectionUtils.newInstance
Posted by "Doug Cutting (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-1017?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12473804 ]
Doug Cutting commented on HADOOP-1017:
--------------------------------------
Hadoop requires Java5 already, so, yes, let's use ConcurrentMap for thread-safety and not worry about GCing classes.
> Optimization: Reduce Overhead from ReflectionUtils.newInstance
> --------------------------------------------------------------
>
> Key: HADOOP-1017
> URL: https://issues.apache.org/jira/browse/HADOOP-1017
> Project: Hadoop
> Issue Type: Improvement
> Components: util
> Reporter: Ron Bodkin
> Attachments: cacheCtor.patch, ReflectionUtils.patch.txt, TestReflectionUtils.java
>
>
> I found that a significant amount of time on my project was being spent in creating constructors for each row of data. I dramatically optimized this performance by creating a simple WeakHashMap to cache constructors by class. For example, in a sample job I find that ReflectionUtils.newInstance takes 200 ms (2% of total) with the cache enabled, but it uses 900 ms (6% of total) without the cache.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-1017) Optimization: Reduce Overhead from
ReflectionUtils.newInstance
Posted by "Doug Cutting (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-1017?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12472895 ]
Doug Cutting commented on HADOOP-1017:
--------------------------------------
Overall, I like this. In most cases, where classes are not GC'd, the use of a WeakHashMap is moot, but it shouldn't hurt and may help in some cases. But I'd prefer we don't expose the cache through a public API, nor permit disabling it through a system property. A larger public API complicates long-term support. If the cache size ever becomes an issue then we can address that then.
> Optimization: Reduce Overhead from ReflectionUtils.newInstance
> --------------------------------------------------------------
>
> Key: HADOOP-1017
> URL: https://issues.apache.org/jira/browse/HADOOP-1017
> Project: Hadoop
> Issue Type: Improvement
> Components: util
> Reporter: Ron Bodkin
> Attachments: ReflectionUtils.patch.txt, TestReflectionUtils.java
>
>
> I found that a significant amount of time on my project was being spent in creating constructors for each row of data. I dramatically optimized this performance by creating a simple WeakHashMap to cache constructors by class. For example, in a sample job I find that ReflectionUtils.newInstance takes 200 ms (2% of total) with the cache enabled, but it uses 900 ms (6% of total) without the cache.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HADOOP-1017) Optimization: Reduce Overhead from
ReflectionUtils.newInstance
Posted by "Ron Bodkin (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-1017?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Ron Bodkin updated HADOOP-1017:
-------------------------------
Attachment: TestReflectionUtils.java
New file: tests for reflection utils.
> Optimization: Reduce Overhead from ReflectionUtils.newInstance
> --------------------------------------------------------------
>
> Key: HADOOP-1017
> URL: https://issues.apache.org/jira/browse/HADOOP-1017
> Project: Hadoop
> Issue Type: Improvement
> Components: util
> Reporter: Ron Bodkin
> Attachments: ReflectionUtils.patch.txt, TestReflectionUtils.java
>
>
> I found that a significant amount of time on my project was being spent in creating constructors for each row of data. I dramatically optimized this performance by creating a simple WeakHashMap to cache constructors by class. For example, in a sample job I find that ReflectionUtils.newInstance takes 200 ms (2% of total) with the cache enabled, but it uses 900 ms (6% of total) without the cache.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-1017) Optimization: Reduce Overhead from
ReflectionUtils.newInstance
Posted by "Ron Bodkin (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-1017?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12472918 ]
Ron Bodkin commented on HADOOP-1017:
------------------------------------
I'd be glad to remove the public API: I certainly appreciate the desire to avoid that complexity, or let me know if you'd rather do so. If I do so, I'll make the additional methods package default access to allow the tests.
> Optimization: Reduce Overhead from ReflectionUtils.newInstance
> --------------------------------------------------------------
>
> Key: HADOOP-1017
> URL: https://issues.apache.org/jira/browse/HADOOP-1017
> Project: Hadoop
> Issue Type: Improvement
> Components: util
> Reporter: Ron Bodkin
> Attachments: ReflectionUtils.patch.txt, TestReflectionUtils.java
>
>
> I found that a significant amount of time on my project was being spent in creating constructors for each row of data. I dramatically optimized this performance by creating a simple WeakHashMap to cache constructors by class. For example, in a sample job I find that ReflectionUtils.newInstance takes 200 ms (2% of total) with the cache enabled, but it uses 900 ms (6% of total) without the cache.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HADOOP-1017) Optimization: Reduce Overhead from
ReflectionUtils.newInstance
Posted by "Ron Bodkin (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-1017?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Ron Bodkin updated HADOOP-1017:
-------------------------------
Attachment: ReflectionUtils.patch.txt
Patch to ReflectionUtils to cache constructors.
> Optimization: Reduce Overhead from ReflectionUtils.newInstance
> --------------------------------------------------------------
>
> Key: HADOOP-1017
> URL: https://issues.apache.org/jira/browse/HADOOP-1017
> Project: Hadoop
> Issue Type: Improvement
> Components: util
> Reporter: Ron Bodkin
> Attachments: ReflectionUtils.patch.txt
>
>
> I found that a significant amount of time on my project was being spent in creating constructors for each row of data. I dramatically optimized this performance by creating a simple WeakHashMap to cache constructors by class. For example, in a sample job I find that ReflectionUtils.newInstance takes 200 ms (2% of total) with the cache enabled, but it uses 900 ms (6% of total) without the cache.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-1017) Optimization: Reduce Overhead from
ReflectionUtils.newInstance
Posted by "Doug Cutting (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-1017?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12473790 ]
Doug Cutting commented on HADOOP-1017:
--------------------------------------
I just realized that this cache is not thread-safe: access to the cache should be synchronized. I can see a few ways of doing this:
a. wrap 'synchronized (constructorCache) { ... }' around the newInstance method body
b. switch to using a synchronized map (Collections.synchronizedMap)
c. keep a separate constructorCache per thread, via a threadLocal
(a) is the simplest, but could result in more contention, since the cache stays locked while constructors are created. (b) would unlock the cache while constructors are created, but might sometimes create a given constructor twice. (c) would be fastest, but would always create a new constructor per thread. I'd probably opt for (b). What do others think?
> Optimization: Reduce Overhead from ReflectionUtils.newInstance
> --------------------------------------------------------------
>
> Key: HADOOP-1017
> URL: https://issues.apache.org/jira/browse/HADOOP-1017
> Project: Hadoop
> Issue Type: Improvement
> Components: util
> Reporter: Ron Bodkin
> Attachments: cacheCtor.patch, ReflectionUtils.patch.txt, TestReflectionUtils.java
>
>
> I found that a significant amount of time on my project was being spent in creating constructors for each row of data. I dramatically optimized this performance by creating a simple WeakHashMap to cache constructors by class. For example, in a sample job I find that ReflectionUtils.newInstance takes 200 ms (2% of total) with the cache enabled, but it uses 900 ms (6% of total) without the cache.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-1017) Optimization: Reduce Overhead from
ReflectionUtils.newInstance
Posted by "Ron Bodkin (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-1017?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12473799 ]
Ron Bodkin commented on HADOOP-1017:
------------------------------------
See for example the methods makeConcurrentMap and makeConcurrentMapClass, which I wrote and contributed, at http://dev.eclipse.org/viewcvs/index.cgi/org.aspectj/modules/weaver/src/org/aspectj/weaver/ltw/LTWWorld.java?root=Tools_Project&view=markup
> Optimization: Reduce Overhead from ReflectionUtils.newInstance
> --------------------------------------------------------------
>
> Key: HADOOP-1017
> URL: https://issues.apache.org/jira/browse/HADOOP-1017
> Project: Hadoop
> Issue Type: Improvement
> Components: util
> Reporter: Ron Bodkin
> Attachments: cacheCtor.patch, ReflectionUtils.patch.txt, TestReflectionUtils.java
>
>
> I found that a significant amount of time on my project was being spent in creating constructors for each row of data. I dramatically optimized this performance by creating a simple WeakHashMap to cache constructors by class. For example, in a sample job I find that ReflectionUtils.newInstance takes 200 ms (2% of total) with the cache enabled, but it uses 900 ms (6% of total) without the cache.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.