Reland "Add support for trace counters with variable arguments and instrument the raster cache." #8145
Conversation
chinmaygarde
requested review from
cbracken,
jamesderlin,
jason-simmons and
liyuqian
chinmaygarde
changed the title
| #define FML_TRACE_COUNTER(category_group, name, count) \ | ||
| ::fml::tracing::TraceCounter(category_group, name, count); | ||
| #define FML_TRACE_COUNTER(category_group, name, counter_id, arg1, ...) \ | ||
| ::fml::tracing::TraceCounter((category_group), (name), (counter_id), (arg1), \ |
There was a problem hiding this comment.
This isn't actually what I meant, but if it works, great.
There was a problem hiding this comment.
Seemed to work on Windows. What did you actually mean?
There was a problem hiding this comment.
I meant that the way simulate variadic macros on platforms that don't support them is to do:
#define MACRO(args) (some_variadic_function args)
and then invoke it as:
MACRO((arg1, arg2, arg3, arg4));
There was a problem hiding this comment.
How is that ... variadic?
There was a problem hiding this comment.
You can invoke the macro with any number of "arguments" and have them forward to an actual variadic function. It'd behave like a variadic macro except that you'd have to live with extra parentheses wherever it's used.
| size_t picture_cache_bytes = 0; | ||
|
|
||
| for (const auto& item : layer_cache_) { | ||
| const auto dimensions = item.second.image.image_dimensions(); |
There was a problem hiding this comment.
Maybe count/separate the number of items that has an empty/nonempty image?
There was a problem hiding this comment.
I did want to get your clarification on this. Why do empty images take up raster cache entries?
There was a problem hiding this comment.
But, if you think thats useful, we can certainly add that stuff to the trace.
There was a problem hiding this comment.
I think it's due to our old logic of only caching when the SkPicture is rasterized a few times:
Before the threshold is met, we create an Entry which has an empty image, and keep incrementing access_count.
|
@fkorotkov the |
|
Also, is there a way in the UI to check how long a presubmit spent in the queue? Once started, only the time taken to finish the job is shown which is only a part of the total turnaround time. |
chinmaygarde
force-pushed
the
reland_counters
branch
from
9f1f018 to
35af4af
Compare
|
@tvolkert there were some issues earlier today with virtualization API inconsistency which led to the very long queue. Cirrus has being catching up for a few hours now and still the queue is pretty long but you have enough credits so the builds are in front of the queue. I'm monitoring the situation. Sorry for the troubles. A fix was deployed which will prevent from this happening again when virtualization API will lag next time. |
|
Is there a status page for such outages I can bookmark before resorting to disabling bots? |
|
No proper status page (follow the issue for updates) but everything is reported on Twitter. |
…nstrument the raster cache." This reverts commit bc90132 and fixes the discovered on Windows builds.
chinmaygarde
force-pushed
the
reland_counters
branch
from
35af4af to
974d2ee
Compare
…ts and instrument the raster cache." (flutter/engine#8145)
…ts and instrument the raster cache." (flutter/engine#8145)
…ts and instrument the raster cache." (flutter/engine#8145)
…ts and instrument the raster cache." (flutter/engine#8145)
…ts and instrument the raster cache." (flutter/engine#8145)
…ts and instrument the raster cache." (flutter/engine#8145)
…ts and instrument the raster cache." (flutter/engine#8145)
…ts and instrument the raster cache." (flutter/engine#8145)
…ts and instrument the raster cache." (flutter/engine#8145)
…ts and instrument the raster cache." (flutter/engine#8145)
…ts and instrument the raster cache." (flutter/engine#8145)
…ts and instrument the raster cache." (flutter/engine#8145)
…ts and instrument the raster cache." (flutter/engine#8145)
…nstrument the raster cache." (flutter#8145) This reverts commit bc90132 and fixes the discovered on Windows builds.
…ts and instrument the raster cache." (flutter#8145)" This reverts commit d373aad.
…ts and instrument the raster cache." (flutter#8145)" This reverts commit d373aad.
7 participants
This reverts commit bc90132 and fixes the issue discovered on Windows builds.