You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@openjpa.apache.org by "Patrick Linskey (JIRA)" <ji...@apache.org> on 2007/04/17 22:32:15 UTC

[jira] Created: (OPENJPA-219) Reflection: negative caching would be beneficial in redeployment scenarios

Reflection: negative caching would be beneficial in redeployment scenarios
--------------------------------------------------------------------------

                 Key: OPENJPA-219
                 URL: https://issues.apache.org/jira/browse/OPENJPA-219
             Project: OpenJPA
          Issue Type: Bug
          Components: kernel
    Affects Versions: 0.9.6, 0.9.0, 0.9.7
            Reporter: Patrick Linskey
             Fix For: 0.9.8


In a variety of situations, OpenJPA searches class hierarchies for fields. These searches invoke Class.getDeclaredField() in order to find non-public fields. This method throws an exception when it fails to find the specified field, and the exception creation is, as ever, slow.

It would be useful to create a static (and thus per-classloader) Map<WeakReference<Class>,Collection<String>> of fields known not to be available in a given class.

It may also be beneficial to maintain a cache of which fields *are* present in a given class, but this issue is being filed as a result of a demonstrated performance hit during deployment due to the lack of a negative cache. If we obtain quantitative data indicating that a positive cache would be useful, we might want to implement such a thing at that time.

Trace 3 (2115 occurances): [excepti][00090] java/lang/NoSuchFieldException: domainName
     at java/lang/Class.getDeclaredField(Ljava/lang/String;I)Ljava/lang/reflect/Field;(Unknown Source)
     at org/apache/openjpa/enhance/Reflection.findField(Ljava/lang/Class;Ljava/lang/String;Z)Ljava/lang/reflect/Field;(Reflection.java:101)
     at org/apache/openjpa/util/ApplicationIds.toPKValues(Ljava/lang/Object;Lorg/apache/openjpa/meta/ClassMetaData;)[Ljava/lang/Object;
(ApplicationIds.java:89)



-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (OPENJPA-219) Reflection: negative caching would be beneficial in redeployment scenarios

Posted by "Patrick Linskey (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/OPENJPA-219?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12491458 ] 

Patrick Linskey commented on OPENJPA-219:
-----------------------------------------

How does this compare to the leaky-patch version?

> Reflection: negative caching would be beneficial in redeployment scenarios
> --------------------------------------------------------------------------
>
>                 Key: OPENJPA-219
>                 URL: https://issues.apache.org/jira/browse/OPENJPA-219
>             Project: OpenJPA
>          Issue Type: Bug
>          Components: kernel
>    Affects Versions: 0.9.0, 0.9.6, 0.9.7
>            Reporter: Patrick Linskey
>             Fix For: 0.9.8
>
>         Attachments: OPENJPA-219-NoLeak.patch, OPENJPA-219.patch
>
>
> In a variety of situations, OpenJPA searches class hierarchies for fields. These searches invoke Class.getDeclaredField() in order to find non-public fields. This method throws an exception when it fails to find the specified field, and the exception creation is, as ever, slow.
> It would be useful to create a static (and thus per-classloader) Map<WeakReference<Class>,Collection<String>> of fields known not to be available in a given class.
> It may also be beneficial to maintain a cache of which fields *are* present in a given class, but this issue is being filed as a result of a demonstrated performance hit during deployment due to the lack of a negative cache. If we obtain quantitative data indicating that a positive cache would be useful, we might want to implement such a thing at that time.
> Trace 3 (2115 occurances): [excepti][00090] java/lang/NoSuchFieldException: domainName
>      at java/lang/Class.getDeclaredField(Ljava/lang/String;I)Ljava/lang/reflect/Field;(Unknown Source)
>      at org/apache/openjpa/enhance/Reflection.findField(Ljava/lang/Class;Ljava/lang/String;Z)Ljava/lang/reflect/Field;(Reflection.java:101)
>      at org/apache/openjpa/util/ApplicationIds.toPKValues(Ljava/lang/Object;Lorg/apache/openjpa/meta/ClassMetaData;)[Ljava/lang/Object;
> (ApplicationIds.java:89)

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (OPENJPA-219) Reflection: negative caching would be beneficial in redeployment scenarios

Posted by "Bret Weinraub (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/OPENJPA-219?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12491077 ] 

Bret Weinraub commented on OPENJPA-219:
---------------------------------------

I developed a patch version that cached both "success" and "failure" cases - here are some of the performance numbers I generated:

VM	Implementation	Benchmark Rate (ms)
Sun	1 - Original	333
Sun	2 - Cache v1	83
Sun	3 - Cache v2	40
JRockit	1 - Original	176
JRockit	2 - Cache v1	72
JRockit	3 - Cache v2	22

Where 

Original : recent svn version
Cache V1: patch above plus a couple of minor tweaks
Cache V2: caches both success and failure calls thereby minimizing calls to Reflection API.

Adding a boolean that says "use cache" or not would be simple.

Anyway let me add that I/we feel that permanently caching Class objects would create memory leak problems in the case where a ClassLoader were to be thrown away.  Holding the class Object in a hash will keep the ClassLoader tree from being garbage collected ; we see this anti-pattern frequently, it is pathalogical, and can eventually lead to OOME for operations that recycle class loaders.  An example would be repeatedly deploying the same application to an application server, such as done during iterative development of WebApps/EnterpriseApps.  So I/we believe to be critical to create a mechanism whereby these Class object were not held in a static context.  If this code were associated with an object then when the object was GC-ed the Class references would be eliminated therey by allowing the class loader to be GC-ed.



> Reflection: negative caching would be beneficial in redeployment scenarios
> --------------------------------------------------------------------------
>
>                 Key: OPENJPA-219
>                 URL: https://issues.apache.org/jira/browse/OPENJPA-219
>             Project: OpenJPA
>          Issue Type: Bug
>          Components: kernel
>    Affects Versions: 0.9.0, 0.9.6, 0.9.7
>            Reporter: Patrick Linskey
>             Fix For: 0.9.8
>
>         Attachments: OPENJPA-219.patch
>
>
> In a variety of situations, OpenJPA searches class hierarchies for fields. These searches invoke Class.getDeclaredField() in order to find non-public fields. This method throws an exception when it fails to find the specified field, and the exception creation is, as ever, slow.
> It would be useful to create a static (and thus per-classloader) Map<WeakReference<Class>,Collection<String>> of fields known not to be available in a given class.
> It may also be beneficial to maintain a cache of which fields *are* present in a given class, but this issue is being filed as a result of a demonstrated performance hit during deployment due to the lack of a negative cache. If we obtain quantitative data indicating that a positive cache would be useful, we might want to implement such a thing at that time.
> Trace 3 (2115 occurances): [excepti][00090] java/lang/NoSuchFieldException: domainName
>      at java/lang/Class.getDeclaredField(Ljava/lang/String;I)Ljava/lang/reflect/Field;(Unknown Source)
>      at org/apache/openjpa/enhance/Reflection.findField(Ljava/lang/Class;Ljava/lang/String;Z)Ljava/lang/reflect/Field;(Reflection.java:101)
>      at org/apache/openjpa/util/ApplicationIds.toPKValues(Ljava/lang/Object;Lorg/apache/openjpa/meta/ClassMetaData;)[Ljava/lang/Object;
> (ApplicationIds.java:89)

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (OPENJPA-219) Reflection: negative caching would be beneficial in redeployment scenarios

Posted by "Patrick Linskey (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/OPENJPA-219?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12491316 ] 

Patrick Linskey commented on OPENJPA-219:
-----------------------------------------

> another way to approach this would be not to cache at all,
> and just iterate over getDeclaredFields/Methods() looking for a match

Good point. That would certainly be a better approach. It looks like Class.getDeclaredFields() will be a bit slower, since more memory copying happens (the array is copied before being returned -- bummer that those APIs don't use collections), but that might not be noticeable in the benchmark.

> Reflection: negative caching would be beneficial in redeployment scenarios
> --------------------------------------------------------------------------
>
>                 Key: OPENJPA-219
>                 URL: https://issues.apache.org/jira/browse/OPENJPA-219
>             Project: OpenJPA
>          Issue Type: Bug
>          Components: kernel
>    Affects Versions: 0.9.0, 0.9.6, 0.9.7
>            Reporter: Patrick Linskey
>             Fix For: 0.9.8
>
>         Attachments: OPENJPA-219.patch
>
>
> In a variety of situations, OpenJPA searches class hierarchies for fields. These searches invoke Class.getDeclaredField() in order to find non-public fields. This method throws an exception when it fails to find the specified field, and the exception creation is, as ever, slow.
> It would be useful to create a static (and thus per-classloader) Map<WeakReference<Class>,Collection<String>> of fields known not to be available in a given class.
> It may also be beneficial to maintain a cache of which fields *are* present in a given class, but this issue is being filed as a result of a demonstrated performance hit during deployment due to the lack of a negative cache. If we obtain quantitative data indicating that a positive cache would be useful, we might want to implement such a thing at that time.
> Trace 3 (2115 occurances): [excepti][00090] java/lang/NoSuchFieldException: domainName
>      at java/lang/Class.getDeclaredField(Ljava/lang/String;I)Ljava/lang/reflect/Field;(Unknown Source)
>      at org/apache/openjpa/enhance/Reflection.findField(Ljava/lang/Class;Ljava/lang/String;Z)Ljava/lang/reflect/Field;(Reflection.java:101)
>      at org/apache/openjpa/util/ApplicationIds.toPKValues(Ljava/lang/Object;Lorg/apache/openjpa/meta/ClassMetaData;)[Ljava/lang/Object;
> (ApplicationIds.java:89)

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (OPENJPA-219) Reflection: negative caching would be beneficial in redeployment scenarios

Posted by "Patrick Linskey (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/OPENJPA-219?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Patrick Linskey updated OPENJPA-219:
------------------------------------

    Attachment:     (was: OPENJPA-219.patch)

> Reflection: negative caching would be beneficial in redeployment scenarios
> --------------------------------------------------------------------------
>
>                 Key: OPENJPA-219
>                 URL: https://issues.apache.org/jira/browse/OPENJPA-219
>             Project: OpenJPA
>          Issue Type: Bug
>          Components: kernel
>    Affects Versions: 0.9.0, 0.9.6, 0.9.7
>            Reporter: Patrick Linskey
>             Fix For: 0.9.8
>
>
> In a variety of situations, OpenJPA searches class hierarchies for fields. These searches invoke Class.getDeclaredField() in order to find non-public fields. This method throws an exception when it fails to find the specified field, and the exception creation is, as ever, slow.
> It would be useful to create a static (and thus per-classloader) Map<WeakReference<Class>,Collection<String>> of fields known not to be available in a given class.
> It may also be beneficial to maintain a cache of which fields *are* present in a given class, but this issue is being filed as a result of a demonstrated performance hit during deployment due to the lack of a negative cache. If we obtain quantitative data indicating that a positive cache would be useful, we might want to implement such a thing at that time.
> Trace 3 (2115 occurances): [excepti][00090] java/lang/NoSuchFieldException: domainName
>      at java/lang/Class.getDeclaredField(Ljava/lang/String;I)Ljava/lang/reflect/Field;(Unknown Source)
>      at org/apache/openjpa/enhance/Reflection.findField(Ljava/lang/Class;Ljava/lang/String;Z)Ljava/lang/reflect/Field;(Reflection.java:101)
>      at org/apache/openjpa/util/ApplicationIds.toPKValues(Ljava/lang/Object;Lorg/apache/openjpa/meta/ClassMetaData;)[Ljava/lang/Object;
> (ApplicationIds.java:89)

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (OPENJPA-219) Reflection: negative caching would be beneficial in redeployment scenarios

Posted by "Patrick Linskey (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/OPENJPA-219?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12489536 ] 

Patrick Linskey commented on OPENJPA-219:
-----------------------------------------

Note that we invoke Class.getDeclaredField() (and Class.getDeclaredMethod()) in a number of places. We should take care to replace all the uses of that method with our optimized version.

> Reflection: negative caching would be beneficial in redeployment scenarios
> --------------------------------------------------------------------------
>
>                 Key: OPENJPA-219
>                 URL: https://issues.apache.org/jira/browse/OPENJPA-219
>             Project: OpenJPA
>          Issue Type: Bug
>          Components: kernel
>    Affects Versions: 0.9.0, 0.9.6, 0.9.7
>            Reporter: Patrick Linskey
>             Fix For: 0.9.8
>
>
> In a variety of situations, OpenJPA searches class hierarchies for fields. These searches invoke Class.getDeclaredField() in order to find non-public fields. This method throws an exception when it fails to find the specified field, and the exception creation is, as ever, slow.
> It would be useful to create a static (and thus per-classloader) Map<WeakReference<Class>,Collection<String>> of fields known not to be available in a given class.
> It may also be beneficial to maintain a cache of which fields *are* present in a given class, but this issue is being filed as a result of a demonstrated performance hit during deployment due to the lack of a negative cache. If we obtain quantitative data indicating that a positive cache would be useful, we might want to implement such a thing at that time.
> Trace 3 (2115 occurances): [excepti][00090] java/lang/NoSuchFieldException: domainName
>      at java/lang/Class.getDeclaredField(Ljava/lang/String;I)Ljava/lang/reflect/Field;(Unknown Source)
>      at org/apache/openjpa/enhance/Reflection.findField(Ljava/lang/Class;Ljava/lang/String;Z)Ljava/lang/reflect/Field;(Reflection.java:101)
>      at org/apache/openjpa/util/ApplicationIds.toPKValues(Ljava/lang/Object;Lorg/apache/openjpa/meta/ClassMetaData;)[Ljava/lang/Object;
> (ApplicationIds.java:89)

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (OPENJPA-219) Reflection: negative caching would be beneficial in redeployment scenarios

Posted by "Bret Weinraub (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/OPENJPA-219?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12491078 ] 

Bret Weinraub commented on OPENJPA-219:
---------------------------------------

I will add the "Cache v2" version here shortly.

> Reflection: negative caching would be beneficial in redeployment scenarios
> --------------------------------------------------------------------------
>
>                 Key: OPENJPA-219
>                 URL: https://issues.apache.org/jira/browse/OPENJPA-219
>             Project: OpenJPA
>          Issue Type: Bug
>          Components: kernel
>    Affects Versions: 0.9.0, 0.9.6, 0.9.7
>            Reporter: Patrick Linskey
>             Fix For: 0.9.8
>
>         Attachments: OPENJPA-219.patch
>
>
> In a variety of situations, OpenJPA searches class hierarchies for fields. These searches invoke Class.getDeclaredField() in order to find non-public fields. This method throws an exception when it fails to find the specified field, and the exception creation is, as ever, slow.
> It would be useful to create a static (and thus per-classloader) Map<WeakReference<Class>,Collection<String>> of fields known not to be available in a given class.
> It may also be beneficial to maintain a cache of which fields *are* present in a given class, but this issue is being filed as a result of a demonstrated performance hit during deployment due to the lack of a negative cache. If we obtain quantitative data indicating that a positive cache would be useful, we might want to implement such a thing at that time.
> Trace 3 (2115 occurances): [excepti][00090] java/lang/NoSuchFieldException: domainName
>      at java/lang/Class.getDeclaredField(Ljava/lang/String;I)Ljava/lang/reflect/Field;(Unknown Source)
>      at org/apache/openjpa/enhance/Reflection.findField(Ljava/lang/Class;Ljava/lang/String;Z)Ljava/lang/reflect/Field;(Reflection.java:101)
>      at org/apache/openjpa/util/ApplicationIds.toPKValues(Ljava/lang/Object;Lorg/apache/openjpa/meta/ClassMetaData;)[Ljava/lang/Object;
> (ApplicationIds.java:89)

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (OPENJPA-219) Reflection: negative caching would be beneficial in redeployment scenarios

Posted by "Patrick Linskey (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/OPENJPA-219?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12491104 ] 

Patrick Linskey commented on OPENJPA-219:
-----------------------------------------

I agree about the mem leak problem in the patch that I created; hence my comments in the original post about using weak references. There are a number of ways that you could solve this caching problem, such as making all / part of Reflection non-static and setting up per-Configuration caches, or using weak references in the cache, or even (and probably least attractively) creating undeploy APIs.

Abe said: 
> If we're going to cache, I don't see why we wouldn't cache the declared 
> fields/methods rather than the nonexistent ones. It would be a simple
> matter of keeping a Class->Set cache, where the Set is the names of 
> the fields/methods returned by Class.getDeclaredFields/Methods(). 
> Then we'd have both a positive (set.contains(x)) and negative 
> (!set.contains(x)) cache.

As I mentioned in the original description, the bug here is that negative lookups are slow because of the exception creation. I see no reason to add extra caching if we don't know if we need it, although Bret's numbers potentially indicate that there is a benefit to positive caching. I have no problem with having a positive cache, but I think that we should only include it if it's going to help, since otherwise it'll just be one more thing contributing to memory consumption.

> Reflection: negative caching would be beneficial in redeployment scenarios
> --------------------------------------------------------------------------
>
>                 Key: OPENJPA-219
>                 URL: https://issues.apache.org/jira/browse/OPENJPA-219
>             Project: OpenJPA
>          Issue Type: Bug
>          Components: kernel
>    Affects Versions: 0.9.0, 0.9.6, 0.9.7
>            Reporter: Patrick Linskey
>             Fix For: 0.9.8
>
>         Attachments: OPENJPA-219.patch
>
>
> In a variety of situations, OpenJPA searches class hierarchies for fields. These searches invoke Class.getDeclaredField() in order to find non-public fields. This method throws an exception when it fails to find the specified field, and the exception creation is, as ever, slow.
> It would be useful to create a static (and thus per-classloader) Map<WeakReference<Class>,Collection<String>> of fields known not to be available in a given class.
> It may also be beneficial to maintain a cache of which fields *are* present in a given class, but this issue is being filed as a result of a demonstrated performance hit during deployment due to the lack of a negative cache. If we obtain quantitative data indicating that a positive cache would be useful, we might want to implement such a thing at that time.
> Trace 3 (2115 occurances): [excepti][00090] java/lang/NoSuchFieldException: domainName
>      at java/lang/Class.getDeclaredField(Ljava/lang/String;I)Ljava/lang/reflect/Field;(Unknown Source)
>      at org/apache/openjpa/enhance/Reflection.findField(Ljava/lang/Class;Ljava/lang/String;Z)Ljava/lang/reflect/Field;(Reflection.java:101)
>      at org/apache/openjpa/util/ApplicationIds.toPKValues(Ljava/lang/Object;Lorg/apache/openjpa/meta/ClassMetaData;)[Ljava/lang/Object;
> (ApplicationIds.java:89)

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Resolved: (OPENJPA-219) Reflection: negative caching would be beneficial in redeployment scenarios

Posted by "Patrick Linskey (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/OPENJPA-219?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Patrick Linskey resolved OPENJPA-219.
-------------------------------------

    Resolution: Fixed

> Reflection: negative caching would be beneficial in redeployment scenarios
> --------------------------------------------------------------------------
>
>                 Key: OPENJPA-219
>                 URL: https://issues.apache.org/jira/browse/OPENJPA-219
>             Project: OpenJPA
>          Issue Type: Bug
>          Components: kernel
>    Affects Versions: 0.9.0, 0.9.6, 0.9.7
>            Reporter: Patrick Linskey
>         Assigned To: Patrick Linskey
>             Fix For: 0.9.8
>
>         Attachments: OPENJPA-219-NoLeak.patch, OPENJPA-219.patch, openJPABenchmark.tar.gz
>
>
> In a variety of situations, OpenJPA searches class hierarchies for fields. These searches invoke Class.getDeclaredField() in order to find non-public fields. This method throws an exception when it fails to find the specified field, and the exception creation is, as ever, slow.
> It would be useful to create a static (and thus per-classloader) Map<WeakReference<Class>,Collection<String>> of fields known not to be available in a given class.
> It may also be beneficial to maintain a cache of which fields *are* present in a given class, but this issue is being filed as a result of a demonstrated performance hit during deployment due to the lack of a negative cache. If we obtain quantitative data indicating that a positive cache would be useful, we might want to implement such a thing at that time.
> Trace 3 (2115 occurances): [excepti][00090] java/lang/NoSuchFieldException: domainName
>      at java/lang/Class.getDeclaredField(Ljava/lang/String;I)Ljava/lang/reflect/Field;(Unknown Source)
>      at org/apache/openjpa/enhance/Reflection.findField(Ljava/lang/Class;Ljava/lang/String;Z)Ljava/lang/reflect/Field;(Reflection.java:101)
>      at org/apache/openjpa/util/ApplicationIds.toPKValues(Ljava/lang/Object;Lorg/apache/openjpa/meta/ClassMetaData;)[Ljava/lang/Object;
> (ApplicationIds.java:89)

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (OPENJPA-219) Reflection: negative caching would be beneficial in redeployment scenarios

Posted by "Patrick Linskey (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/OPENJPA-219?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Patrick Linskey updated OPENJPA-219:
------------------------------------

    Attachment: OPENJPA-219.patch

I think that this patch should fix the issue, but haven't tested it yet. I only replaced the getDeclaredField() cases that were iterating through superclasses.

> Reflection: negative caching would be beneficial in redeployment scenarios
> --------------------------------------------------------------------------
>
>                 Key: OPENJPA-219
>                 URL: https://issues.apache.org/jira/browse/OPENJPA-219
>             Project: OpenJPA
>          Issue Type: Bug
>          Components: kernel
>    Affects Versions: 0.9.0, 0.9.6, 0.9.7
>            Reporter: Patrick Linskey
>             Fix For: 0.9.8
>
>         Attachments: OPENJPA-219.patch
>
>
> In a variety of situations, OpenJPA searches class hierarchies for fields. These searches invoke Class.getDeclaredField() in order to find non-public fields. This method throws an exception when it fails to find the specified field, and the exception creation is, as ever, slow.
> It would be useful to create a static (and thus per-classloader) Map<WeakReference<Class>,Collection<String>> of fields known not to be available in a given class.
> It may also be beneficial to maintain a cache of which fields *are* present in a given class, but this issue is being filed as a result of a demonstrated performance hit during deployment due to the lack of a negative cache. If we obtain quantitative data indicating that a positive cache would be useful, we might want to implement such a thing at that time.
> Trace 3 (2115 occurances): [excepti][00090] java/lang/NoSuchFieldException: domainName
>      at java/lang/Class.getDeclaredField(Ljava/lang/String;I)Ljava/lang/reflect/Field;(Unknown Source)
>      at org/apache/openjpa/enhance/Reflection.findField(Ljava/lang/Class;Ljava/lang/String;Z)Ljava/lang/reflect/Field;(Reflection.java:101)
>      at org/apache/openjpa/util/ApplicationIds.toPKValues(Ljava/lang/Object;Lorg/apache/openjpa/meta/ClassMetaData;)[Ljava/lang/Object;
> (ApplicationIds.java:89)

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Assigned: (OPENJPA-219) Reflection: negative caching would be beneficial in redeployment scenarios

Posted by "Patrick Linskey (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/OPENJPA-219?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Patrick Linskey reassigned OPENJPA-219:
---------------------------------------

    Assignee: Patrick Linskey

> Reflection: negative caching would be beneficial in redeployment scenarios
> --------------------------------------------------------------------------
>
>                 Key: OPENJPA-219
>                 URL: https://issues.apache.org/jira/browse/OPENJPA-219
>             Project: OpenJPA
>          Issue Type: Bug
>          Components: kernel
>    Affects Versions: 0.9.0, 0.9.6, 0.9.7
>            Reporter: Patrick Linskey
>         Assigned To: Patrick Linskey
>             Fix For: 0.9.8
>
>         Attachments: OPENJPA-219-NoLeak.patch, OPENJPA-219.patch
>
>
> In a variety of situations, OpenJPA searches class hierarchies for fields. These searches invoke Class.getDeclaredField() in order to find non-public fields. This method throws an exception when it fails to find the specified field, and the exception creation is, as ever, slow.
> It would be useful to create a static (and thus per-classloader) Map<WeakReference<Class>,Collection<String>> of fields known not to be available in a given class.
> It may also be beneficial to maintain a cache of which fields *are* present in a given class, but this issue is being filed as a result of a demonstrated performance hit during deployment due to the lack of a negative cache. If we obtain quantitative data indicating that a positive cache would be useful, we might want to implement such a thing at that time.
> Trace 3 (2115 occurances): [excepti][00090] java/lang/NoSuchFieldException: domainName
>      at java/lang/Class.getDeclaredField(Ljava/lang/String;I)Ljava/lang/reflect/Field;(Unknown Source)
>      at org/apache/openjpa/enhance/Reflection.findField(Ljava/lang/Class;Ljava/lang/String;Z)Ljava/lang/reflect/Field;(Reflection.java:101)
>      at org/apache/openjpa/util/ApplicationIds.toPKValues(Ljava/lang/Object;Lorg/apache/openjpa/meta/ClassMetaData;)[Ljava/lang/Object;
> (ApplicationIds.java:89)

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (OPENJPA-219) Reflection: negative caching would be beneficial in redeployment scenarios

Posted by "Bret Weinraub (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/OPENJPA-219?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Bret Weinraub updated OPENJPA-219:
----------------------------------

    Attachment: openJPABenchmark.tar.gz

My benchmark code.

Easy to write a new implementation and get comparitive benchmark data.

Measurements created in this issue were generated from this code.


> Reflection: negative caching would be beneficial in redeployment scenarios
> --------------------------------------------------------------------------
>
>                 Key: OPENJPA-219
>                 URL: https://issues.apache.org/jira/browse/OPENJPA-219
>             Project: OpenJPA
>          Issue Type: Bug
>          Components: kernel
>    Affects Versions: 0.9.0, 0.9.6, 0.9.7
>            Reporter: Patrick Linskey
>         Assigned To: Patrick Linskey
>             Fix For: 0.9.8
>
>         Attachments: OPENJPA-219-NoLeak.patch, OPENJPA-219.patch, openJPABenchmark.tar.gz
>
>
> In a variety of situations, OpenJPA searches class hierarchies for fields. These searches invoke Class.getDeclaredField() in order to find non-public fields. This method throws an exception when it fails to find the specified field, and the exception creation is, as ever, slow.
> It would be useful to create a static (and thus per-classloader) Map<WeakReference<Class>,Collection<String>> of fields known not to be available in a given class.
> It may also be beneficial to maintain a cache of which fields *are* present in a given class, but this issue is being filed as a result of a demonstrated performance hit during deployment due to the lack of a negative cache. If we obtain quantitative data indicating that a positive cache would be useful, we might want to implement such a thing at that time.
> Trace 3 (2115 occurances): [excepti][00090] java/lang/NoSuchFieldException: domainName
>      at java/lang/Class.getDeclaredField(Ljava/lang/String;I)Ljava/lang/reflect/Field;(Unknown Source)
>      at org/apache/openjpa/enhance/Reflection.findField(Ljava/lang/Class;Ljava/lang/String;Z)Ljava/lang/reflect/Field;(Reflection.java:101)
>      at org/apache/openjpa/util/ApplicationIds.toPKValues(Ljava/lang/Object;Lorg/apache/openjpa/meta/ClassMetaData;)[Ljava/lang/Object;
> (ApplicationIds.java:89)

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (OPENJPA-219) Reflection: negative caching would be beneficial in redeployment scenarios

Posted by "Patrick Linskey (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/OPENJPA-219?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Patrick Linskey updated OPENJPA-219:
------------------------------------

    Attachment: OPENJPA-219.patch

New version of patch that uses iterated class instead of method argument.

> Reflection: negative caching would be beneficial in redeployment scenarios
> --------------------------------------------------------------------------
>
>                 Key: OPENJPA-219
>                 URL: https://issues.apache.org/jira/browse/OPENJPA-219
>             Project: OpenJPA
>          Issue Type: Bug
>          Components: kernel
>    Affects Versions: 0.9.0, 0.9.6, 0.9.7
>            Reporter: Patrick Linskey
>             Fix For: 0.9.8
>
>         Attachments: OPENJPA-219.patch
>
>
> In a variety of situations, OpenJPA searches class hierarchies for fields. These searches invoke Class.getDeclaredField() in order to find non-public fields. This method throws an exception when it fails to find the specified field, and the exception creation is, as ever, slow.
> It would be useful to create a static (and thus per-classloader) Map<WeakReference<Class>,Collection<String>> of fields known not to be available in a given class.
> It may also be beneficial to maintain a cache of which fields *are* present in a given class, but this issue is being filed as a result of a demonstrated performance hit during deployment due to the lack of a negative cache. If we obtain quantitative data indicating that a positive cache would be useful, we might want to implement such a thing at that time.
> Trace 3 (2115 occurances): [excepti][00090] java/lang/NoSuchFieldException: domainName
>      at java/lang/Class.getDeclaredField(Ljava/lang/String;I)Ljava/lang/reflect/Field;(Unknown Source)
>      at org/apache/openjpa/enhance/Reflection.findField(Ljava/lang/Class;Ljava/lang/String;Z)Ljava/lang/reflect/Field;(Reflection.java:101)
>      at org/apache/openjpa/util/ApplicationIds.toPKValues(Ljava/lang/Object;Lorg/apache/openjpa/meta/ClassMetaData;)[Ljava/lang/Object;
> (ApplicationIds.java:89)

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (OPENJPA-219) Reflection: negative caching would be beneficial in redeployment scenarios

Posted by "Abe White (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/OPENJPA-219?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12491310 ] 

Abe White commented on OPENJPA-219:
-----------------------------------

1. Yes, clearly we'd use our org.apache.openjpa.lib.util.concurrent. ConcurrentReferenceHashMap with weak keys to hold the Classes.

2. We have no evidence that a positive cache would consume any more memory than a negative cache in this case.  A positive cache's entry size for a given class is limited by the number of fields/methods in that class.  A negative cache's entry size is limited only by how many nonexistent field/method names we look for.  The negative cache will probably be smaller in this case except in deep inheritance hierarchies because I don't think we look for field or method names outside the inheritance hierarchy, but (a) I'm not sure of that and (b) there's no guarantee that will always be the case.

3. You know, another way to approach this would be not to cache at all, and just iterate over getDeclaredFields/Methods() looking for a match rather than using the singular getDeclaredField/Method() that throws an exception.  If that gives decent performance, we wouldn't have to worry about the memory consumption and complication of caching.  Bret, do you think you could try that out and see how it does?

> Reflection: negative caching would be beneficial in redeployment scenarios
> --------------------------------------------------------------------------
>
>                 Key: OPENJPA-219
>                 URL: https://issues.apache.org/jira/browse/OPENJPA-219
>             Project: OpenJPA
>          Issue Type: Bug
>          Components: kernel
>    Affects Versions: 0.9.0, 0.9.6, 0.9.7
>            Reporter: Patrick Linskey
>             Fix For: 0.9.8
>
>         Attachments: OPENJPA-219.patch
>
>
> In a variety of situations, OpenJPA searches class hierarchies for fields. These searches invoke Class.getDeclaredField() in order to find non-public fields. This method throws an exception when it fails to find the specified field, and the exception creation is, as ever, slow.
> It would be useful to create a static (and thus per-classloader) Map<WeakReference<Class>,Collection<String>> of fields known not to be available in a given class.
> It may also be beneficial to maintain a cache of which fields *are* present in a given class, but this issue is being filed as a result of a demonstrated performance hit during deployment due to the lack of a negative cache. If we obtain quantitative data indicating that a positive cache would be useful, we might want to implement such a thing at that time.
> Trace 3 (2115 occurances): [excepti][00090] java/lang/NoSuchFieldException: domainName
>      at java/lang/Class.getDeclaredField(Ljava/lang/String;I)Ljava/lang/reflect/Field;(Unknown Source)
>      at org/apache/openjpa/enhance/Reflection.findField(Ljava/lang/Class;Ljava/lang/String;Z)Ljava/lang/reflect/Field;(Reflection.java:101)
>      at org/apache/openjpa/util/ApplicationIds.toPKValues(Ljava/lang/Object;Lorg/apache/openjpa/meta/ClassMetaData;)[Ljava/lang/Object;
> (ApplicationIds.java:89)

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (OPENJPA-219) Reflection: negative caching would be beneficial in redeployment scenarios

Posted by "Abe White (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/OPENJPA-219?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12491038 ] 

Abe White commented on OPENJPA-219:
-----------------------------------

If we're going to cache, I don't see why we wouldn't cache the declared fields/methods rather than the nonexistent ones.  It would be a simple matter of keeping a Class->Set cache, where the Set is the names of the fields/methods returned by Class.getDeclaredFields/Methods().  Then we'd have both a positive (set.contains(x)) and negative (!set.contains(x)) cache.

I'd also like to see some explicit control over whether requests to find a field/method cause cache additions.  It might be that we always want caching now, but it's such a general class that I can imagine naïvely using the Reflection class's helper methods on one-off random field/method requests in the future without remembering the memory consequences.  If all use results in caching, that could be bad.  So I'd like us to either add separate caching methods or a boolean parameter to the existing methods.  And either way, we need to revisit our current use of Reflection to decide which version to use in the current code.

> Reflection: negative caching would be beneficial in redeployment scenarios
> --------------------------------------------------------------------------
>
>                 Key: OPENJPA-219
>                 URL: https://issues.apache.org/jira/browse/OPENJPA-219
>             Project: OpenJPA
>          Issue Type: Bug
>          Components: kernel
>    Affects Versions: 0.9.0, 0.9.6, 0.9.7
>            Reporter: Patrick Linskey
>             Fix For: 0.9.8
>
>         Attachments: OPENJPA-219.patch
>
>
> In a variety of situations, OpenJPA searches class hierarchies for fields. These searches invoke Class.getDeclaredField() in order to find non-public fields. This method throws an exception when it fails to find the specified field, and the exception creation is, as ever, slow.
> It would be useful to create a static (and thus per-classloader) Map<WeakReference<Class>,Collection<String>> of fields known not to be available in a given class.
> It may also be beneficial to maintain a cache of which fields *are* present in a given class, but this issue is being filed as a result of a demonstrated performance hit during deployment due to the lack of a negative cache. If we obtain quantitative data indicating that a positive cache would be useful, we might want to implement such a thing at that time.
> Trace 3 (2115 occurances): [excepti][00090] java/lang/NoSuchFieldException: domainName
>      at java/lang/Class.getDeclaredField(Ljava/lang/String;I)Ljava/lang/reflect/Field;(Unknown Source)
>      at org/apache/openjpa/enhance/Reflection.findField(Ljava/lang/Class;Ljava/lang/String;Z)Ljava/lang/reflect/Field;(Reflection.java:101)
>      at org/apache/openjpa/util/ApplicationIds.toPKValues(Ljava/lang/Object;Lorg/apache/openjpa/meta/ClassMetaData;)[Ljava/lang/Object;
> (ApplicationIds.java:89)

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (OPENJPA-219) Reflection: negative caching would be beneficial in redeployment scenarios

Posted by "Bret Weinraub (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/OPENJPA-219?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Bret Weinraub updated OPENJPA-219:
----------------------------------

    Attachment: OPENJPA-219-NoLeak.patch

Includes a non-caching performance enhancement for Reflection.findField()

Other implementations held on to references to class objects in static collections.   If the class loader which created this Class went away (like an application classloader during redeployment), this reference would prevent the ClassLoader object and associated Class objects from being garbage collected, which would have led to a memory leak.

This version is about 3X faster than the original for JRockit, and about 5X faster for HotSpot.

> Reflection: negative caching would be beneficial in redeployment scenarios
> --------------------------------------------------------------------------
>
>                 Key: OPENJPA-219
>                 URL: https://issues.apache.org/jira/browse/OPENJPA-219
>             Project: OpenJPA
>          Issue Type: Bug
>          Components: kernel
>    Affects Versions: 0.9.0, 0.9.6, 0.9.7
>            Reporter: Patrick Linskey
>             Fix For: 0.9.8
>
>         Attachments: OPENJPA-219-NoLeak.patch, OPENJPA-219.patch
>
>
> In a variety of situations, OpenJPA searches class hierarchies for fields. These searches invoke Class.getDeclaredField() in order to find non-public fields. This method throws an exception when it fails to find the specified field, and the exception creation is, as ever, slow.
> It would be useful to create a static (and thus per-classloader) Map<WeakReference<Class>,Collection<String>> of fields known not to be available in a given class.
> It may also be beneficial to maintain a cache of which fields *are* present in a given class, but this issue is being filed as a result of a demonstrated performance hit during deployment due to the lack of a negative cache. If we obtain quantitative data indicating that a positive cache would be useful, we might want to implement such a thing at that time.
> Trace 3 (2115 occurances): [excepti][00090] java/lang/NoSuchFieldException: domainName
>      at java/lang/Class.getDeclaredField(Ljava/lang/String;I)Ljava/lang/reflect/Field;(Unknown Source)
>      at org/apache/openjpa/enhance/Reflection.findField(Ljava/lang/Class;Ljava/lang/String;Z)Ljava/lang/reflect/Field;(Reflection.java:101)
>      at org/apache/openjpa/util/ApplicationIds.toPKValues(Ljava/lang/Object;Lorg/apache/openjpa/meta/ClassMetaData;)[Ljava/lang/Object;
> (ApplicationIds.java:89)

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (OPENJPA-219) Reflection: negative caching would be beneficial in redeployment scenarios

Posted by "Bret Weinraub (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/OPENJPA-219?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12491470 ] 

Bret Weinraub commented on OPENJPA-219:
---------------------------------------

The no-leak version is "cache v3" although that's a misnomer ; there's no cache involved.

VM - Version ms

Sun - Original 333
Sun - Cache v1 83
Sun - Cache v2 40
Sun - Cache v3 73

JRockit - Original 176
JRockit - Cache v1 72
JRockit - Cache v2 22
JRockit - Cache v3 52



> Reflection: negative caching would be beneficial in redeployment scenarios
> --------------------------------------------------------------------------
>
>                 Key: OPENJPA-219
>                 URL: https://issues.apache.org/jira/browse/OPENJPA-219
>             Project: OpenJPA
>          Issue Type: Bug
>          Components: kernel
>    Affects Versions: 0.9.0, 0.9.6, 0.9.7
>            Reporter: Patrick Linskey
>         Assigned To: Patrick Linskey
>             Fix For: 0.9.8
>
>         Attachments: OPENJPA-219-NoLeak.patch, OPENJPA-219.patch
>
>
> In a variety of situations, OpenJPA searches class hierarchies for fields. These searches invoke Class.getDeclaredField() in order to find non-public fields. This method throws an exception when it fails to find the specified field, and the exception creation is, as ever, slow.
> It would be useful to create a static (and thus per-classloader) Map<WeakReference<Class>,Collection<String>> of fields known not to be available in a given class.
> It may also be beneficial to maintain a cache of which fields *are* present in a given class, but this issue is being filed as a result of a demonstrated performance hit during deployment due to the lack of a negative cache. If we obtain quantitative data indicating that a positive cache would be useful, we might want to implement such a thing at that time.
> Trace 3 (2115 occurances): [excepti][00090] java/lang/NoSuchFieldException: domainName
>      at java/lang/Class.getDeclaredField(Ljava/lang/String;I)Ljava/lang/reflect/Field;(Unknown Source)
>      at org/apache/openjpa/enhance/Reflection.findField(Ljava/lang/Class;Ljava/lang/String;Z)Ljava/lang/reflect/Field;(Reflection.java:101)
>      at org/apache/openjpa/util/ApplicationIds.toPKValues(Ljava/lang/Object;Lorg/apache/openjpa/meta/ClassMetaData;)[Ljava/lang/Object;
> (ApplicationIds.java:89)

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.