You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@struts.apache.org by "Giovanni Azua Garcia (JIRA)" <ji...@apache.org> on 2007/10/31 23:42:40 UTC

[jira] Created: (WW-2282) Apply patch to Struts2 core module on performance issues reported by FindBugs

Apply patch to Struts2 core module on performance issues reported by FindBugs 
------------------------------------------------------------------------------

                 Key: WW-2282
                 URL: https://issues.apache.org/struts/browse/WW-2282
             Project: Struts 2
          Issue Type: Improvement
          Components: Core Actions
    Affects Versions: 2.1.0
            Reporter: Giovanni Azua Garcia
            Assignee: Ted Husted
            Priority: Minor


Post to DEV list:

Recently I was following some threads in the Struts users list about performance issues in S2 and was curious to look at Struts code. Running findBugs evidences many small improvements that would overall and cheaply improve S2 performance e.g.

- In many places it is continuously creating large number of small objects by using new rather than valueOf. 
http://findbugs.sourceforge.net/bugDescriptions.html#DM_NUMBER_CTOR
- FindBugs spotted several places using non static inner classes unnecessarily.
- Also a few places use inneficiently keySet iterators on maps rather than using entrySet.
- Calling toString on String types.

Good idea probably adding findbugs to the pom reporting.


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


[jira] Updated: (WW-2282) Apply patch to Struts2 core module on performance issues reported by FindBugs

Posted by "Giovanni Azua Garcia (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/struts/browse/WW-2282?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Giovanni Azua Garcia updated WW-2282:
-------------------------------------

    Attachment: Struts2CorePerfPatch_ToRev590856.diff

Use patch to apply on the directory from:

$STRUTS_BASE/struts2/

regards,
Giovanni

> Apply patch to Struts2 core module on performance issues reported by FindBugs 
> ------------------------------------------------------------------------------
>
>                 Key: WW-2282
>                 URL: https://issues.apache.org/struts/browse/WW-2282
>             Project: Struts 2
>          Issue Type: Improvement
>          Components: Core Actions
>    Affects Versions: 2.1.0
>            Reporter: Giovanni Azua Garcia
>            Assignee: Ted Husted
>            Priority: Minor
>         Attachments: Struts2CorePerfPatch_ToRev590856.diff
>
>
> Post to DEV list:
> Recently I was following some threads in the Struts users list about performance issues in S2 and was curious to look at Struts code. Running findBugs evidences many small improvements that would overall and cheaply improve S2 performance e.g.
> - In many places it is continuously creating large number of small objects by using new rather than valueOf. 
> http://findbugs.sourceforge.net/bugDescriptions.html#DM_NUMBER_CTOR
> - FindBugs spotted several places using non static inner classes unnecessarily.
> - Also a few places use inneficiently keySet iterators on maps rather than using entrySet.
> - Calling toString on String types.
> Good idea probably adding findbugs to the pom reporting.

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


[jira] Commented: (WW-2282) Apply patch to Struts2 core module on performance issues reported by FindBugs

Posted by "Ted Husted (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/struts/browse/WW-2282?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_42501 ] 

Ted Husted commented on WW-2282:
--------------------------------

Thanks for doing this, Giovanni. 

I reviewed the DIFF, and I agree that these changes would be worthwhile. 

Are these changes being generated by a tool? If so, would they be easy to run again later. 

My concern is that we have a few dozen other patches to apply, and since these changes touch so many files, there may be conflicts with the other patches. Though, many seem to be one:one line substitutions, and so the patch system might be smart enough to work around. 

I do want to apply the changes, it's just a matter of timing. 

-Ted. 

> Apply patch to Struts2 core module on performance issues reported by FindBugs 
> ------------------------------------------------------------------------------
>
>                 Key: WW-2282
>                 URL: https://issues.apache.org/struts/browse/WW-2282
>             Project: Struts 2
>          Issue Type: Improvement
>          Components: Core Actions
>    Affects Versions: 2.1.0
>            Reporter: Giovanni Azua Garcia
>            Assignee: Ted Husted
>            Priority: Minor
>         Attachments: Struts2CorePerfPatch_ToRev590856.diff, Struts2CorePerfPatch_ToRev590856.zip
>
>
> Post to DEV list:
> Recently I was following some threads in the Struts users list about performance issues in S2 and was curious to look at Struts code. Running findBugs evidences many small improvements that would overall and cheaply improve S2 performance e.g.
> - In many places it is continuously creating large number of small objects by using new rather than valueOf. 
> http://findbugs.sourceforge.net/bugDescriptions.html#DM_NUMBER_CTOR
> - FindBugs spotted several places using non static inner classes unnecessarily.
> - Also a few places use inneficiently keySet iterators on maps rather than using entrySet.
> - Calling toString on String types.
> Good idea probably adding findbugs to the pom reporting.

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


[jira] Resolved: (WW-2282) Apply patch to Struts2 core module on performance issues reported by FindBugs

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

Ted Husted resolved WW-2282.
----------------------------

       Resolution: Fixed
    Fix Version/s: 2.1.1

r591174

OK, I applied these changes, since they are less likely to conflict with other changes that we make, and patch conflict always happens anyway. 

Feel free to submit other patches, but we may have to deal with some of the older patches first. 

-Ted.


> Apply patch to Struts2 core module on performance issues reported by FindBugs 
> ------------------------------------------------------------------------------
>
>                 Key: WW-2282
>                 URL: https://issues.apache.org/struts/browse/WW-2282
>             Project: Struts 2
>          Issue Type: Improvement
>          Components: Core Actions
>    Affects Versions: 2.1.0
>            Reporter: Giovanni Azua Garcia
>            Assignee: Ted Husted
>            Priority: Minor
>             Fix For: 2.1.1
>
>         Attachments: Struts2CorePerfPatch_ToRev590856.diff, Struts2CorePerfPatch_ToRev590856.zip
>
>
> Post to DEV list:
> Recently I was following some threads in the Struts users list about performance issues in S2 and was curious to look at Struts code. Running findBugs evidences many small improvements that would overall and cheaply improve S2 performance e.g.
> - In many places it is continuously creating large number of small objects by using new rather than valueOf. 
> http://findbugs.sourceforge.net/bugDescriptions.html#DM_NUMBER_CTOR
> - FindBugs spotted several places using non static inner classes unnecessarily.
> - Also a few places use inneficiently keySet iterators on maps rather than using entrySet.
> - Calling toString on String types.
> Good idea probably adding findbugs to the pom reporting.

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


[jira] Updated: (WW-2282) Apply patch to Struts2 core module on performance issues reported by FindBugs

Posted by "Giovanni Azua Garcia (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/struts/browse/WW-2282?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Giovanni Azua Garcia updated WW-2282:
-------------------------------------

    Attachment: Struts2CorePerfPatch_ToRev590856.zip

Zip containing all files patched.

> Apply patch to Struts2 core module on performance issues reported by FindBugs 
> ------------------------------------------------------------------------------
>
>                 Key: WW-2282
>                 URL: https://issues.apache.org/struts/browse/WW-2282
>             Project: Struts 2
>          Issue Type: Improvement
>          Components: Core Actions
>    Affects Versions: 2.1.0
>            Reporter: Giovanni Azua Garcia
>            Assignee: Ted Husted
>            Priority: Minor
>         Attachments: Struts2CorePerfPatch_ToRev590856.diff, Struts2CorePerfPatch_ToRev590856.zip
>
>
> Post to DEV list:
> Recently I was following some threads in the Struts users list about performance issues in S2 and was curious to look at Struts code. Running findBugs evidences many small improvements that would overall and cheaply improve S2 performance e.g.
> - In many places it is continuously creating large number of small objects by using new rather than valueOf. 
> http://findbugs.sourceforge.net/bugDescriptions.html#DM_NUMBER_CTOR
> - FindBugs spotted several places using non static inner classes unnecessarily.
> - Also a few places use inneficiently keySet iterators on maps rather than using entrySet.
> - Calling toString on String types.
> Good idea probably adding findbugs to the pom reporting.

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


[jira] Commented: (WW-2282) Apply patch to Struts2 core module on performance issues reported by FindBugs

Posted by "Giovanni Azua Garcia (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/struts/browse/WW-2282?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_42492 ] 

Giovanni Azua Garcia commented on WW-2282:
------------------------------------------

btw I checked all tests pass before/after the patch.

regards,
Giovanni

> Apply patch to Struts2 core module on performance issues reported by FindBugs 
> ------------------------------------------------------------------------------
>
>                 Key: WW-2282
>                 URL: https://issues.apache.org/struts/browse/WW-2282
>             Project: Struts 2
>          Issue Type: Improvement
>          Components: Core Actions
>    Affects Versions: 2.1.0
>            Reporter: Giovanni Azua Garcia
>            Assignee: Ted Husted
>            Priority: Minor
>         Attachments: Struts2CorePerfPatch_ToRev590856.diff, Struts2CorePerfPatch_ToRev590856.zip
>
>
> Post to DEV list:
> Recently I was following some threads in the Struts users list about performance issues in S2 and was curious to look at Struts code. Running findBugs evidences many small improvements that would overall and cheaply improve S2 performance e.g.
> - In many places it is continuously creating large number of small objects by using new rather than valueOf. 
> http://findbugs.sourceforge.net/bugDescriptions.html#DM_NUMBER_CTOR
> - FindBugs spotted several places using non static inner classes unnecessarily.
> - Also a few places use inneficiently keySet iterators on maps rather than using entrySet.
> - Calling toString on String types.
> Good idea probably adding findbugs to the pom reporting.

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


[jira] Commented: (WW-2282) Apply patch to Struts2 core module on performance issues reported by FindBugs

Posted by "Giovanni Azua Garcia (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/struts/browse/WW-2282?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_42507 ] 

Giovanni Azua Garcia commented on WW-2282:
------------------------------------------

Hi Ted,

I am glad to help! These issues were spotted by FindBugs but the fixes were done manually, so if you plan to do it in the future I will need the new code revision to work on it again, though it will be trivial to do. I would not expect the Unix patch tool will be able to merge this into another version of the files because the DIFF includes line numbers but I am not completely sure, will give it a try.

btw FindBugs also finds "Dodgy" and "Malicious" code e.g. situations that statically and predictably will lead to NullPointerException (I prefer the NullObject pattern to stay away from nulls though). Or situations that lead for sure to ClassCastException like this one:

SubsetIteratorTag

252     else if (startObj instanceof Double) {
253         start = ((Long) startObj).intValue();
254     }

I would like to also provide fixes for these issues if you would let me to ... 

I found use of raw Reflection in some points that could probably also be improved e.g. using Delegates MethodConfigurationProvider is error prone as the ActionConfiguration carries too much low level info about action methods.

I am working in a patterns framework that implements the Delegates in Java and I could also help to simplify that code:

http://perfectjpattern.svn.sourceforge.net/viewvc/perfectjpattern/trunk/perfectjpattern-api/src/main/java/perfectjpattern/core/api/behavioral/delegate/IDelegator.java?revision=158&view=markup

regards,
Giovanni

> Apply patch to Struts2 core module on performance issues reported by FindBugs 
> ------------------------------------------------------------------------------
>
>                 Key: WW-2282
>                 URL: https://issues.apache.org/struts/browse/WW-2282
>             Project: Struts 2
>          Issue Type: Improvement
>          Components: Core Actions
>    Affects Versions: 2.1.0
>            Reporter: Giovanni Azua Garcia
>            Assignee: Ted Husted
>            Priority: Minor
>         Attachments: Struts2CorePerfPatch_ToRev590856.diff, Struts2CorePerfPatch_ToRev590856.zip
>
>
> Post to DEV list:
> Recently I was following some threads in the Struts users list about performance issues in S2 and was curious to look at Struts code. Running findBugs evidences many small improvements that would overall and cheaply improve S2 performance e.g.
> - In many places it is continuously creating large number of small objects by using new rather than valueOf. 
> http://findbugs.sourceforge.net/bugDescriptions.html#DM_NUMBER_CTOR
> - FindBugs spotted several places using non static inner classes unnecessarily.
> - Also a few places use inneficiently keySet iterators on maps rather than using entrySet.
> - Calling toString on String types.
> Good idea probably adding findbugs to the pom reporting.

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