[
https://issues.apache.org/jira/browse/FLEX-33273?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Maurice Nicholson updated FLEX-33273:
-------------------------------------
Attachment: PerformanceTest.zip
Find attached the performance test project I am using for benchmarking. As I
said it is based on Grant Skinner's PerformanceTest v2 Beta.
Note that I am only testing the part of the algorithm inside the "if (kind ==
CSSConditionKind.CLASS) {...}" block.
Before uploading, I cleaned it up a bit and added a few more possible
alternatives... on my machine running in Chrome with bundled FP 11.5 release
player, the "alternative_new2a" implementation is actually the fastest in
nearly all cases, and has is most readable too, IMHO. This is similar to the
version in patch except it uses const instead of var for the 2 temporary
I've added the result XML dumps in the "results" directory for my runs. Note
the times will be different to those above, since I changed the benchmark to
make it slightly more realistic. I wrote my own tool to compare 2 XML files
(it's written in Go and lives here: https://github.com/maurice/ptcmp if you
want to use it too)
> 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
