You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@struts.apache.org by "Patrick HIggins (JIRA)" <ji...@apache.org> on 2007/06/13 20:19:26 UTC

[jira] Created: (STR-3056) PropertyMessageResources.getMessage() does not cache failed lookups

PropertyMessageResources.getMessage() does not cache failed lookups
-------------------------------------------------------------------

                 Key: STR-3056
                 URL: https://issues.apache.org/struts/browse/STR-3056
             Project: Struts 1
          Issue Type: Improvement
          Components: Core
    Affects Versions: 1.3.8, 1.1 Final
         Environment: Applies to all environments
            Reporter: Patrick HIggins
            Priority: Minor


We have an application that allocates lots of garbage (sometimes over 100MB) when rendering a single page. After using Netbeans profiler with it, I've found that a lot of that garbage is created by MessageResources.messageKey().

It appears that we are calling bean:write (WriteTag) thousands of times, and it looks up the message for "org.apache.struts.taglib.bean.format.int" to try to find a default format for integers. We have not defined this property in our ApplicationResources, so it ends up returning the value "???en_US.org.apache.struts.taglib.bean.format.int???" after searching exhaustively for it. It does not cache this value. Then, when we call it the next 8,000 times, it performs the same exhaustive search over and over because it's not caching the negative response.

I propose that negative responses get cached, too. That would save a lot of time and memory so that WriteTag can just go ahead and call toString() on the instance of java.lang.Integer.

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


[jira] Commented: (STR-3056) PropertyMessageResources.getMessage() does not cache failed lookups

Posted by "Patrick HIggins (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/struts/browse/STR-3056?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_42096 ] 

Patrick HIggins commented on STR-3056:
--------------------------------------

I won't be able to run that under a profiler until I get back to work on Tuesday, but I'm pretty sure that it will have poor memory characteristics. That was one of the first things I tried a few months ago. If you trace the call tree, you'll see that there are many messageKey() and localeKey() calls invoked to get there. Also, if returnNull is true, I think it won't change the behavior at all to set cacheMisses to true.

I haven't inspected it super closely so I may be missing something, but it looks like you just added the cacheMisses property and then you store the null or ???locale.key??? in messages if it's set, right? If that's all you've done, I've definitely already tried this and it cut the memory usage down only slightly.

The real problem with the existing code is all the calls to messageKey() and localeKey() because they create new Strings on every call. A simple object like my MessageKey will consume a very modest amount of memory, but allocating a String is going to also require the allocation of a char array, and those will chew up memory fast.

> PropertyMessageResources.getMessage() does not cache failed lookups
> -------------------------------------------------------------------
>
>                 Key: STR-3056
>                 URL: https://issues.apache.org/struts/browse/STR-3056
>             Project: Struts 1
>          Issue Type: Improvement
>          Components: Core
>    Affects Versions: 1.1.0, 1.3.8
>         Environment: Applies to all environments
>            Reporter: Patrick HIggins
>             Fix For: 1.4.0
>
>         Attachments: cache_null.patch, message_resources.patch, message_resources_cache_null.patch, prof.tar.gz, PropertyMessageResources.java
>
>
> We have an application that allocates lots of garbage (sometimes over 100MB) when rendering a single page. After using Netbeans profiler with it, I've found that a lot of that garbage is created by MessageResources.messageKey().
> It appears that we are calling bean:write (WriteTag) thousands of times, and it looks up the message for "org.apache.struts.taglib.bean.format.int" to try to find a default format for integers. We have not defined this property in our ApplicationResources, so it ends up returning the value "???en_US.org.apache.struts.taglib.bean.format.int???" after searching exhaustively for it. It does not cache this value. Then, when we call it the next 8,000 times, it performs the same exhaustive search over and over because it's not caching the negative response.
> I propose that negative responses get cached, too. That would save a lot of time and memory so that WriteTag can just go ahead and call toString() on the instance of java.lang.Integer.

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


[jira] Updated: (STR-3056) PropertyMessageResources.getMessage() does not cache failed lookups

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

Patrick HIggins updated STR-3056:
---------------------------------

    Attachment: prof.tar.gz

Profiler results from a simple test program that looks up an non-existing key 10,000 times. Results collected with default Struts (baseline) and with each of my 3 patches applied.

Summary:

11M of char[] allocated in baseline
3.5M of char[] allocated in cacheNull
128k of char[] allocated in MessageKey
128k of char[] allocated in MessageKey + cacheNull

Big time memory savings with MessageKey refactor!

> PropertyMessageResources.getMessage() does not cache failed lookups
> -------------------------------------------------------------------
>
>                 Key: STR-3056
>                 URL: https://issues.apache.org/struts/browse/STR-3056
>             Project: Struts 1
>          Issue Type: Improvement
>          Components: Core
>    Affects Versions: 1.1.0, 1.3.8
>         Environment: Applies to all environments
>            Reporter: Patrick HIggins
>             Fix For: 1.4.0
>
>         Attachments: cache_null.patch, message_resources.patch, message_resources_cache_null.patch, prof.tar.gz
>
>
> We have an application that allocates lots of garbage (sometimes over 100MB) when rendering a single page. After using Netbeans profiler with it, I've found that a lot of that garbage is created by MessageResources.messageKey().
> It appears that we are calling bean:write (WriteTag) thousands of times, and it looks up the message for "org.apache.struts.taglib.bean.format.int" to try to find a default format for integers. We have not defined this property in our ApplicationResources, so it ends up returning the value "???en_US.org.apache.struts.taglib.bean.format.int???" after searching exhaustively for it. It does not cache this value. Then, when we call it the next 8,000 times, it performs the same exhaustive search over and over because it's not caching the negative response.
> I propose that negative responses get cached, too. That would save a lot of time and memory so that WriteTag can just go ahead and call toString() on the instance of java.lang.Integer.

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


[jira] Commented: (STR-3056) PropertyMessageResources.getMessage() does not cache failed lookups

Posted by "Paul Benedict (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/struts/browse/STR-3056?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_42084 ] 

Paul Benedict commented on STR-3056:
------------------------------------

Patrick, thank you for the patches. I will be reviewing them soon.

> PropertyMessageResources.getMessage() does not cache failed lookups
> -------------------------------------------------------------------
>
>                 Key: STR-3056
>                 URL: https://issues.apache.org/struts/browse/STR-3056
>             Project: Struts 1
>          Issue Type: Improvement
>          Components: Core
>    Affects Versions: 1.1.0, 1.3.8
>         Environment: Applies to all environments
>            Reporter: Patrick HIggins
>             Fix For: 1.4.0
>
>         Attachments: cache_null.patch, message_resources.patch, message_resources_cache_null.patch, prof.tar.gz
>
>
> We have an application that allocates lots of garbage (sometimes over 100MB) when rendering a single page. After using Netbeans profiler with it, I've found that a lot of that garbage is created by MessageResources.messageKey().
> It appears that we are calling bean:write (WriteTag) thousands of times, and it looks up the message for "org.apache.struts.taglib.bean.format.int" to try to find a default format for integers. We have not defined this property in our ApplicationResources, so it ends up returning the value "???en_US.org.apache.struts.taglib.bean.format.int???" after searching exhaustively for it. It does not cache this value. Then, when we call it the next 8,000 times, it performs the same exhaustive search over and over because it's not caching the negative response.
> I propose that negative responses get cached, too. That would save a lot of time and memory so that WriteTag can just go ahead and call toString() on the instance of java.lang.Integer.

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


[jira] Updated: (STR-3056) PropertyMessageResources.getMessage() does not cache failed lookups

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

Patrick HIggins updated STR-3056:
---------------------------------

    Attachment: message_resources.patch

Patch which allocates much less memory, but changes protected APIs, so subclasses would be broken.

The patch mainly changes from using String keys to MessageKey instances which store String locale and String key fields separately, but this allows to change the locale to another String during the search without having to allocate more storage for it.

Also, the locale search path (en_US -> en -> "") is stored in a static cache keyed by Locale to save computation and substring operations.

This should make lookups allocate a lot less garbage.

> PropertyMessageResources.getMessage() does not cache failed lookups
> -------------------------------------------------------------------
>
>                 Key: STR-3056
>                 URL: https://issues.apache.org/struts/browse/STR-3056
>             Project: Struts 1
>          Issue Type: Improvement
>          Components: Core
>    Affects Versions: 1.1.0, 1.3.8
>         Environment: Applies to all environments
>            Reporter: Patrick HIggins
>             Fix For: 1.4.0
>
>         Attachments: message_resources.patch
>
>
> We have an application that allocates lots of garbage (sometimes over 100MB) when rendering a single page. After using Netbeans profiler with it, I've found that a lot of that garbage is created by MessageResources.messageKey().
> It appears that we are calling bean:write (WriteTag) thousands of times, and it looks up the message for "org.apache.struts.taglib.bean.format.int" to try to find a default format for integers. We have not defined this property in our ApplicationResources, so it ends up returning the value "???en_US.org.apache.struts.taglib.bean.format.int???" after searching exhaustively for it. It does not cache this value. Then, when we call it the next 8,000 times, it performs the same exhaustive search over and over because it's not caching the negative response.
> I propose that negative responses get cached, too. That would save a lot of time and memory so that WriteTag can just go ahead and call toString() on the instance of java.lang.Integer.

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


[jira] Updated: (STR-3056) PropertyMessageResources.getMessage() does not cache failed lookups

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

Patrick HIggins updated STR-3056:
---------------------------------

    Attachment: cache_null.patch

Simple patch for caching null return values only. No changes to key objects.

> PropertyMessageResources.getMessage() does not cache failed lookups
> -------------------------------------------------------------------
>
>                 Key: STR-3056
>                 URL: https://issues.apache.org/struts/browse/STR-3056
>             Project: Struts 1
>          Issue Type: Improvement
>          Components: Core
>    Affects Versions: 1.1.0, 1.3.8
>         Environment: Applies to all environments
>            Reporter: Patrick HIggins
>             Fix For: 1.4.0
>
>         Attachments: cache_null.patch, message_resources.patch, message_resources_cache_null.patch
>
>
> We have an application that allocates lots of garbage (sometimes over 100MB) when rendering a single page. After using Netbeans profiler with it, I've found that a lot of that garbage is created by MessageResources.messageKey().
> It appears that we are calling bean:write (WriteTag) thousands of times, and it looks up the message for "org.apache.struts.taglib.bean.format.int" to try to find a default format for integers. We have not defined this property in our ApplicationResources, so it ends up returning the value "???en_US.org.apache.struts.taglib.bean.format.int???" after searching exhaustively for it. It does not cache this value. Then, when we call it the next 8,000 times, it performs the same exhaustive search over and over because it's not caching the negative response.
> I propose that negative responses get cached, too. That would save a lot of time and memory so that WriteTag can just go ahead and call toString() on the instance of java.lang.Integer.

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


[jira] Updated: (STR-3056) PropertyMessageResources.getMessage() does not cache failed lookups

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

Patrick HIggins updated STR-3056:
---------------------------------

    Attachment: message_resources_cache_null.patch

Here's a refinement of the previous patch that includes a cacheNull property on PropertyMessageResources and PropertyMessageResourcesFactory which will allow less work to be done when looking up a key that won't be found.

I may submit a third patch that includes this functionality without the big refactor of String keys to MessageKey keys.

Let me know what you think.

> PropertyMessageResources.getMessage() does not cache failed lookups
> -------------------------------------------------------------------
>
>                 Key: STR-3056
>                 URL: https://issues.apache.org/struts/browse/STR-3056
>             Project: Struts 1
>          Issue Type: Improvement
>          Components: Core
>    Affects Versions: 1.1.0, 1.3.8
>         Environment: Applies to all environments
>            Reporter: Patrick HIggins
>             Fix For: 1.4.0
>
>         Attachments: message_resources.patch, message_resources_cache_null.patch
>
>
> We have an application that allocates lots of garbage (sometimes over 100MB) when rendering a single page. After using Netbeans profiler with it, I've found that a lot of that garbage is created by MessageResources.messageKey().
> It appears that we are calling bean:write (WriteTag) thousands of times, and it looks up the message for "org.apache.struts.taglib.bean.format.int" to try to find a default format for integers. We have not defined this property in our ApplicationResources, so it ends up returning the value "???en_US.org.apache.struts.taglib.bean.format.int???" after searching exhaustively for it. It does not cache this value. Then, when we call it the next 8,000 times, it performs the same exhaustive search over and over because it's not caching the negative response.
> I propose that negative responses get cached, too. That would save a lot of time and memory so that WriteTag can just go ahead and call toString() on the instance of java.lang.Integer.

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


[jira] Updated: (STR-3056) PropertyMessageResources.getMessage() does not cache failed lookups

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

Patrick HIggins updated STR-3056:
---------------------------------

    Priority: Major  (was: Minor)

> PropertyMessageResources.getMessage() does not cache failed lookups
> -------------------------------------------------------------------
>
>                 Key: STR-3056
>                 URL: https://issues.apache.org/struts/browse/STR-3056
>             Project: Struts 1
>          Issue Type: Improvement
>          Components: Core
>    Affects Versions: 1.1 Final, 1.3.8
>         Environment: Applies to all environments
>            Reporter: Patrick HIggins
>
> We have an application that allocates lots of garbage (sometimes over 100MB) when rendering a single page. After using Netbeans profiler with it, I've found that a lot of that garbage is created by MessageResources.messageKey().
> It appears that we are calling bean:write (WriteTag) thousands of times, and it looks up the message for "org.apache.struts.taglib.bean.format.int" to try to find a default format for integers. We have not defined this property in our ApplicationResources, so it ends up returning the value "???en_US.org.apache.struts.taglib.bean.format.int???" after searching exhaustively for it. It does not cache this value. Then, when we call it the next 8,000 times, it performs the same exhaustive search over and over because it's not caching the negative response.
> I propose that negative responses get cached, too. That would save a lot of time and memory so that WriteTag can just go ahead and call toString() on the instance of java.lang.Integer.

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


[jira] Updated: (STR-3056) PropertyMessageResources.getMessage() does not cache failed lookups

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

Patrick HIggins updated STR-3056:
---------------------------------

    Issue Type: Bug  (was: Improvement)

> PropertyMessageResources.getMessage() does not cache failed lookups
> -------------------------------------------------------------------
>
>                 Key: STR-3056
>                 URL: https://issues.apache.org/struts/browse/STR-3056
>             Project: Struts 1
>          Issue Type: Bug
>          Components: Core
>    Affects Versions: 1.1 Final, 1.3.8
>         Environment: Applies to all environments
>            Reporter: Patrick HIggins
>
> We have an application that allocates lots of garbage (sometimes over 100MB) when rendering a single page. After using Netbeans profiler with it, I've found that a lot of that garbage is created by MessageResources.messageKey().
> It appears that we are calling bean:write (WriteTag) thousands of times, and it looks up the message for "org.apache.struts.taglib.bean.format.int" to try to find a default format for integers. We have not defined this property in our ApplicationResources, so it ends up returning the value "???en_US.org.apache.struts.taglib.bean.format.int???" after searching exhaustively for it. It does not cache this value. Then, when we call it the next 8,000 times, it performs the same exhaustive search over and over because it's not caching the negative response.
> I propose that negative responses get cached, too. That would save a lot of time and memory so that WriteTag can just go ahead and call toString() on the instance of java.lang.Integer.

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


[jira] Commented: (STR-3056) PropertyMessageResources.getMessage() does not cache failed lookups

Posted by "Paul Benedict (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/struts/browse/STR-3056?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_42021 ] 

Paul Benedict commented on STR-3056:
------------------------------------

Patrick, I am interested in addressing this problem. If it can be solved for 1.3 too, let us know. If you have any patches, please provide them.

> PropertyMessageResources.getMessage() does not cache failed lookups
> -------------------------------------------------------------------
>
>                 Key: STR-3056
>                 URL: https://issues.apache.org/struts/browse/STR-3056
>             Project: Struts 1
>          Issue Type: Improvement
>          Components: Core
>    Affects Versions: 1.1.0, 1.3.8
>         Environment: Applies to all environments
>            Reporter: Patrick HIggins
>             Fix For: 1.4.0
>
>
> We have an application that allocates lots of garbage (sometimes over 100MB) when rendering a single page. After using Netbeans profiler with it, I've found that a lot of that garbage is created by MessageResources.messageKey().
> It appears that we are calling bean:write (WriteTag) thousands of times, and it looks up the message for "org.apache.struts.taglib.bean.format.int" to try to find a default format for integers. We have not defined this property in our ApplicationResources, so it ends up returning the value "???en_US.org.apache.struts.taglib.bean.format.int???" after searching exhaustively for it. It does not cache this value. Then, when we call it the next 8,000 times, it performs the same exhaustive search over and over because it's not caching the negative response.
> I propose that negative responses get cached, too. That would save a lot of time and memory so that WriteTag can just go ahead and call toString() on the instance of java.lang.Integer.

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


[jira] Commented: (STR-3056) PropertyMessageResources.getMessage() does not cache failed lookups

Posted by "Patrick HIggins (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/struts/browse/STR-3056?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_41951 ] 

Patrick HIggins commented on STR-3056:
--------------------------------------

returnNull reduced the memory problem very slightly. The problem isn't creating the return value, but all the keys that are used in messages.get(key) calls.

We've decided to subclass PropertyMessageResources already. First we were doing our own caching, but Niall's suggestion to return null for these special keys rather than caching is good, and we're trying that out now, and is probably what we'll go with.

I'm pretty sure it will create more garbage than my caching strategy did, but we don't have to worry about bizarre bugs involving locale load orders.

Thanks for the ideas!

> PropertyMessageResources.getMessage() does not cache failed lookups
> -------------------------------------------------------------------
>
>                 Key: STR-3056
>                 URL: https://issues.apache.org/struts/browse/STR-3056
>             Project: Struts 1
>          Issue Type: Improvement
>          Components: Core
>    Affects Versions: 1.1.0, 1.3.8
>         Environment: Applies to all environments
>            Reporter: Patrick HIggins
>             Fix For: 1.4.0
>
>
> We have an application that allocates lots of garbage (sometimes over 100MB) when rendering a single page. After using Netbeans profiler with it, I've found that a lot of that garbage is created by MessageResources.messageKey().
> It appears that we are calling bean:write (WriteTag) thousands of times, and it looks up the message for "org.apache.struts.taglib.bean.format.int" to try to find a default format for integers. We have not defined this property in our ApplicationResources, so it ends up returning the value "???en_US.org.apache.struts.taglib.bean.format.int???" after searching exhaustively for it. It does not cache this value. Then, when we call it the next 8,000 times, it performs the same exhaustive search over and over because it's not caching the negative response.
> I propose that negative responses get cached, too. That would save a lot of time and memory so that WriteTag can just go ahead and call toString() on the instance of java.lang.Integer.

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


[jira] Commented: (STR-3056) PropertyMessageResources.getMessage() does not cache failed lookups

Posted by "Niall Pemberton (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/struts/browse/STR-3056?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_41743 ] 

Niall Pemberton commented on STR-3056:
--------------------------------------

I don't have a strong opnion - except if you change it be careful to not mess up the caching - we had some bugs previously where depending on which locale was loaded first different messages resulted.

The other point is does setting the "returnNull" configuration parameter to "true" help? It won't create that key so many times - although from memory it may be doing other things like creating String keys for locales. Another workaround would be to customize PropertyMessageResources yourself - if you never use/specify "org.apache.struts.taglib.bean.format.int" - then you could just return null straight away and cache nothing.

> PropertyMessageResources.getMessage() does not cache failed lookups
> -------------------------------------------------------------------
>
>                 Key: STR-3056
>                 URL: https://issues.apache.org/struts/browse/STR-3056
>             Project: Struts 1
>          Issue Type: Improvement
>          Components: Core
>    Affects Versions: 1.1.0, 1.3.8
>         Environment: Applies to all environments
>            Reporter: Patrick HIggins
>             Fix For: 1.4.0
>
>
> We have an application that allocates lots of garbage (sometimes over 100MB) when rendering a single page. After using Netbeans profiler with it, I've found that a lot of that garbage is created by MessageResources.messageKey().
> It appears that we are calling bean:write (WriteTag) thousands of times, and it looks up the message for "org.apache.struts.taglib.bean.format.int" to try to find a default format for integers. We have not defined this property in our ApplicationResources, so it ends up returning the value "???en_US.org.apache.struts.taglib.bean.format.int???" after searching exhaustively for it. It does not cache this value. Then, when we call it the next 8,000 times, it performs the same exhaustive search over and over because it's not caching the negative response.
> I propose that negative responses get cached, too. That would save a lot of time and memory so that WriteTag can just go ahead and call toString() on the instance of java.lang.Integer.

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


[jira] Updated: (STR-3056) PropertyMessageResources.getMessage() does not cache failed lookups

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

Patrick HIggins updated STR-3056:
---------------------------------

    Attachment: message_resources_cache_null.patch2
                struts-patches.html

I have attached an updated version of my MessageKey + cacheNull patch which uses Paul's idea of storing the ???message.key??? return values in the messages HashMap if returnNull is false. This makes the patch behave well for all combinations of returnNull and cacheNull.

I have also attached a spreadsheet comparing the performance of unpatched Struts, my patch, and Paul's patch. Paul's patch does not have any significant change in behavior over unpatched Struts. My patch uses only 2% of the memory and between 6% and 20% of the time of the unpatched version.

Please review my patch more closely and let me know if there are any specific objections to the techniques I'm using. My testing indicates that this could speed things up for our usage tremendously.

Also, I may create a separate patch intended to go on top of this patch if it is accepted. I've found that the isTraceEnabled() at the top of loadLocale() is actually a bit of a hotspot after my patch is applied.

> PropertyMessageResources.getMessage() does not cache failed lookups
> -------------------------------------------------------------------
>
>                 Key: STR-3056
>                 URL: https://issues.apache.org/struts/browse/STR-3056
>             Project: Struts 1
>          Issue Type: Improvement
>          Components: Core
>    Affects Versions: 1.1.0, 1.3.8
>         Environment: Applies to all environments
>            Reporter: Patrick HIggins
>             Fix For: 1.4.0
>
>         Attachments: cache_null.patch, message_resources.patch, message_resources_cache_null.patch, message_resources_cache_null.patch2, prof.tar.gz, PropertyMessageResources.java, struts-patches.html
>
>
> We have an application that allocates lots of garbage (sometimes over 100MB) when rendering a single page. After using Netbeans profiler with it, I've found that a lot of that garbage is created by MessageResources.messageKey().
> It appears that we are calling bean:write (WriteTag) thousands of times, and it looks up the message for "org.apache.struts.taglib.bean.format.int" to try to find a default format for integers. We have not defined this property in our ApplicationResources, so it ends up returning the value "???en_US.org.apache.struts.taglib.bean.format.int???" after searching exhaustively for it. It does not cache this value. Then, when we call it the next 8,000 times, it performs the same exhaustive search over and over because it's not caching the negative response.
> I propose that negative responses get cached, too. That would save a lot of time and memory so that WriteTag can just go ahead and call toString() on the instance of java.lang.Integer.

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


[jira] Updated: (STR-3056) PropertyMessageResources.getMessage() does not cache failed lookups

Posted by "Paul Benedict (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/struts/browse/STR-3056?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Paul Benedict updated STR-3056:
-------------------------------

    Fix Version/s:     (was: 1.3.10)

> PropertyMessageResources.getMessage() does not cache failed lookups
> -------------------------------------------------------------------
>
>                 Key: STR-3056
>                 URL: https://issues.apache.org/struts/browse/STR-3056
>             Project: Struts 1
>          Issue Type: Improvement
>          Components: Core
>    Affects Versions: 1.1.0, 1.3.8
>         Environment: Applies to all environments
>            Reporter: Patrick HIggins
>             Fix For: 1.4.0
>
>
> We have an application that allocates lots of garbage (sometimes over 100MB) when rendering a single page. After using Netbeans profiler with it, I've found that a lot of that garbage is created by MessageResources.messageKey().
> It appears that we are calling bean:write (WriteTag) thousands of times, and it looks up the message for "org.apache.struts.taglib.bean.format.int" to try to find a default format for integers. We have not defined this property in our ApplicationResources, so it ends up returning the value "???en_US.org.apache.struts.taglib.bean.format.int???" after searching exhaustively for it. It does not cache this value. Then, when we call it the next 8,000 times, it performs the same exhaustive search over and over because it's not caching the negative response.
> I propose that negative responses get cached, too. That would save a lot of time and memory so that WriteTag can just go ahead and call toString() on the instance of java.lang.Integer.

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


[jira] Commented: (STR-3056) PropertyMessageResources.getMessage() does not cache failed lookups

Posted by "Henri Yandell (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/struts/browse/STR-3056?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_41738 ] 

Henri Yandell commented on STR-3056:
------------------------------------

Seems useful - anyone know if this would break any existing functionality?

> PropertyMessageResources.getMessage() does not cache failed lookups
> -------------------------------------------------------------------
>
>                 Key: STR-3056
>                 URL: https://issues.apache.org/struts/browse/STR-3056
>             Project: Struts 1
>          Issue Type: Bug
>          Components: Core
>    Affects Versions: 1.1.0, 1.3.8
>         Environment: Applies to all environments
>            Reporter: Patrick HIggins
>
> We have an application that allocates lots of garbage (sometimes over 100MB) when rendering a single page. After using Netbeans profiler with it, I've found that a lot of that garbage is created by MessageResources.messageKey().
> It appears that we are calling bean:write (WriteTag) thousands of times, and it looks up the message for "org.apache.struts.taglib.bean.format.int" to try to find a default format for integers. We have not defined this property in our ApplicationResources, so it ends up returning the value "???en_US.org.apache.struts.taglib.bean.format.int???" after searching exhaustively for it. It does not cache this value. Then, when we call it the next 8,000 times, it performs the same exhaustive search over and over because it's not caching the negative response.
> I propose that negative responses get cached, too. That would save a lot of time and memory so that WriteTag can just go ahead and call toString() on the instance of java.lang.Integer.

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


[jira] Commented: (STR-3056) PropertyMessageResources.getMessage() does not cache failed lookups

Posted by "Paul Benedict (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/struts/browse/STR-3056?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_41742 ] 

Paul Benedict commented on STR-3056:
------------------------------------

Message resources are on the classpath and JVMs pre-6.0 cannot reload/change those. However, that's not true since JDK 6: 
http://java.sun.com/javase/6/docs/api/java/util/ResourceBundle.Control.html

The issue, while problematic for some use cases, is not a bug. I marked this as an improvement for 1.4 only because I foresee a backwards compatibility problem. I recommend a flag to be added to message resources to control whether caching should happen also for cache misses.

What do you say Niall?

> PropertyMessageResources.getMessage() does not cache failed lookups
> -------------------------------------------------------------------
>
>                 Key: STR-3056
>                 URL: https://issues.apache.org/struts/browse/STR-3056
>             Project: Struts 1
>          Issue Type: Improvement
>          Components: Core
>    Affects Versions: 1.1.0, 1.3.8
>         Environment: Applies to all environments
>            Reporter: Patrick HIggins
>             Fix For: 1.3.10, 1.4.0
>
>
> We have an application that allocates lots of garbage (sometimes over 100MB) when rendering a single page. After using Netbeans profiler with it, I've found that a lot of that garbage is created by MessageResources.messageKey().
> It appears that we are calling bean:write (WriteTag) thousands of times, and it looks up the message for "org.apache.struts.taglib.bean.format.int" to try to find a default format for integers. We have not defined this property in our ApplicationResources, so it ends up returning the value "???en_US.org.apache.struts.taglib.bean.format.int???" after searching exhaustively for it. It does not cache this value. Then, when we call it the next 8,000 times, it performs the same exhaustive search over and over because it's not caching the negative response.
> I propose that negative responses get cached, too. That would save a lot of time and memory so that WriteTag can just go ahead and call toString() on the instance of java.lang.Integer.

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


[jira] Updated: (STR-3056) PropertyMessageResources.getMessage() does not cache failed lookups

Posted by "Henri Yandell (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/struts/browse/STR-3056?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Henri Yandell updated STR-3056:
-------------------------------

    Fix Version/s: 1.3.10

> PropertyMessageResources.getMessage() does not cache failed lookups
> -------------------------------------------------------------------
>
>                 Key: STR-3056
>                 URL: https://issues.apache.org/struts/browse/STR-3056
>             Project: Struts 1
>          Issue Type: Bug
>          Components: Core
>    Affects Versions: 1.1.0, 1.3.8
>         Environment: Applies to all environments
>            Reporter: Patrick HIggins
>             Fix For: 1.3.10
>
>
> We have an application that allocates lots of garbage (sometimes over 100MB) when rendering a single page. After using Netbeans profiler with it, I've found that a lot of that garbage is created by MessageResources.messageKey().
> It appears that we are calling bean:write (WriteTag) thousands of times, and it looks up the message for "org.apache.struts.taglib.bean.format.int" to try to find a default format for integers. We have not defined this property in our ApplicationResources, so it ends up returning the value "???en_US.org.apache.struts.taglib.bean.format.int???" after searching exhaustively for it. It does not cache this value. Then, when we call it the next 8,000 times, it performs the same exhaustive search over and over because it's not caching the negative response.
> I propose that negative responses get cached, too. That would save a lot of time and memory so that WriteTag can just go ahead and call toString() on the instance of java.lang.Integer.

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


[jira] Commented: (STR-3056) PropertyMessageResources.getMessage() does not cache failed lookups

Posted by "Paul Benedict (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/struts/browse/STR-3056?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_41935 ] 

Paul Benedict commented on STR-3056:
------------------------------------

Patrick, can you set returnNull to true and tell us if your memory problem goes away? Please respond.

> PropertyMessageResources.getMessage() does not cache failed lookups
> -------------------------------------------------------------------
>
>                 Key: STR-3056
>                 URL: https://issues.apache.org/struts/browse/STR-3056
>             Project: Struts 1
>          Issue Type: Improvement
>          Components: Core
>    Affects Versions: 1.1.0, 1.3.8
>         Environment: Applies to all environments
>            Reporter: Patrick HIggins
>             Fix For: 1.4.0
>
>
> We have an application that allocates lots of garbage (sometimes over 100MB) when rendering a single page. After using Netbeans profiler with it, I've found that a lot of that garbage is created by MessageResources.messageKey().
> It appears that we are calling bean:write (WriteTag) thousands of times, and it looks up the message for "org.apache.struts.taglib.bean.format.int" to try to find a default format for integers. We have not defined this property in our ApplicationResources, so it ends up returning the value "???en_US.org.apache.struts.taglib.bean.format.int???" after searching exhaustively for it. It does not cache this value. Then, when we call it the next 8,000 times, it performs the same exhaustive search over and over because it's not caching the negative response.
> I propose that negative responses get cached, too. That would save a lot of time and memory so that WriteTag can just go ahead and call toString() on the instance of java.lang.Integer.

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


[jira] Updated: (STR-3056) PropertyMessageResources.getMessage() does not cache failed lookups

Posted by "Paul Benedict (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/struts/browse/STR-3056?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Paul Benedict updated STR-3056:
-------------------------------

    Attachment: PropertyMessageResources.java

Please try this. I've taken a slightly different approach and provide an option to cache keys go unresolved. Tell me if this works.

> PropertyMessageResources.getMessage() does not cache failed lookups
> -------------------------------------------------------------------
>
>                 Key: STR-3056
>                 URL: https://issues.apache.org/struts/browse/STR-3056
>             Project: Struts 1
>          Issue Type: Improvement
>          Components: Core
>    Affects Versions: 1.1.0, 1.3.8
>         Environment: Applies to all environments
>            Reporter: Patrick HIggins
>             Fix For: 1.4.0
>
>         Attachments: cache_null.patch, message_resources.patch, message_resources_cache_null.patch, prof.tar.gz, PropertyMessageResources.java
>
>
> We have an application that allocates lots of garbage (sometimes over 100MB) when rendering a single page. After using Netbeans profiler with it, I've found that a lot of that garbage is created by MessageResources.messageKey().
> It appears that we are calling bean:write (WriteTag) thousands of times, and it looks up the message for "org.apache.struts.taglib.bean.format.int" to try to find a default format for integers. We have not defined this property in our ApplicationResources, so it ends up returning the value "???en_US.org.apache.struts.taglib.bean.format.int???" after searching exhaustively for it. It does not cache this value. Then, when we call it the next 8,000 times, it performs the same exhaustive search over and over because it's not caching the negative response.
> I propose that negative responses get cached, too. That would save a lot of time and memory so that WriteTag can just go ahead and call toString() on the instance of java.lang.Integer.

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


[jira] Updated: (STR-3056) PropertyMessageResources.getMessage() does not cache failed lookups

Posted by "Paul Benedict (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/struts/browse/STR-3056?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Paul Benedict updated STR-3056:
-------------------------------

    Fix Version/s: 1.4.0
       Issue Type: Improvement  (was: Bug)

> PropertyMessageResources.getMessage() does not cache failed lookups
> -------------------------------------------------------------------
>
>                 Key: STR-3056
>                 URL: https://issues.apache.org/struts/browse/STR-3056
>             Project: Struts 1
>          Issue Type: Improvement
>          Components: Core
>    Affects Versions: 1.1.0, 1.3.8
>         Environment: Applies to all environments
>            Reporter: Patrick HIggins
>             Fix For: 1.3.10, 1.4.0
>
>
> We have an application that allocates lots of garbage (sometimes over 100MB) when rendering a single page. After using Netbeans profiler with it, I've found that a lot of that garbage is created by MessageResources.messageKey().
> It appears that we are calling bean:write (WriteTag) thousands of times, and it looks up the message for "org.apache.struts.taglib.bean.format.int" to try to find a default format for integers. We have not defined this property in our ApplicationResources, so it ends up returning the value "???en_US.org.apache.struts.taglib.bean.format.int???" after searching exhaustively for it. It does not cache this value. Then, when we call it the next 8,000 times, it performs the same exhaustive search over and over because it's not caching the negative response.
> I propose that negative responses get cached, too. That would save a lot of time and memory so that WriteTag can just go ahead and call toString() on the instance of java.lang.Integer.

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


[jira] Commented: (STR-3056) PropertyMessageResources.getMessage() does not cache failed lookups

Posted by "Patrick HIggins (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/struts/browse/STR-3056?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_42071 ] 

Patrick HIggins commented on STR-3056:
--------------------------------------

I haven't coded any patches other than some simple caching so far. I think the best thing to do would be to refactor those classes entirely to use non-string keys. However, that would be an API change and would break any subclasses that users have written.

I'm not sure how strict your policy is on source-level compatibility within a release. I'll try reworking those classes (MessageResources and PropertyMessageResources) into something more efficient and see just how badly I have to break compatibility. I think it will only require changing the signature of a protected method, although changing the signature will probably ripple out larger changes to any existing subclasses.

> PropertyMessageResources.getMessage() does not cache failed lookups
> -------------------------------------------------------------------
>
>                 Key: STR-3056
>                 URL: https://issues.apache.org/struts/browse/STR-3056
>             Project: Struts 1
>          Issue Type: Improvement
>          Components: Core
>    Affects Versions: 1.1.0, 1.3.8
>         Environment: Applies to all environments
>            Reporter: Patrick HIggins
>             Fix For: 1.4.0
>
>
> We have an application that allocates lots of garbage (sometimes over 100MB) when rendering a single page. After using Netbeans profiler with it, I've found that a lot of that garbage is created by MessageResources.messageKey().
> It appears that we are calling bean:write (WriteTag) thousands of times, and it looks up the message for "org.apache.struts.taglib.bean.format.int" to try to find a default format for integers. We have not defined this property in our ApplicationResources, so it ends up returning the value "???en_US.org.apache.struts.taglib.bean.format.int???" after searching exhaustively for it. It does not cache this value. Then, when we call it the next 8,000 times, it performs the same exhaustive search over and over because it's not caching the negative response.
> I propose that negative responses get cached, too. That would save a lot of time and memory so that WriteTag can just go ahead and call toString() on the instance of java.lang.Integer.

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