You are viewing a plain text version of this content. The canonical link for it is here.
Posted to scm@geronimo.apache.org by mc...@apache.org on 2009/01/07 21:56:39 UTC
svn commit: r732486 -
/geronimo/server/trunk/framework/modules/geronimo-kernel/src/main/java/org/apache/geronimo/kernel/config/MultiParentClassLoader.java
Author: mcconne
Date: Wed Jan 7 12:56:39 2009
New Revision: 732486
URL: http://svn.apache.org/viewvc?rev=732486&view=rev
Log:
GERONIMO-4497 Change to prevent repetitive searches for the same (not found) resource
Modified:
geronimo/server/trunk/framework/modules/geronimo-kernel/src/main/java/org/apache/geronimo/kernel/config/MultiParentClassLoader.java
Modified: geronimo/server/trunk/framework/modules/geronimo-kernel/src/main/java/org/apache/geronimo/kernel/config/MultiParentClassLoader.java
URL: http://svn.apache.org/viewvc/geronimo/server/trunk/framework/modules/geronimo-kernel/src/main/java/org/apache/geronimo/kernel/config/MultiParentClassLoader.java?rev=732486&r1=732485&r2=732486&view=diff
==============================================================================
--- geronimo/server/trunk/framework/modules/geronimo-kernel/src/main/java/org/apache/geronimo/kernel/config/MultiParentClassLoader.java (original)
+++ geronimo/server/trunk/framework/modules/geronimo-kernel/src/main/java/org/apache/geronimo/kernel/config/MultiParentClassLoader.java Wed Jan 7 12:56:39 2009
@@ -60,6 +60,7 @@
private final ClassLoader[] parents;
private final ClassLoadingRules classLoadingRules;
private boolean destroyed = false;
+ private Set<String> knownResources = new HashSet<String>();
// I used this pattern as its temporary and with the static final we get compile time
// optimizations.
@@ -514,7 +515,7 @@
}
public URL getResource(String name) {
- if (isDestroyed()) {
+ if (isDestroyed() || knownResources.contains(name)) {
return null;
}
@@ -548,7 +549,17 @@
// resource, so we can override now
if (!isDestroyed()) {
// parents didn't have the resource; attempt to load it from my urls
- return findResource(name);
+ URL url = findResource(name);
+ if (url != null) {
+ return url;
+ }
+ }
+
+ //
+ // Resource not found -- no need to search for it again
+ //
+ if (!knownResources.contains(name)) {
+ knownResources.add(name);
}
return null;
Re: svn commit: r732486 - /geronimo/server/trunk/framework/modules/geronimo-kernel/src/main/java/org/apache/geronimo/kernel/config/MultiParentClassLoader.java
Posted by Tim McConnell <ti...@gmail.com>.
Hi Kevan, that's probably a good/safe idea, plus I don't think the maximum bound
needs to be very large. At least not for the BIRT engine failing scenario; if I
remember correctly it was only searching for -- but not finding -- a few hundred
resources. I don't think we'd necessarily require another class though, seems
like we can impose the bound easily enough and still benefit from the
efficiencies of the HashSet.....
Kevan Miller wrote:
> Hmm. Is there a HashCache or BoundedHashSet, or similar? Something that
> would limit the maximum number of unknown resources that we'll try to
> remember... Otherwise, we'll try to allocate an infinite amount of
> memory if somebody is silly enough to search for an infinite number of
> non-existent resources...
>
> --kevan
>
Re: svn commit: r732486 - /geronimo/server/trunk/framework/modules/geronimo-kernel/src/main/java/org/apache/geronimo/kernel/config/MultiParentClassLoader.java
Posted by Tim McConnell <ti...@gmail.com>.
Hi David, I'm concerned that an index of everything "findable" might be
prohibitively large. I've noticed that some of the resources that are searched
for are class files, which would mean we'd also have to maintain the string
representation of every class file (e.g., "javax/servlet/jsp/JspException.class")
in the classloader hierarchy in that index. Or am I mis-understanding what you're
suggesting ??
David Jencks wrote:
> I was wondering if we wanted to index everything that _is_ findable up
> front...
>
> david jencks
>
> On Jan 8, 2009, at 10:46 AM, Kevan Miller wrote:
>
>> Hmm. Is there a HashCache or BoundedHashSet, or similar? Something
>> that would limit the maximum number of unknown resources that we'll
>> try to remember... Otherwise, we'll try to allocate an infinite amount
>> of memory if somebody is silly enough to search for an infinite number
>> of non-existent resources...
>>
>> --kevan
>
>
Re: svn commit: r732486 - /geronimo/server/trunk/framework/modules/geronimo-kernel/src/main/java/org/apache/geronimo/kernel/config/MultiParentClassLoader.java
Posted by David Jencks <da...@yahoo.com>.
I was wondering if we wanted to index everything that _is_ findable up
front...
david jencks
On Jan 8, 2009, at 10:46 AM, Kevan Miller wrote:
> Hmm. Is there a HashCache or BoundedHashSet, or similar? Something
> that would limit the maximum number of unknown resources that we'll
> try to remember... Otherwise, we'll try to allocate an infinite
> amount of memory if somebody is silly enough to search for an
> infinite number of non-existent resources...
>
> --kevan
Re: svn commit: r732486 - /geronimo/server/trunk/framework/modules/geronimo-kernel/src/main/java/org/apache/geronimo/kernel/config/MultiParentClassLoader.java
Posted by Kevan Miller <ke...@gmail.com>.
Hmm. Is there a HashCache or BoundedHashSet, or similar? Something
that would limit the maximum number of unknown resources that we'll
try to remember... Otherwise, we'll try to allocate an infinite amount
of memory if somebody is silly enough to search for an infinite number
of non-existent resources...
--kevan
Re: svn commit: r732486 - /geronimo/server/trunk/framework/modules/geronimo-kernel/src/main/java/org/apache/geronimo/kernel/config/MultiParentClassLoader.java
Posted by Tim McConnell <ti...@gmail.com>.
Hi Jarek, I originally had named it "notFoundResources" and probably should have
left it at that. I'll change it to something less confusing. Thanks for reviewing....
Jarek Gawor wrote:
> Can we maybe change the variable name to something else, e.g.
> unknownResources? Calling it "knownResources" for resources that are
> not found is a little confusing.
>
> Jarek
>
> On Wed, Jan 7, 2009 at 3:56 PM, <mc...@apache.org> wrote:
>> Author: mcconne
>> Date: Wed Jan 7 12:56:39 2009
>> New Revision: 732486
>>
>> URL: http://svn.apache.org/viewvc?rev=732486&view=rev
>> Log:
>> GERONIMO-4497 Change to prevent repetitive searches for the same (not found) resource
>>
>> Modified:
>> geronimo/server/trunk/framework/modules/geronimo-kernel/src/main/java/org/apache/geronimo/kernel/config/MultiParentClassLoader.java
>>
>> Modified: geronimo/server/trunk/framework/modules/geronimo-kernel/src/main/java/org/apache/geronimo/kernel/config/MultiParentClassLoader.java
>> URL: http://svn.apache.org/viewvc/geronimo/server/trunk/framework/modules/geronimo-kernel/src/main/java/org/apache/geronimo/kernel/config/MultiParentClassLoader.java?rev=732486&r1=732485&r2=732486&view=diff
>> ==============================================================================
>> --- geronimo/server/trunk/framework/modules/geronimo-kernel/src/main/java/org/apache/geronimo/kernel/config/MultiParentClassLoader.java (original)
>> +++ geronimo/server/trunk/framework/modules/geronimo-kernel/src/main/java/org/apache/geronimo/kernel/config/MultiParentClassLoader.java Wed Jan 7 12:56:39 2009
>> @@ -60,6 +60,7 @@
>> private final ClassLoader[] parents;
>> private final ClassLoadingRules classLoadingRules;
>> private boolean destroyed = false;
>> + private Set<String> knownResources = new HashSet<String>();
>>
>> // I used this pattern as its temporary and with the static final we get compile time
>> // optimizations.
>> @@ -514,7 +515,7 @@
>> }
>>
>> public URL getResource(String name) {
>> - if (isDestroyed()) {
>> + if (isDestroyed() || knownResources.contains(name)) {
>> return null;
>> }
>>
>> @@ -548,7 +549,17 @@
>> // resource, so we can override now
>> if (!isDestroyed()) {
>> // parents didn't have the resource; attempt to load it from my urls
>> - return findResource(name);
>> + URL url = findResource(name);
>> + if (url != null) {
>> + return url;
>> + }
>> + }
>> +
>> + //
>> + // Resource not found -- no need to search for it again
>> + //
>> + if (!knownResources.contains(name)) {
>> + knownResources.add(name);
>> }
>>
>> return null;
>>
>>
>>
>
Re: svn commit: r732486 - /geronimo/server/trunk/framework/modules/geronimo-kernel/src/main/java/org/apache/geronimo/kernel/config/MultiParentClassLoader.java
Posted by Jarek Gawor <jg...@gmail.com>.
Can we maybe change the variable name to something else, e.g.
unknownResources? Calling it "knownResources" for resources that are
not found is a little confusing.
Jarek
On Wed, Jan 7, 2009 at 3:56 PM, <mc...@apache.org> wrote:
> Author: mcconne
> Date: Wed Jan 7 12:56:39 2009
> New Revision: 732486
>
> URL: http://svn.apache.org/viewvc?rev=732486&view=rev
> Log:
> GERONIMO-4497 Change to prevent repetitive searches for the same (not found) resource
>
> Modified:
> geronimo/server/trunk/framework/modules/geronimo-kernel/src/main/java/org/apache/geronimo/kernel/config/MultiParentClassLoader.java
>
> Modified: geronimo/server/trunk/framework/modules/geronimo-kernel/src/main/java/org/apache/geronimo/kernel/config/MultiParentClassLoader.java
> URL: http://svn.apache.org/viewvc/geronimo/server/trunk/framework/modules/geronimo-kernel/src/main/java/org/apache/geronimo/kernel/config/MultiParentClassLoader.java?rev=732486&r1=732485&r2=732486&view=diff
> ==============================================================================
> --- geronimo/server/trunk/framework/modules/geronimo-kernel/src/main/java/org/apache/geronimo/kernel/config/MultiParentClassLoader.java (original)
> +++ geronimo/server/trunk/framework/modules/geronimo-kernel/src/main/java/org/apache/geronimo/kernel/config/MultiParentClassLoader.java Wed Jan 7 12:56:39 2009
> @@ -60,6 +60,7 @@
> private final ClassLoader[] parents;
> private final ClassLoadingRules classLoadingRules;
> private boolean destroyed = false;
> + private Set<String> knownResources = new HashSet<String>();
>
> // I used this pattern as its temporary and with the static final we get compile time
> // optimizations.
> @@ -514,7 +515,7 @@
> }
>
> public URL getResource(String name) {
> - if (isDestroyed()) {
> + if (isDestroyed() || knownResources.contains(name)) {
> return null;
> }
>
> @@ -548,7 +549,17 @@
> // resource, so we can override now
> if (!isDestroyed()) {
> // parents didn't have the resource; attempt to load it from my urls
> - return findResource(name);
> + URL url = findResource(name);
> + if (url != null) {
> + return url;
> + }
> + }
> +
> + //
> + // Resource not found -- no need to search for it again
> + //
> + if (!knownResources.contains(name)) {
> + knownResources.add(name);
> }
>
> return null;
>
>
>