incubator-flex-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Maurice Nicholson (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (FLEX-33273) CSSCondition.matchesStyleClient() is slow and creates excessive garbage
Date Mon, 03 Dec 2012 20:47:59 GMT

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

Maurice Nicholson commented on FLEX-33273:
------------------------------------------

Using StringUtil.isWhitespace() doesn't seem to harm performance that much.

This is the current SDK implementation compared with a version of the current patch except
using StringUtil.isWhitespace instead of comparing with " ":

Comparing speed of matching a string in space delimited string. 1000000 loops.
TEST                                                           OLD MS   NEW MS    DELTA  OLD
MEM  NEW MEM    DELTA
matchesStyleClient({styleName: "barfoo foo"}, "foo")          2432.80   173.00   -92.89  
 96.00     0.00  -100.00
matchesStyleClient({styleName: "foobar foo"}, "foo")          2426.50   168.80   -93.04  
 96.00     0.00  -100.00
matchesStyleClient({styleName: "foobar foo barfoo"}, "foo")   3673.50   185.00   -94.96  
107.00     0.00  -100.00
matchesStyleClient({styleName: "foo"}, "foo")                 1100.80    76.00   -93.10  
 97.00     0.00  -100.00
matchesStyleClient({styleName: "foo bar"}, "foo")             2246.30    94.00   -95.82  
 95.00     0.00  -100.00
matchesStyleClient({styleName: "bar foo"}, "foo")             2235.50   106.00   -95.26  
 97.00     0.00  -100.00
matchesStyleClient({styleName: "baz foo bar"}, "foo")         3318.30   121.80   -96.33  
107.00     0.00  -100.00
matchesStyleClient({styleName: "foobar"}, "foo")              1265.50   136.30   -89.23  
 92.00     0.00  -100.00
matchesStyleClient({styleName: "foofoo bar"}, "foo")          2504.80   202.80   -91.90  
 94.00     0.00  -100.00
matchesStyleClient({styleName: "fo"}, "foo")                  1013.80    67.00   -93.39  
 92.00     0.00  -100.00
matchesStyleClient({styleName: "fooo"}, "foo")                1143.50   135.50   -88.15  
 92.00     0.00  -100.00
matchesStyleClient({styleName: "2foo bar"}, "foo")            2291.00   146.30   -93.61  
 92.00     0.00  -100.00
matchesStyleClient({styleName: "foo2 bar"}, "foo")            2294.30   163.00   -92.90  
 92.00     0.00  -100.00
matchesStyleClient({styleName: ""}, "foo")                     598.30    58.50   -90.22  
128.00     0.00  -100.00
matchesStyleClient({styleName: null}, "foo")                    58.00    58.00     0.00  
  0.00     0.00     0.00
matchesStyleClient({styleName: [object Object]}, "foo")         84.50    57.30   -32.19  
  0.00     0.00     0.00

                
> 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

Mime
View raw message