[ 
https://issues.apache.org/jira/browse/FLEX-33273?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13570921#comment-13570921
 ] 

Justin Mclean commented on FLEX-33273:
--------------------------------------

Changes are made in the develop branch not trunk. It will show up in trunk only 
after a release is made.

You can see the change here.
http://svn.apache.org/viewvc/incubator/flex/sdk/branches/develop/frameworks/projects/framework/src/mx/styles/CSSCondition.as?view=log&pathrev=1420855
                
> CSSCondition.matchesStyleClient() is slow and creates excessive garbage
> -----------------------------------------------------------------------
>
>                 Key: FLEX-33273
>                 URL: https://issues.apache.org/jira/browse/FLEX-33273
>             Project: Apache Flex
>          Issue Type: Improvement
>          Components: Styles
>    Affects Versions: Adobe Flex SDK 4.1 (Release), Adobe Flex SDK 4.5 
> (Release), Adobe Flex SDK 4.5.1 (Release), Adobe Flex SDK 4.6 (Release)
>            Reporter: Maurice Nicholson
>            Assignee: Frédéric THOMAS
>              Labels: patch, performance
>         Attachments: FLEX-33273.patch, PerformanceTest.zip
>
>
> CSSCondition.matchesStyleClient() is called *very* frequently during layout 
> of components in many different scenarios.
> I've done a lot of profiling of Flex apps and it comes up again and again.
> The current implementation is slow and creates unnecessary garbage (which 
> slows down the runtime further, since it needs to collect the garbage instead 
> of doing more useful things).
> Here is the current implementation:
> {code}
>     public function matchesStyleClient(object:IAdvancedStyleClient):Boolean
>     {
>         var match:Boolean = false;
>         if (kind == CSSConditionKind.CLASS)
>         {
>             if (object.styleName != null && object.styleName is String)
>             {
>                 // Look for a match in a potential list of styleNames 
>                 var styleNames:Array = object.styleName.split(/\s+/);
>                 for (var i:uint = 0; i < styleNames.length; i++)
>                 {
>                     if (styleNames[i] == value)
>                     {
>                         match = true;
>                         break;
>                     }
>                 }
>             }
>         }
>         else if (kind == CSSConditionKind.ID)
>         {
>             if (object.id == value)
>                 match = true;
>         }
>         else if (kind == CSSConditionKind.PSEUDO)
>         {
>             if (object.matchesCSSState(value))
>                 match = true;
>         }
>         return match;
>     }
> {code}
> Here is what I suggest instead:
> {code}
>     public function matchesStyleClient(object:IAdvancedStyleClient):Boolean
>     {
>         var match:Boolean = false;
>         if (kind == CSSConditionKind.CLASS)
>         {
>             const styleName:String = object.styleName as String;
>             if (styleName)
>             {
>                 // Look for a match in a potential list of styleNames 
>                 FIND:
>                 {
>                     var index:int = styleName.indexOf(value);
>                     if (index == -1)
>                     {
>                         break FIND;
>                     }
>                     if (index != 0 && styleName.charAt(index - 1) != ' ')
>                     {
>                         break FIND;
>                     }
>                     const next:int = index + value.length;
>                     if (next != styleName.length && styleName.charAt(next) != 
> ' ')
>                     {
>                         break FIND;
>                     }
>                     match = true;
>                 }
>             }
>         }
>         else if (kind == CSSConditionKind.ID)
>         {
>             if (object.id == value)
>                 match = true;
>         }
>         else if (kind == CSSConditionKind.PSEUDO)
>         {
>             if (object.matchesCSSState(value))
>                 match = true;
>         }
>         return match;
>     }
> {code}
> There are obviously more concise ways to express this code, but the above 
> seems to match the current style more or less.
> Here is the output from a benchmark I created using Grant Skinner's 
> PerformanceTest v2 Beta, comparing the current version and the suggested 
> version:
> {noformat}
> Comparing speed of matching a string in space delimited string. 1000000 loops.
> Test                                                           Old ms   New 
> ms Speedup  Old mem  New mem  Change
> matchesStyleClient(styleName: "foo", value: "foo")            1006.00   
> 181.00   -82.0   123.00     0.00  -100.0
> matchesStyleClient(styleName: "foo bar", value: "foo")        2107.00   
> 206.80   -90.2   115.00     0.00  -100.0
> matchesStyleClient(styleName: "bar foo", value: "foo")        2099.80   
> 232.30   -88.9   117.00     0.00  -100.0
> matchesStyleClient(styleName: "baz foo bar", value: "foo")    3193.80   
> 251.30   -92.1   119.00     0.00  -100.0
> matchesStyleClient(styleName: "foobar", value: "foo")         1169.50   
> 192.00   -83.6   112.00     0.00  -100.0
> matchesStyleClient(styleName: "foofoo bar", value: "foo")     2273.80   
> 191.30   -91.6   116.00     0.00  -100.0
> matchesStyleClient(styleName: "fo", value: "foo")              918.80   
> 141.00   -84.7   108.00     0.00  -100.0
> matchesStyleClient(styleName: "fooo", value: "foo")           1052.80   
> 192.00   -81.8   108.00     0.00  -100.0
> matchesStyleClient(styleName: "2foo bar", value: "foo")       2149.50   
> 205.30   -90.4   116.00     0.00  -100.0
> matchesStyleClient(styleName: "foo2 bar", value: "foo")       3849.50   
> 190.80   -95.0   111.00     0.00  -100.0
> matchesStyleClient(styleName: "", value: "foo")               1801.80   
> 141.00   -92.2   132.00     0.00  -100.0
> {noformat}
> As you can see, the new version doesn't create garbage, and is at least 80% 
> faster in the above test cases.
> I would be happy to contribute a patch and a FlexUnit test, but I haven't 
> seen any existing FlexUnit tests in the current source tree, so not sure 
> where to put it.
> Otherwise Mustella seems like a beast to get to know and run, so I will need 
> some guidance as to what to do if you require new Mustella tests.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

Reply via email to