codenarc-developer Mailing List for CodeNarc (Page 4)
Brought to you by:
chrismair
Menu
▾
▴
codenarc-developer — CodeNarc Developers
This list is closed, nobody may subscribe to it.
| 2010 |
Jan
|
Feb
|
Mar
|
Apr
(8) |
May
(17) |
Jun
|
Jul
|
Aug
|
Sep
|
Oct
(8) |
Nov
(67) |
Dec
(9) |
|---|---|---|---|---|---|---|---|---|---|---|---|---|
| 2011 |
Jan
(23) |
Feb
(19) |
Mar
(15) |
Apr
(7) |
May
(5) |
Jun
(43) |
Jul
(5) |
Aug
(11) |
Sep
(18) |
Oct
(9) |
Nov
(6) |
Dec
|
| 2012 |
Jan
(7) |
Feb
(2) |
Mar
(7) |
Apr
|
May
|
Jun
|
Jul
(17) |
Aug
(5) |
Sep
|
Oct
(1) |
Nov
|
Dec
(1) |
| 2013 |
Jan
|
Feb
(1) |
Mar
(7) |
Apr
|
May
(6) |
Jun
|
Jul
(2) |
Aug
|
Sep
|
Oct
|
Nov
|
Dec
|
| 2014 |
Jan
(6) |
Feb
(3) |
Mar
|
Apr
|
May
|
Jun
|
Jul
|
Aug
|
Sep
|
Oct
|
Nov
|
Dec
|
| 2016 |
Jan
(2) |
Feb
|
Mar
|
Apr
(2) |
May
|
Jun
|
Jul
|
Aug
|
Sep
|
Oct
|
Nov
|
Dec
|
| 2017 |
Jan
|
Feb
|
Mar
(4) |
Apr
|
May
|
Jun
|
Jul
|
Aug
|
Sep
|
Oct
|
Nov
|
Dec
|
|
From: Chris M. <chr...@ea...> - 2011-11-10 18:49:25
|
Hamlet, The documentation for UnnecessaryOverridingGetterRule says: If a parent class defines a public method that follows the Java getter notation, and a subclass overrides that method to return a constant, then it is cleaner to provide a Groovy property for the value rather than a Groovy method. But the implementation doesn't seem to check for a parent class. And I think the rule would apply whether or not the current class extended another or not, and whether or not the getter method was overriding a superclass method or not. If that is true, then are you ok with me renaming the rule (perhaps GetterMethodCouldBeProperty), and perhaps moving into "Groovyism" ruleset (not sure about that, yet)? Chris |
|
From: Chris M. <chr...@ea...> - 2011-11-07 00:05:23
|
I am still working on the "Basic" ruleset reorg. But other than that, I think I am done with the work I intended for v0.16. Any reason to delay a release? Chris |
|
From: Chris M. <chr...@ea...> - 2011-11-05 01:45:11
|
Here is my plan for reorganizing the CodeNarc "Basic" ruleset. This will include adding three new rulesets: 1. "String" - String-related rules 2. "Groovyism" - Groovy idiomatic usage, and Groovy-specific bad practices 3. "Convention" - Coding conventions; not typically errors; might end up become something of a "miscellaneous" set. Current Basic RuleSet and (New Destination) The rules in bold will stay in the Basic ruleset. . AddEmptyString (string) . AssignCollectionSort (groovyism) . AssignCollectionUnique (groovyism) . AssignmentInConditional . BigDecimalInstantiation . BitwiseOperatorInConditional . BooleanGetBoolean . BooleanMethodReturnsNull . BrokenOddnessCheck . ClassForName . CloneableWithoutClone (design) . ClosureAsLastMethodParameter (groovyism) . CollectAllIsDeprecated (groovyism) . CompareToWithoutComparable (design) . ComparisonOfTwoConstants . ComparisonWithSelf . ConfusingMultipleReturns (groovyism) . ConfusingTernary (convention) . ConsecutiveLiteralAppends (string) . ConsecutiveStringConcatenation (string) . ConstantIfExpression . ConstantTernaryExpression . CouldBeElvis (convention) . DeadCode . DoubleNegative . DuplicateCaseStatement . DuplicateMapKey . DuplicateSetValue . EmptyCatchBlock . EmptyElseBlock . EmptyFinallyBlock . EmptyForStatement . EmptyIfStatement . EmptyInstanceInitializer . EmptyMethod . EmptyStaticInitializer . EmptySwitchStatement . EmptySynchronizedStatement . EmptyTryBlock . EmptyWhileStatement . EqualsAndHashCode . EqualsOverloaded . ExplicitArrayListInstantiation (groovyism) . ExplicitCallToAndMethod (groovyism) . ExplicitCallToCompareToMethod (groovyism) . ExplicitCallToDivMethod (groovyism) . ExplicitCallToEqualsMethod (groovyism) . ExplicitCallToGetAtMethod (groovyism) . ExplicitCallToLeftShiftMethod (groovyism) . ExplicitCallToMinusMethod (groovyism) . ExplicitCallToModMethod (groovyism) . ExplicitCallToMultiplyMethod (groovyism) . ExplicitCallToOrMethod (groovyism) . ExplicitCallToPlusMethod (groovyism) . ExplicitCallToPowerMethod (groovyism) . ExplicitCallToRightShiftMethod (groovyism) . ExplicitCallToXorMethod (groovyism) . ExplicitGarbageCollection . ExplicitHashMapInstantiation (groovyism) . ExplicitHashSetInstantiation (groovyism) . ExplicitLinkedHashMapInstantiation (groovyism) . ExplicitLinkedListInstantiation (groovyism) . ExplicitStackInstantiation (groovyism) . ExplicitTreeSetInstantiation (groovyism) . ForLoopShouldBeWhileLoop . GStringAsMapKey (groovyism) . GroovyLangImmutable (groovyism) . HardCodedWindowsFileSeparator . HardcodedWindowsRootDirectory . IntegerGetInteger . InvertedIfElse (convention) . LongLiteralWithLowerCaseL (convention) . RandomDoubleCoercedToZero . RemoveAllOnSelf . ReturnFromFinallyBlock . ReturnsNullInsteadOfEmptyArray (design) . ReturnsNullInsteadOfEmptyCollection (design) . SimpleDateFormatMissingLocale (design) (or convention?) . ThrowExceptionFromFinallyBlock . UseCollectMany (groovyism) . UseCollectNested (groovyism) |
|
From: Chris M. <chr...@ea...> - 2011-11-02 00:56:34
|
Hamlet, For UnusedMethodParameter, I tightened up the logic to ignore main() methods. Through experimentation in both Eclipse and IDEA, I discovered that In Groovy, the main() method does not have to specify void return type or String[] args type. But it must be static, with one parameter. I also renamed the 'categoryClassRegex' property to 'ignoreClassRegex'. If you object, I can undo that. Re: Regex versus wildcard patterns, I still think I prefer the wildcard patterns for lists of names, and regex for specs. But I don't think it is a big deal. I think we can support both approaches, at the risk of some minor inconsistencies. Thanks. Chris -----Original Message----- From: Hamlet DArcy [mailto:ham...@ca...] Sent: Monday, October 31, 2011 1:46 AM To: Chris Mair Cc: cod...@li... Subject: Re: [Codenarc-developer] UnusedMethodParameterRule - ignore main() method I think in this case I would just hard code the rule not to error on "public static void main(String[] args)" methods. If you do add a property, then you might as well make it a regex. That has been my thinking... just make it a regex unless there is a good reason not to. ----- Original Message ----- > > > > > Hamlet, > > > > Running CodeNarc snapshot against my projects at work, I am getting > violations for UnusedMethodParameterRule on main () methods. I think > it should ignore those methods by default. > > > > My first instinct would be to introduce a ignoreMethodNames property > (defaulted to ‘main’) on that rule, consistent with the Naming, Size > and other existing rules. But you seem to prefer regex. Do you have an > opinion in this case? Or I guess we could hard-code it to just ignore > main (). > > > > Chris > ---------------------------------------------------------------------- > -------- Get your Android app more play: Bring it to the BlackBerry > PlayBook in minutes. BlackBerry App World™ now supports > Android™ Apps for the BlackBerry® PlayBook™. Discover > just how easy and simple it is! http://p.sf.net/sfu/android-dev2dev > > _______________________________________________ > Codenarc-developer mailing list > Cod...@li... > https://lists.sourceforge.net/lists/listinfo/codenarc-developer > |
|
From: Hamlet D. <ham...@ca...> - 2011-10-31 05:46:24
|
I think in this case I would just hard code the rule not to error on "public static void main(String[] args)" methods. If you do add a property, then you might as well make it a regex. That has been my thinking... just make it a regex unless there is a good reason not to. ----- Original Message ----- > > > > > Hamlet, > > > > Running CodeNarc snapshot against my projects at work, I am getting > violations for UnusedMethodParameterRule on main () methods. I think > it should ignore those methods by default. > > > > My first instinct would be to introduce a ignoreMethodNames property > (defaulted to ‘main’) on that rule, consistent with the Naming, Size > and other existing rules. But you seem to prefer regex. Do you have > an opinion in this case? Or I guess we could hard-code it to just > ignore main (). > > > > Chris > ------------------------------------------------------------------------------ > Get your Android app more play: Bring it to the BlackBerry PlayBook > in minutes. BlackBerry App World™ now supports Android™ > Apps > for the BlackBerry® PlayBook™. Discover just how easy and > simple > it is! http://p.sf.net/sfu/android-dev2dev > > _______________________________________________ > Codenarc-developer mailing list > Cod...@li... > https://lists.sourceforge.net/lists/listinfo/codenarc-developer > |
|
From: Chris M. <chr...@ea...> - 2011-10-29 01:36:29
|
Hamlet, Running CodeNarc snapshot against my projects at work, I am getting violations for UnusedMethodParameterRule on main() methods. I think it should ignore those methods by default. My first instinct would be to introduce a ignoreMethodNames property (defaulted to 'main') on that rule, consistent with the Naming, Size and other existing rules. But you seem to prefer regex. Do you have an opinion in this case? Or I guess we could hard-code it to just ignore main(). Chris |
|
From: Chris M. <chr...@ea...> - 2011-10-29 01:11:59
|
Hamlet, Just FYI (hopefully). I updated the new UnnecessaryOverridingGetter rule so that it only applies to public getter methods. As you know, of course, it is possible to override a protected method, and replacing that with a field would make that public. Chris |
|
From: Hamlet D. <ham...@ca...> - 2011-10-17 06:12:01
|
It really doesn't matter to me how the rules are reorganized. There are enough new rules with each release that you need to reconfigure things anyway. One option is to print warnings when mis-configurations are found. If you know a rule is moved, then add code to the property file reader that looks for incorrect packages and such. ----- Original Message ----- > > > > > To CodeNarc Users, > > > > I am considering moving a subset of the rules currently in the > CodeNarc “ Basic ” ruleset into one or more other new rulesets, > probably including at least a ruleset encoding Groovy-specific > idioms or best practices. > > > > I do not make this decision lightly. I know this will impact current > users (including me), requiring adjustment of custom CodeNarc > rulesets, if they: > > · Include the “Basic” ruleset and then configured any of the affected > rules within that ruleset closure. > > · Include rules explicitly by class name > > > > My motivations include: > > Ø The “Basic” ruleset has strayed from my original intent of being a > core set of serious and uncontroversial rules. All violations of > these should be fixed. As it is now, that ruleset includes a bunch > of rules that are less-important or are not universal practices. > > Ø The “Basic” ruleset has become large and unwieldy. > > Ø There may be value in having a separate ruleset encoding > Groovy-specific idioms. > > > > That all being said, I’d like to (continue to) reduce the emphasis on > the organization and categorization of the predefined ruleset > anyway. I’d like users to be able to focus more on the rules, not > really caring which rulesets contain them. > > > > Some Basic Rules That Could Be Groovy-Specific (“GroovyIdiom” or > “Groovyism”?) > > • AssignCollectionSort > • AssignCollectionUnique > • ClosureAsLastMethodParameter > • ExplicitArrayListInstantiation > • ExplicitCallToAndMethod > • ExplicitCallToCompareToMethod > • ExplicitCallToDivMethod > • ExplicitCallToEqualsMethod > • ExplicitCallToGetAtMethod > • ExplicitCallToLeftShiftMethod > • ExplicitCallToMinusMethod > • ExplicitCallToModMethod > • ExplicitCallToMultiplyMethod > • ExplicitCallToOrMethod > • ExplicitCallToPlusMethod > • ExplicitCallToPowerMethod > • ExplicitCallToRightShiftMethod > • ExplicitCallToXorMethod > • ExplicitGarbageCollection > • ExplicitHashMapInstantiation > • ExplicitHashSetInstantiation > • ExplicitLinkedHashMapInstantiation > • ExplicitLinkedListInstantiation > • ExplicitStackInstantiation > • ExplicitTreeSetInstantiation > • GStringAsMapKey > • GroovyLangImmutable > > > > Please let me know if you have any opinions one way or another. > > > > Thanks. > > Chris > ------------------------------------------------------------------------------ > All the data continuously generated in your IT infrastructure > contains a > definitive record of customers, application performance, security > threats, fraudulent activity and more. Splunk takes this data and > makes > sense of it. Business sense. IT sense. Common sense. > http://p.sf.net/sfu/splunk-d2d-oct > _______________________________________________ > Codenarc-user mailing list > Cod...@li... > https://lists.sourceforge.net/lists/listinfo/codenarc-user > |
|
From: Chris M. <chr...@ea...> - 2011-10-13 23:57:04
|
To CodeNarc Users, I am considering moving a subset of the rules currently in the CodeNarc Basic ruleset into one or more other new rulesets, probably including at least a ruleset encoding Groovy-specific idioms or best practices. I do not make this decision lightly. I know this will impact current users (including me), requiring adjustment of custom CodeNarc rulesets, if they: · Include the Basic ruleset and then configured any of the affected rules within that ruleset closure. · Include rules explicitly by class name My motivations include: Ø The Basic ruleset has strayed from my original intent of being a core set of serious and uncontroversial rules. All violations of these should be fixed. As it is now, that ruleset includes a bunch of rules that are less-important or are not universal practices. Ø The Basic ruleset has become large and unwieldy. Ø There may be value in having a separate ruleset encoding Groovy-specific idioms. That all being said, Id like to (continue to) reduce the emphasis on the organization and categorization of the predefined ruleset anyway. Id like users to be able to focus more on the rules, not really caring which rulesets contain them. Some Basic Rules That Could Be Groovy-Specific (GroovyIdiom or Groovyism?) * AssignCollectionSort * AssignCollectionUnique * ClosureAsLastMethodParameter * ExplicitArrayListInstantiation * ExplicitCallToAndMethod * ExplicitCallToCompareToMethod * ExplicitCallToDivMethod * ExplicitCallToEqualsMethod * ExplicitCallToGetAtMethod * ExplicitCallToLeftShiftMethod * ExplicitCallToMinusMethod * ExplicitCallToModMethod * ExplicitCallToMultiplyMethod * ExplicitCallToOrMethod * ExplicitCallToPlusMethod * ExplicitCallToPowerMethod * ExplicitCallToRightShiftMethod * ExplicitCallToXorMethod * ExplicitGarbageCollection * ExplicitHashMapInstantiation * ExplicitHashSetInstantiation * ExplicitLinkedHashMapInstantiation * ExplicitLinkedListInstantiation * ExplicitStackInstantiation * ExplicitTreeSetInstantiation * GStringAsMapKey * GroovyLangImmutable Please let me know if you have any opinions one way or another. Thanks. Chris |
|
From: <chr...@we...> - 2011-10-12 11:32:34
|
I'll take care of it. I've just been heads-down on the GMetrics release. -----Original Message----- From: Hamlet DArcy [mailto:ham...@ca...] Sent: Wednesday, October 12, 2011 4:35 AM To: cod...@li... Subject: [Codenarc-developer] Patch: issue 3411724 Hi Chris, Again I have no time to merge this patch. Did you see this patch in the issue tracker? Thanks, ----- Original Message ----- > Feature Requests item #3420629, was opened at 2011-10-08 20:01 > Message generated for change (Comment added) made by joachim-baumann > You can respond by visiting: > https://sourceforge.net/tracker/?func=detail&atid=1126575&aid=3420629&group_id=250145 > > Please note that this message will contain a full copy of the comment > thread, > including the initial issue submission, for this request, > not just the latest update. > Category: None > Group: None > Status: Open > Priority: 5 > Private: No > Submitted By: Joachim Baumann (joachim-baumann) > Assigned to: Nobody/Anonymous (nobody) > Summary: Patch: issue 3411724 > > Initial Comment: > Two rules implementing the two potential problems described in issue > 3411724 regarding the method collectNested{} > > > ---------------------------------------------------------------------- > > >Comment By: Joachim Baumann (joachim-baumann) > Date: 2011-10-09 09:33 > > Message: > I forgot to mention the three assumptions underlying the code for > collectNested-rule regarding multiple collect calls. > 1. The treewalk is depth-first > 2. a whole subtree is walked by one instance of the visitor object > 3. an instance of the visitor object is not used in parallel > > If the first assumption is incorrect, then the algorithm identifying > the > collect statements would not work. > > The other assumptions are with regard to the data held in the visitor > object. If these assumptions were incorrect the variable used to > store the > parameters of the surrounding closure would have to be implemented > differently. > > ---------------------------------------------------------------------- > > You can respond by visiting: > https://sourceforge.net/tracker/?func=detail&atid=1126575&aid=3420629&group_id=250145 > ------------------------------------------------------------------------------ All the data continuously generated in your IT infrastructure contains a definitive record of customers, application performance, security threats, fraudulent activity and more. Splunk takes this data and makes sense of it. Business sense. IT sense. Common sense. http://p.sf.net/sfu/splunk-d2d-oct _______________________________________________ Codenarc-developer mailing list Cod...@li... https://lists.sourceforge.net/lists/listinfo/codenarc-developer |
|
From: Hamlet D. <ham...@ca...> - 2011-10-12 08:35:40
|
Hi Chris, Again I have no time to merge this patch. Did you see this patch in the issue tracker? Thanks, ----- Original Message ----- > Feature Requests item #3420629, was opened at 2011-10-08 20:01 > Message generated for change (Comment added) made by joachim-baumann > You can respond by visiting: > https://sourceforge.net/tracker/?func=detail&atid=1126575&aid=3420629&group_id=250145 > > Please note that this message will contain a full copy of the comment > thread, > including the initial issue submission, for this request, > not just the latest update. > Category: None > Group: None > Status: Open > Priority: 5 > Private: No > Submitted By: Joachim Baumann (joachim-baumann) > Assigned to: Nobody/Anonymous (nobody) > Summary: Patch: issue 3411724 > > Initial Comment: > Two rules implementing the two potential problems described in issue > 3411724 regarding the method collectNested{} > > > ---------------------------------------------------------------------- > > >Comment By: Joachim Baumann (joachim-baumann) > Date: 2011-10-09 09:33 > > Message: > I forgot to mention the three assumptions underlying the code for > collectNested-rule regarding multiple collect calls. > 1. The treewalk is depth-first > 2. a whole subtree is walked by one instance of the visitor object > 3. an instance of the visitor object is not used in parallel > > If the first assumption is incorrect, then the algorithm identifying > the > collect statements would not work. > > The other assumptions are with regard to the data held in the visitor > object. If these assumptions were incorrect the variable used to > store the > parameters of the surrounding closure would have to be implemented > differently. > > ---------------------------------------------------------------------- > > You can respond by visiting: > https://sourceforge.net/tracker/?func=detail&atid=1126575&aid=3420629&group_id=250145 > |
|
From: Chris M. <chr...@ea...> - 2011-10-08 00:01:22
|
Done. -----Original Message----- From: Hamlet DArcy [mailto:ham...@ca...] Sent: Friday, October 07, 2011 12:47 AM To: cod...@li... Subject: [Codenarc-developer] patch for codenarc Hi Chris, Can you merge this patch that was submitted? https://sourceforge.net/tracker/?func=detail&atid=1126575&aid=3417093&group_ id=250145 I have no time at all right now... :( -- Hamlet D'Arcy ham...@ca... ---------------------------------------------------------------------------- -- All of the data generated in your IT infrastructure is seriously valuable. Why? It contains a definitive record of application performance, security threats, fraudulent activity, and more. Splunk takes this data and makes sense of it. IT sense. And common sense. http://p.sf.net/sfu/splunk-d2dcopy2 _______________________________________________ Codenarc-developer mailing list Cod...@li... https://lists.sourceforge.net/lists/listinfo/codenarc-developer |
|
From: Hamlet D. <ham...@ca...> - 2011-10-07 04:46:49
|
Hi Chris, Can you merge this patch that was submitted? https://sourceforge.net/tracker/?func=detail&atid=1126575&aid=3417093&group_id=250145 I have no time at all right now... :( -- Hamlet D'Arcy ham...@ca... |
|
From: Chris M. <chr...@ea...> - 2011-09-24 14:15:44
|
Hamlet, I have checked in the RunCodeNarcAgainstGrails class. Note that it is a Groovy/Java app rather than a test case. You must set the "grails.home" system property within the Run Configuration to run from IDEA. This is the same ruleset that I used for the sample Grails/Gradle reports. Feel free to tweak as needed. Once we get it the way we want it, I will probably do something similar for running against Gradle and then use those to generate the sample reports for the online docs. The (very) informal performance testing that I did was to run against both Grails and Gradle source trees -- one top-level Ant task that called two sub-tasks. As I mentioned above, my plan is to also create a similar class to run against Gradle source. But if providing a combined script helps you with relative performance testing, we certainly can create that, too. I was testing against version 1.3.7 of Grails. I downloaded 2.0.0.M2 (zip file) but that does not seem to contain the test source. What is the recommended way to get ALL of the source now? Git, I assume. If it is easy for you to paste in the Git command, that might help me get started Chris -----Original Message----- From: Hamlet DArcy [mailto:ham...@ca...] Sent: Monday, September 19, 2011 8:40 AM To: chris mair Cc: Cod...@li... Subject: Re: [Codenarc-developer] AstUtils rewrite - can you test against Grails? The RunCodeNarcAgainstCodeNarc test is really useful, because I can make my changes and then run YourKit through IDEA for performance benchmarks. It's slick because YourKit allows you to jump back and forth between IDE and the benchmarks. All we'd need to do is clone that test, point it at the Grails source on disk (which we both have), share your codenarc.properties file, and then run it as a unit test. Seems simple, and a more real-world benchmark, even if we alos set the test to @Ignore or comment it out. I wouldn't bother pointing CodeNarc at an SVN repo, unless it were integrated into the CodeNarc web console. But then, it's not a useful tool so much as a gateway to demonstrate the power of CodeNarc. I think our efforts are still better suited towards expanding the ruleset. By the way, the GMetrics rules are the biggest bottleneck right now. I'm estimating a 15% or higher performance improvement if we rewrite the GMetrics AstUtils. The other feature I'd really like is a CodeNarc properties visual editor. You could run it from the command line or through the Grails plugin. It could pop open a Swing UI that looks like the IDEA inspections UI. That would be a nice way to edit rule. But I'd want the feature to use only core JDK/Groovy classes so we don't introduce extra dependencies. -- Hamlet ----- Original Message ----- > Hamlet, > > I will rebuild and run against Grails+Gradle again. > > In my current setup, I build a CodeNarc jar (mvn package) and copy > that jar to a separate directory where I already have the other jars > (GMetrics and Log4J) and a bat file to execute it. I point it at a > locally-installed instance of the Grails source (and also Gradle > source). This was not really (originally) intended as a performance > benchmark, but more as a way to generate the sample reports and also > be a regression test at the same time. > > Running CodeNarc against a Git or SVN repo, or against a jar/zip file > for that matter, are still only potential future features. Does > automating any of the above make sense as a unit test? > > Chris > -----Original Message----- > From: Hamlet D'Arcy [mailto:ham...@gm...] > Sent: Monday, September 19, 2011 7:11 AM > To: Cod...@li... > Subject: [Codenarc-developer] AstUtils rewrite - can you test against > Grails? > > Hi Chris, > > I finally got my merge issues resolved and checked in. > > I rewrite AstUtil.groovy to be AstUtil.java. It gave me less > performance improvements than I expected on my test run. I am seeing > about 4% improvement total, which is not very much, IMO. > > Can you try running the latest against Grails? How are you doing that? > I propose that we add a unit test to the > RunCodeNarcAgainstCodeNarcTest that runs the code against the Grails > source. We can comment out the test so it never runs with the build, > but then at least we share the same benchmark? My current benchmark is > just running CodeNarc against CodeNarc. What do you think? > > > -- > Hamlet D'Arcy > ham...@gm... > > ---------------------------------------------------------------------- > -------- BlackBerry® DevCon Americas, Oct. 18-20, San Francisco, > CA Learn about the latest advances in developing for the > BlackBerry® mobile platform with sessions, labs & more. > See new tools and technologies. Register for BlackBerry® DevCon > today! > http://p.sf.net/sfu/rim-devcon-copy1 > _______________________________________________ > Codenarc-developer mailing list > Cod...@li... > https://lists.sourceforge.net/lists/listinfo/codenarc-developer > > ---------------------------------------------------------------------- > -------- BlackBerry® DevCon Americas, Oct. 18-20, San Francisco, > CA Learn about the latest advances in developing for the > BlackBerry® mobile platform with sessions, labs & more. > See new tools and technologies. Register for BlackBerry® DevCon > today! > http://p.sf.net/sfu/rim-devcon-copy1 > _______________________________________________ > Codenarc-developer mailing list > Cod...@li... > https://lists.sourceforge.net/lists/listinfo/codenarc-developer > ---------------------------------------------------------------------------- -- BlackBerry® DevCon Americas, Oct. 18-20, San Francisco, CA Learn about the latest advances in developing for the BlackBerry® mobile platform with sessions, labs & more. See new tools and technologies. Register for BlackBerry® DevCon today! http://p.sf.net/sfu/rim-devcon-copy1 _______________________________________________ Codenarc-developer mailing list Cod...@li... https://lists.sourceforge.net/lists/listinfo/codenarc-developer |
|
From: Chris M. <chr...@ea...> - 2011-09-20 01:52:50
|
Hamlet, After pulling in your latest changes, I am seeing some significant performance improvements. Run times for Grails + Gradle source, with my sample set of 178 rules enabled, the average runtime went down to ~ 49 seconds (down from ~ 70sec). Very nice work! -----Original Message----- From: Chris Mair [mailto:chr...@ea...] Sent: Monday, September 12, 2011 10:26 PM To: 'Hamlet D'Arcy'; 'Cod...@li...' Subject: RE: [Codenarc-developer] More performance improvements - options? After I pulled in your most recent changes... Run times for Grails + Gradle source, with my sample set of 178 rules enabled, the average runtime went down to ~ 70 seconds (down from ~ 75sec). This is only from 13 runs, so not at all scientific. I also noticed a difference in timings if I had IDEA running in the background, for what that's worth. Chris -----Original Message----- From: Hamlet D'Arcy [mailto:ham...@gm...] Sent: Monday, September 12, 2011 2:24 AM To: Cod...@li... Subject: Re: [Codenarc-developer] More performance improvements - options? Hi Chris, Can you run your benchmark again with the latest codebase? I'd like to see how these changes affect performance. Locally, I see an 8-10% improvement from this change, but I think it will be much higher in a real-world scenario. Thanks, On Sun, Sep 11, 2011 at 10:52 PM, Chris Mair <chr...@ea...> wrote: > Hamlet, > > I like the getMethodCallExpressions() option. I would prefer to keep > the rules independent, and adding that method to SourceCode > interface/impl sounds like a reasonable and clean way to do that. And, > as you say, we could use that same pattern for other node types as well, if needed. Sounds good. > > Chris > -----Original Message----- > From: Hamlet D'Arcy [mailto:ham...@gm...] > Sent: Sunday, September 11, 2011 4:06 PM > To: Cod...@li... > Subject: [Codenarc-developer] More performance improvements - options? > > Hi everyone/Chris, > > I have an idea for more performance improvements that I believe will > make another big difference in how CodeNarc performs. > > There are many, many rules that look at only MethodCallExpressions. > Each of these rules (about 50 of them), walks the AST individually and > searches out these MethodCallExpressions. Most the time of these rules > is spent in just walking the tree, which is 95% redundant processing > for all the rules. It would be better if the tree were walked once, > all the MethodCallExpressions were collected, and then each of the 50 > rules called with just the relevant visitMethodCallExpression without > re-walking the entire tree. > > One idea to implement this is to create a CompositeRule that has a > list of other rules. Then the composite walks the tree once and calls > the other > (children) rules when encountered. The problem with this is that a lot > of logic works off rule names, and the composite has many rule names. > I don't see a clean way to implement this. > > Another idea is to create a parent class that all the MethodCall > visitors could extend. Then that parent class could, in a static > block, cache all of the MethodCallExpressions and avoid walking the > tree redundantly. But this is not clean because each rule is run many > times, once for each SourceCode object (a source file). It's complex > to implement, has to worry about multithreading, and uses static > fields on instance objects. I just don't like it. > > My last idea, which I think is the best solution, is to add a new > method to the SourceCode interface. SourceCode#getAst exists today. I > propose we add SourceCode#getMethodCallExpressions. Then for each > SourceCode object we would only walk the tree once and collect useful > information about the tree, and rules could get the information when > they need it. This is pretty clean to implement. The lifecycle of the > cache has the same lifecycle as the source file, which makes sense. It > would still need to be multithread safe, but that doesn't sound hard. > I would write this method on SourceCode: > Map<ClassNode, MethodCallExpression> getMethodCallExpressions() > > There are other performance enhancements like this as well. I think > PropertyExpressions have many rules. Then as we want to optimize we > could just add new methods onto the interface. > > What do you think of these approaches? > > -- > Hamlet D'Arcy > ham...@gm... > > ---------------------------------------------------------------------- > ------ > -- > Using storage to extend the benefits of virtualization and iSCSI > Virtualization increases hardware utilization and delivers a new level > of agility. Learn what those decisions are and how to modernize your > storage and backup environments for virtualization. > http://www.accelacomm.com/jaw/sfnl/114/51434361/ > _______________________________________________ > Codenarc-developer mailing list > Cod...@li... > https://lists.sourceforge.net/lists/listinfo/codenarc-developer > > -- Hamlet D'Arcy ham...@gm... ---------------------------------------------------------------------------- -- Doing More with Less: The Next Generation Virtual Desktop What are the key obstacles that have prevented many mid-market businesses from deploying virtual desktops? How do next-generation virtual desktops provide companies an easier-to-deploy, easier-to-manage and more affordable virtual desktop model.http://www.accelacomm.com/jaw/sfnl/114/51426474/ _______________________________________________ Codenarc-developer mailing list Cod...@li... https://lists.sourceforge.net/lists/listinfo/codenarc-developer |
|
From: Hamlet D. <ham...@ca...> - 2011-09-19 12:40:27
|
The RunCodeNarcAgainstCodeNarc test is really useful, because I can make my changes and then run YourKit through IDEA for performance benchmarks. It's slick because YourKit allows you to jump back and forth between IDE and the benchmarks. All we'd need to do is clone that test, point it at the Grails source on disk (which we both have), share your codenarc.properties file, and then run it as a unit test. Seems simple, and a more real-world benchmark, even if we alos set the test to @Ignore or comment it out. I wouldn't bother pointing CodeNarc at an SVN repo, unless it were integrated into the CodeNarc web console. But then, it's not a useful tool so much as a gateway to demonstrate the power of CodeNarc. I think our efforts are still better suited towards expanding the ruleset. By the way, the GMetrics rules are the biggest bottleneck right now. I'm estimating a 15% or higher performance improvement if we rewrite the GMetrics AstUtils. The other feature I'd really like is a CodeNarc properties visual editor. You could run it from the command line or through the Grails plugin. It could pop open a Swing UI that looks like the IDEA inspections UI. That would be a nice way to edit rule. But I'd want the feature to use only core JDK/Groovy classes so we don't introduce extra dependencies. -- Hamlet ----- Original Message ----- > Hamlet, > > I will rebuild and run against Grails+Gradle again. > > In my current setup, I build a CodeNarc jar (mvn package) and copy > that jar to a separate directory where I already have the other jars > (GMetrics and Log4J) and a bat file to execute it. I point it at a > locally-installed instance of the Grails source (and also Gradle > source). This was not really (originally) intended as a performance > benchmark, but more as a way to generate the sample reports and also > be a regression test at the same time. > > Running CodeNarc against a Git or SVN repo, or against a jar/zip file > for that matter, are still only potential future features. Does > automating any of the above make sense as a unit test? > > Chris > -----Original Message----- > From: Hamlet D'Arcy [mailto:ham...@gm...] > Sent: Monday, September 19, 2011 7:11 AM > To: Cod...@li... > Subject: [Codenarc-developer] AstUtils rewrite - can you test against > Grails? > > Hi Chris, > > I finally got my merge issues resolved and checked in. > > I rewrite AstUtil.groovy to be AstUtil.java. It gave me less > performance improvements than I expected on my test run. I am seeing > about 4% improvement total, which is not very much, IMO. > > Can you try running the latest against Grails? How are you doing > that? > I propose that we add a unit test to the > RunCodeNarcAgainstCodeNarcTest that runs the code against the Grails > source. We can comment out the test so it never runs with the build, > but then at least we share the same benchmark? My current benchmark > is > just running CodeNarc against CodeNarc. What do you think? > > > -- > Hamlet D'Arcy > ham...@gm... > > ------------------------------------------------------------------------------ > BlackBerry® DevCon Americas, Oct. 18-20, San Francisco, CA > Learn about the latest advances in developing for the > BlackBerry® mobile platform with sessions, labs & more. > See new tools and technologies. Register for BlackBerry® DevCon > today! > http://p.sf.net/sfu/rim-devcon-copy1 > _______________________________________________ > Codenarc-developer mailing list > Cod...@li... > https://lists.sourceforge.net/lists/listinfo/codenarc-developer > > ------------------------------------------------------------------------------ > BlackBerry® DevCon Americas, Oct. 18-20, San Francisco, CA > Learn about the latest advances in developing for the > BlackBerry® mobile platform with sessions, labs & more. > See new tools and technologies. Register for BlackBerry® DevCon > today! > http://p.sf.net/sfu/rim-devcon-copy1 > _______________________________________________ > Codenarc-developer mailing list > Cod...@li... > https://lists.sourceforge.net/lists/listinfo/codenarc-developer > |
|
From: <chr...@we...> - 2011-09-19 12:30:30
|
>> I suspect a CodeNarc rule that checks that this kind of 'coercion' isn't
>> normally going on outside builder code might be the right short-term answer.
Something to consider.
-----Original Message-----
From: Paul King [mailto:pau...@gm...] On Behalf Of Paul King
Sent: Monday, September 19, 2011 8:20 AM
To: us...@gr...
Subject: Re: [groovy-user] Violation of the Principle of Least Surprise?
On 19/09/2011 9:05 PM, Russel Winder wrote:
> There may be consistency of internal compiler rule application, but this
> is a far cry from programmers being able to reason about their programs
> given the code in front of them. The fact that there is an automated
> ['a'] -> 'a' transform seems to me like a completely wrong rule — even
> given that this is a dynamic language.
Agreed it looks weird and good to know that Jochen has something in mind
for 2.0 but just to try to give some clarification on why the behavior
is there, it isn't an arbitrary replacement of a collection by its first
element. It is typed argument matching going on. Java users might sometimes
be surprised (a little less often perhaps) when using varargs in Java.
Groovy is just letting a List be used instead of Object[] which offers
a lot of convenience in some places (e.g. the SwingBuilder case already
mentioned) but can be a little bit more surprising. Perhaps the example
below helps (not sure):
def countArgs(a) { 1 }
def countArgs(a, b) { 2 }
def countArgs(a, b, c) { 3 }
assert countArgs(11) == 1
assert countArgs(11, 22) == 2
assert countArgs(11, 22, 33) == 3
assert countArgs([11, 22]) == 1 // list matches untyped 'a' 1st method
assert countArgs(*[11, 22]) == 2 // explicit spread matches 2nd method
def countIntArgs(int a, int b) { 2 }
assert countIntArgs([11, 22]) == 2 // no List or Object version but
// there is an (int, int) version so
// explicitspread not required
assert countIntArgs(*[11, 22]) == 2 // you can still do explicit spread
I suspect a CodeNarc rule that checks that this kind of 'coercion' isn't
normally going on outside builder code might be the right short-term answer.
Cheers, Paul.
---------------------------------------------------------------------
To unsubscribe from this list, please visit:
http://xircles.codehaus.org/manage_email
|
|
From: <chr...@we...> - 2011-09-19 12:25:01
|
Hamlet, I will rebuild and run against Grails+Gradle again. In my current setup, I build a CodeNarc jar (mvn package) and copy that jar to a separate directory where I already have the other jars (GMetrics and Log4J) and a bat file to execute it. I point it at a locally-installed instance of the Grails source (and also Gradle source). This was not really (originally) intended as a performance benchmark, but more as a way to generate the sample reports and also be a regression test at the same time. Running CodeNarc against a Git or SVN repo, or against a jar/zip file for that matter, are still only potential future features. Does automating any of the above make sense as a unit test? Chris -----Original Message----- From: Hamlet D'Arcy [mailto:ham...@gm...] Sent: Monday, September 19, 2011 7:11 AM To: Cod...@li... Subject: [Codenarc-developer] AstUtils rewrite - can you test against Grails? Hi Chris, I finally got my merge issues resolved and checked in. I rewrite AstUtil.groovy to be AstUtil.java. It gave me less performance improvements than I expected on my test run. I am seeing about 4% improvement total, which is not very much, IMO. Can you try running the latest against Grails? How are you doing that? I propose that we add a unit test to the RunCodeNarcAgainstCodeNarcTest that runs the code against the Grails source. We can comment out the test so it never runs with the build, but then at least we share the same benchmark? My current benchmark is just running CodeNarc against CodeNarc. What do you think? -- Hamlet D'Arcy ham...@gm... ------------------------------------------------------------------------------ BlackBerry® DevCon Americas, Oct. 18-20, San Francisco, CA Learn about the latest advances in developing for the BlackBerry® mobile platform with sessions, labs & more. See new tools and technologies. Register for BlackBerry® DevCon today! http://p.sf.net/sfu/rim-devcon-copy1 _______________________________________________ Codenarc-developer mailing list Cod...@li... https://lists.sourceforge.net/lists/listinfo/codenarc-developer |
|
From: Hamlet D'A. <ham...@gm...> - 2011-09-19 11:10:51
|
Hi Chris, I finally got my merge issues resolved and checked in. I rewrite AstUtil.groovy to be AstUtil.java. It gave me less performance improvements than I expected on my test run. I am seeing about 4% improvement total, which is not very much, IMO. Can you try running the latest against Grails? How are you doing that? I propose that we add a unit test to the RunCodeNarcAgainstCodeNarcTest that runs the code against the Grails source. We can comment out the test so it never runs with the build, but then at least we share the same benchmark? My current benchmark is just running CodeNarc against CodeNarc. What do you think? -- Hamlet D'Arcy ham...@gm... |
|
From: Chris M. <chr...@ea...> - 2011-09-17 12:30:54
|
First of all, thanks very much for all of your effort (and results!) on
improving performance.
Regarding this specific issue, I have not seen any replies yet from the
Groovy gurus. Unless they come up with something unexpectedly helpful, I'd
be open to #1 (object pooling), but I think that would also require some
standard API and extra code to clear out ASTVisitor instances so that they
can be reused. And it would also have to support concurrent access.
I'd probably lean toward #5 (leave it the way it is). I'm also wondering
whether this is one of those problems that might get better through eventual
performance improvements in the Groovy core.
Chris
-----Original Message-----
From: Hamlet DArcy [mailto:ham...@ca...]
Sent: Friday, September 16, 2011 2:47 AM
To: cod...@li...
Subject: [Codenarc-developer] Perf Boost - Lock Contention in Class.forName
Hi Chris,
If you look at CodeNarc in a perf analyzer, then you'll see a few cases of
threads blocking calling the synchronized Class.forName method. On average,
I see about 9 seconds of blocking across three threads for every ~37 second
run. If we could remove this synchronization then we could get /maybe/
another 8-10% improvement. The problem is in the AST Visitor Rules that have
a Class field like this:
Class astVisitorClass = SomeClass
The parent rule reads this field and instantiates the visitor once for every
source file. But the slowness does not come from the reflection of
Class.newInstance. I changed the rules to all create a instance directly
like this, and saw no improvement:
@Override AstVisitor getAstVisitor() { new SomeClass() }
The problem is that Groovy uses Class.forName even when you directly invoke
a constructor call like this. There seems is no way to avoid the reflection
in Groovy. I'll check the mailing list to see if there is any easy solution,
but here is what I see as possible solutions:
1 Use object pooling - Don't create new rule instances, cache them instead
2 Rewrite every rule in Java
3 Write an ASTtransformation annotation that converts these statements to
the correct bytecode
4 Use ASM (or something) to change the bytecode at compile time
5 Leave it as is
I don't know any other good ways to do it. Option 1 is not so bad, Option 2
is unappealing, Option 3 is fragile.
Thoughts?
--
Hamlet D'Arcy
ham...@ca...
----------------------------------------------------------------------------
--
BlackBerry® DevCon Americas, Oct. 18-20, San Francisco, CA
http://p.sf.net/sfu/rim-devcon-copy2
_______________________________________________
Codenarc-developer mailing list
Cod...@li...
https://lists.sourceforge.net/lists/listinfo/codenarc-developer
|
|
From: Hamlet D. <ham...@ca...> - 2011-09-16 06:47:33
|
Hi Chris,
If you look at CodeNarc in a perf analyzer, then you'll see a few cases of threads blocking calling the synchronized Class.forName method. On average, I see about 9 seconds of blocking across three threads for every ~37 second run. If we could remove this synchronization then we could get /maybe/ another 8-10% improvement. The problem is in the AST Visitor Rules that have a Class field like this:
Class astVisitorClass = SomeClass
The parent rule reads this field and instantiates the visitor once for every source file. But the slowness does not come from the reflection of Class.newInstance. I changed the rules to all create a instance directly like this, and saw no improvement:
@Override AstVisitor getAstVisitor() { new SomeClass() }
The problem is that Groovy uses Class.forName even when you directly invoke a constructor call like this. There seems is no way to avoid the reflection in Groovy. I'll check the mailing list to see if there is any easy solution, but here is what I see as possible solutions:
1 Use object pooling - Don't create new rule instances, cache them instead
2 Rewrite every rule in Java
3 Write an ASTtransformation annotation that converts these statements to the correct bytecode
4 Use ASM (or something) to change the bytecode at compile time
5 Leave it as is
I don't know any other good ways to do it. Option 1 is not so bad, Option 2 is unappealing, Option 3 is fragile.
Thoughts?
--
Hamlet D'Arcy
ham...@ca...
|
|
From: Hamlet D. <ham...@ca...> - 2011-09-16 06:22:26
|
I would delay the next release for the fix to GMetrics. It's another 8-9% performance boost.
In my mind, I've been thinking that October would bring the new release. So it shouldn't be hard to push out the release of GMetrics.
----- Original Message -----
> Hamlet,
>
> Sounds good. I will put that at the top of my GMetrics to-do's. Would
> you
> want to delay the next CodeNarc release to wait for that?
>
> Chris
> -----Original Message-----
> From: Hamlet DArcy [mailto:ham...@ca...]
> Sent: Thursday, September 15, 2011 3:17 AM
> To: cod...@li...
> Subject: [Codenarc-developer] GMetrics ASTUtils needs to change
>
> Hi Chris,
>
> One of the bigger bottlenecks in CodeNarc is now the GMetrics#ASTUtil
> getVariableExpressions class. If you could change it to be
> implemented
> *exactly* the same way as CodeNarc, then we would shave about 8-10%
> off the
> total times.
>
> Here is how the method should be implemented:
>
> static List getVariableExpressions(DeclarationExpression
> declarationExpression) {
> def leftExpression = declarationExpression.leftExpression
>
> // !important: performance enhancement
> if (leftExpression instanceof ArrayExpression) {
> leftExpression.expressions ?: [leftExpression]
> } else if (leftExpression instanceof ListExpression) {
> leftExpression.expressions ?: [leftExpression]
> } else if (leftExpression instanceof TupleExpression) {
> leftExpression.expressions ?: [leftExpression]
> } else if (leftExpression instanceof VariableExpression) {
> [leftExpression]
> } else {
> leftExpression.properties['expressions'] ?:
> [leftExpression]
> }
> }
>
>
> Basically, the last else block should *never* be called. If we ever
> find
> that it is called then we need to add a new if branch to avoid that.
> Hmmm...
> perhaps we should add a LOG.warning there?
>
> --
> Hamlet D'Arcy
> ham...@ca...
>
>
> ----------------------------------------------------------------------------
> --
> Doing More with Less: The Next Generation Virtual Desktop What are
> the key
> obstacles that have prevented many mid-market businesses
> from deploying virtual desktops? How do next-generation virtual
> desktops
> provide companies an easier-to-deploy, easier-to-manage and more
> affordable
> virtual desktop
> model.http://www.accelacomm.com/jaw/sfnl/114/51426474/
> _______________________________________________
> Codenarc-developer mailing list
> Cod...@li...
> https://lists.sourceforge.net/lists/listinfo/codenarc-developer
>
>
|
|
From: Chris M. <chr...@ea...> - 2011-09-16 00:09:44
|
Hamlet,
Sounds good. I will put that at the top of my GMetrics to-do's. Would you
want to delay the next CodeNarc release to wait for that?
Chris
-----Original Message-----
From: Hamlet DArcy [mailto:ham...@ca...]
Sent: Thursday, September 15, 2011 3:17 AM
To: cod...@li...
Subject: [Codenarc-developer] GMetrics ASTUtils needs to change
Hi Chris,
One of the bigger bottlenecks in CodeNarc is now the GMetrics#ASTUtil
getVariableExpressions class. If you could change it to be implemented
*exactly* the same way as CodeNarc, then we would shave about 8-10% off the
total times.
Here is how the method should be implemented:
static List getVariableExpressions(DeclarationExpression
declarationExpression) {
def leftExpression = declarationExpression.leftExpression
// !important: performance enhancement
if (leftExpression instanceof ArrayExpression) {
leftExpression.expressions ?: [leftExpression]
} else if (leftExpression instanceof ListExpression) {
leftExpression.expressions ?: [leftExpression]
} else if (leftExpression instanceof TupleExpression) {
leftExpression.expressions ?: [leftExpression]
} else if (leftExpression instanceof VariableExpression) {
[leftExpression]
} else {
leftExpression.properties['expressions'] ?: [leftExpression]
}
}
Basically, the last else block should *never* be called. If we ever find
that it is called then we need to add a new if branch to avoid that. Hmmm...
perhaps we should add a LOG.warning there?
--
Hamlet D'Arcy
ham...@ca...
----------------------------------------------------------------------------
--
Doing More with Less: The Next Generation Virtual Desktop What are the key
obstacles that have prevented many mid-market businesses
from deploying virtual desktops? How do next-generation virtual desktops
provide companies an easier-to-deploy, easier-to-manage and more affordable
virtual desktop model.http://www.accelacomm.com/jaw/sfnl/114/51426474/
_______________________________________________
Codenarc-developer mailing list
Cod...@li...
https://lists.sourceforge.net/lists/listinfo/codenarc-developer
|
|
From: Hamlet D. <ham...@ca...> - 2011-09-15 07:17:14
|
Hi Chris,
One of the bigger bottlenecks in CodeNarc is now the GMetrics#ASTUtil getVariableExpressions class. If you could change it to be implemented *exactly* the same way as CodeNarc, then we would shave about 8-10% off the total times.
Here is how the method should be implemented:
static List getVariableExpressions(DeclarationExpression declarationExpression) {
def leftExpression = declarationExpression.leftExpression
// !important: performance enhancement
if (leftExpression instanceof ArrayExpression) {
leftExpression.expressions ?: [leftExpression]
} else if (leftExpression instanceof ListExpression) {
leftExpression.expressions ?: [leftExpression]
} else if (leftExpression instanceof TupleExpression) {
leftExpression.expressions ?: [leftExpression]
} else if (leftExpression instanceof VariableExpression) {
[leftExpression]
} else {
leftExpression.properties['expressions'] ?: [leftExpression]
}
}
Basically, the last else block should *never* be called. If we ever find that it is called then we need to add a new if branch to avoid that. Hmmm... perhaps we should add a LOG.warning there?
--
Hamlet D'Arcy
ham...@ca...
|
|
From: Hamlet D. <ham...@ca...> - 2011-09-13 06:34:28
|
Hi Chris, I refactored AntSourceAnalyzer so that only one thread pool is created by it. Previously, a new threadpool was created for each FileSet. Now the same threadpool is reused throughout. Also, I rewrote the class in Java because it used a lot of unnecessary closures. The RunCodeNarcAgainstCodeNarc uses 2 FileSets. It went from ~48 seconds to ~39. This is good, but I don't know how much RunCodeNarcAgainstCodeNarc represents real usage. Hopefully quite well! Anyway, my next performance idea is to rewrite AbstractRule in Java. There are a lot of instances of this class in our system, and perhaps it will help. My performance metrics aren't giving me a lot of clues at this point. Too much time is still spent in Groovy method dispatch, but it's not clear what can be rewritten in Java to help, besides rewriting all of the rule classes. Thanks, -- Hamlet D'Arcy ham...@ca... |
