Add a complexity scoring class for Metal and OpenGL #31417
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!). If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
| return; | ||
| } | ||
| // The performance penalties seem fairly consistent percentage-wise | ||
| float non_hairline_penalty = 1.0f; |
There was a problem hiding this comment.
Right now this suggestion is a premature optimization, but just want to throw it out there in case it's useful later:
Consider avoiding floating point. You can do that by multiplying these factors by 10, and then dividing by 10 later when you need the final result.
Other strategies might be needed below, but we can delay thinking about them until they're needed.
| // m = 1/2 | ||
| // c = 1 | ||
| save_layer_complexity = (save_layer_count_ + 2) * 100000; | ||
| } |
There was a problem hiding this comment.
So if I have 200 saveLayers then I can save time by adding one more? If the slope changes at 200, then perhaps this can be handled in saveLayer by doing something like:
if (++count > 200) {
accumulate(M1 * x + B1);
} else {
accumulate(M2 * x + B2);
}
Also, is this depth based or sequential based or both?
There was a problem hiding this comment.
Plugging in 200 for the count in both equations shows a huge difference in the values. That doesn't sound right. Is my comment about 201 saveLayers taking less time than 199 saveLayers true?
There was a problem hiding this comment.
Depth vs. sequential might make a difference but there's no clear trend. Looking at the benchmark data, it varies from -30% to +18%.
Bizarrely, the benchmarking data shows a decrease in overall time at around 128 saveLayer calls, then it starts to spike upwards at a much higher rate starting at around 256 calls. I see this dip in both the nested and the unnested benchmark runs. See the attached screenshot - for the blue line, the X axis values are the saveLayer count; for the orange line, multiply them by 8. That being said, the data is very hard to actually fit to a trend so this is really just a very rough approximation.
With all that being said, any saveLayer right now is going to hit the threshold for caching, and when we're talking about 200 saveLayer calls, whether we're talking about a cost of 20,200,000 (very roughly a time cost of 101 milliseconds) or 8,200,000 (roughly 41ms), they both far exceed the threshold for caching. I think we're overthinking this.
.
| // one and a less expensive one. Both scale linearly with area. | ||
| // | ||
| // Expensive: All filled style, symmetric w/AA | ||
| bool expensive = |
There was a problem hiding this comment.
symmetric RRects are more expensive than non-symmetric versions? Did you mention this to Skia?
There was a problem hiding this comment.
Not yet, but yes, symmetric does seem to be more expensive.
gw280
changed the title
|
Big update:
Tests to come. |
gw280
force-pushed
the
gwright-complexity-score
branch
from
79d7cc6
to
49f792c
Compare
|
/cc @iskakaushik |
gw280
force-pushed
the
gwright-complexity-score
branch
from
82b2e54
to
51e92a6
Compare
…e other ops. Add some comments on the rationale behind the decisions made. Remove some floating point arithmetic.
…L calculators use
…delines, fix minor bug in GL calculator's drawLine
gw280
force-pushed
the
gwright-complexity-score
branch
from
d359980
to
d822f46
Compare
This adds an initial implementation of a complexity scoring class for Metal. Some notes:
mandc. This stems fromy=mx+c, and details the line graph used to best fit the benchmark data for that particular usecase. Important: these constants are before dividing the benchmark data by the number of draw calls made.flutter/flutter#86728
Tests to come.
Pre-launch Checklist
writing and running engine tests.
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.