You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@flex.apache.org by "Maurice Nicholson (JIRA)" <ji...@apache.org> on 2012/11/26 21:58:58 UTC

[jira] [Updated] (FLEX-33273) CSSCondition.matchesStyleClient() is slow and creates excessive garbage

     [ https://issues.apache.org/jira/browse/FLEX-33273?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Maurice Nicholson updated FLEX-33273:
-------------------------------------

    Labels: patch performance  (was: performance)
    
> 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
>              Labels: patch, performance
>         Attachments: FLEX-33273.patch
>
>
> 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