You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@flex.apache.org by "Frédéric THOMAS (JIRA)" <ji...@apache.org> on 2012/11/26 22:45:00 UTC
[jira] [Commented] (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:comment-tabpanel&focusedCommentId=13504129#comment-13504129 ]
Frédéric THOMAS commented on FLEX-33273:
----------------------------------------
Hi Maurice,
I applied those changes few days ago on my 4.8 version and ran it against a big app, it seems ok, I sitll have to run the mustalla tests against (and check we have the appropraited ones).
Thru time, could you please provide the performence tests you did ?
> 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