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:17:58 GMT

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

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

Good spot :-)

I've updated the attached patch so it works for these test cases too, and it still compares
very favourably with the original implementation on my hardware (MacBook Air) and FP (Chrome
Release, v11.5):

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   114.50   -95.29  
 96.00     0.00  -100.00
matchesStyleClient({styleName: "foobar foo"}, "foo")          2426.50   115.30   -95.25  
 96.00     0.00  -100.00
matchesStyleClient({styleName: "foobar foo barfoo"}, "foo")   3673.50   117.80   -96.79  
107.00     0.00  -100.00
matchesStyleClient({styleName: "foo"}, "foo")                 1100.80    75.50   -93.14  
 97.00     0.00  -100.00
matchesStyleClient({styleName: "foo bar"}, "foo")             2246.30    79.50   -96.46  
 95.00     0.00  -100.00
matchesStyleClient({styleName: "bar foo"}, "foo")             2235.50    88.30   -96.05  
 97.00     0.00  -100.00
matchesStyleClient({styleName: "baz foo bar"}, "foo")         3318.30    94.30   -97.16  
107.00     0.00  -100.00
matchesStyleClient({styleName: "foobar"}, "foo")              1265.50    97.50   -92.30  
 92.00     0.00  -100.00
matchesStyleClient({styleName: "foofoo bar"}, "foo")          2504.80   125.30   -95.00  
 94.00     0.00  -100.00
matchesStyleClient({styleName: "fo"}, "foo")                  1013.80    67.30   -93.36  
 92.00     0.00  -100.00
matchesStyleClient({styleName: "fooo"}, "foo")                1143.50    95.80   -91.62  
 92.00     0.00  -100.00
matchesStyleClient({styleName: "2foo bar"}, "foo")            2291.00   102.00   -95.55  
 92.00     0.00  -100.00
matchesStyleClient({styleName: "foo2 bar"}, "foo")            2294.30   100.50   -95.62  
 92.00     0.00  -100.00
matchesStyleClient({styleName: ""}, "foo")                     598.30    55.30   -90.76  
128.00     0.00  -100.00
matchesStyleClient({styleName: null}, "foo")                    58.00    57.80    -0.34  
  0.00     0.00     0.00
matchesStyleClient({styleName: [object Object]}, "foo")         84.50    56.80   -32.78  
  0.00     0.00     0.00

The new version looks like this:

    public function matchesStyleClient(object:IAdvancedStyleClient):Boolean
    {
        if (kind == CSSConditionKind.CLASS)
        {
            const styleName:String = object.styleName as String;
            if (!styleName)
                return false;

            // Look for a match in a potential list of styleNames
            var index:int = styleName.indexOf(value);
            while (index != -1)
            {
                var next:int = index + value.length;

                if ((index == 0 || styleName.charAt(index - 1) == ' ') &&
                    (next == styleName.length || styleName.charAt(next) == ' '))
                    return true;

                index = styleName.indexOf(value, next)
            }

            return false;
        }
        else if (kind == CSSConditionKind.ID)
        {
            return (object.id == value);
        }
        else if (kind == CSSConditionKind.PSEUDO)
        {
            return (object.matchesCSSState(value));
        }

        return false;
    }

                
> 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