Bug 103463 – Create Profiling Tool for Image Decoders

• Component: WebCore Misc.
• Status: NEW

[Brent Fulgham]

This bug creates a simple profiling tool to assist in investigating various proposed improvements to the image decoders.

[Brent Fulgham]

Created attachment 176364 [details]
Patch

[Brent Fulgham]

Created attachment 176369 [details]
Patch

[Brent Fulgham]

The bulk of the patch is a bunch of Windows cruft for building.  This patch does not land a change to the main solution file, so no build changes are expected.

[noel gordon]

Comment on attachment 176369 [details]
Patch

Tools/ImageDecoderTest/win/ImageDecoderTest.vcproj doesn't look like a vcproj file.

[Brent Fulgham]

Created attachment 176398 [details]
Patch

[WebKit Review Bot]

Attachment 176398 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Tools/ChangeLog', u'Tools/ImageDecoderTest..." exit_code: 1
Tools/ImageDecoderTest/ImageDecoderPerf.cpp:29:  Alphabetical sorting problem.  [build/include_order] [4]
Tools/ImageDecoderTest/config.h:35:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 2 in 6 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Brent Fulgham]

Created attachment 176515 [details]
Patch

[Brent Fulgham]

Created attachment 176517 [details]
Patch

[Brent Fulgham]

Comment on attachment 176517 [details]
Patch

Ugh.  I hate these automatic mime-type additions.

[WebKit Review Bot]

Attachment 176517 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit/win/ChangeLog', u'Source/Web..." exit_code: 1
Tools/ImageDecoderTest/ImageDecoderPerf.cpp:29:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 16 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Brent Fulgham]

Created attachment 176522 [details]
Patch

[WebKit Review Bot]

Attachment 176522 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit/win/ChangeLog', u'Source/Web..." exit_code: 1
Tools/ImageDecoderTest/ImageDecoderPerf.cpp:29:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 16 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Build Bot]

Comment on attachment 176522 [details]
Patch

Attachment 176522 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/15028510

[Brent Fulgham]

Created attachment 176553 [details]
Patch

[Brent Fulgham]

I think the build dependencies are now correct, and I've managed to satisfy the style checker.

[Build Bot]

Comment on attachment 176553 [details]
Patch

Attachment 176553 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/15029491

[Brent Fulgham]

Created attachment 176574 [details]
Patch

[Build Bot]

Comment on attachment 176574 [details]
Patch

Attachment 176574 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/15044026

[Brent Fulgham]

Created attachment 176770 [details]
Patch

[Brent Fulgham]

Created attachment 176779 [details]
Patch

[Jussi Kukkonen (jku)]

Hi, thanks for this. I've used it in bug 105405 already.

The results are already useful but I'm seeing some variance... mostly rare spikes in execution time: as an example 9 runs finished in 9.1-9.2 seconds each and then one run took >12 seconds (this was with 100 iterations). This sort of variance is enough to prevent real performance profiling... My first guess is IO blocking (even if I have an SSD so several seconds sounds like a lot). 

Would it make sense to add the timing inside the tool so the buffer loading could be discounted? 

Also, as an idea: if you start doing the timing in the tool you could filter any outliers (like X% furthest from the mean) in the tool, before calculating the final result. Further idea: Either output the variance as well or refuse to output a result if variance exceeds X.

I guess doing either of those would only make sense after figuring out where the variance actually is. I'll see what I can do about that.

[Viatcheslav Ostapenko]

(In reply to comment #21)
> Hi, thanks for this. I've used it in bug 105405 already.
> 
> The results are already useful but I'm seeing some variance... mostly rare spikes in execution time: as an example 9 runs finished in 9.1-9.2 seconds each and then one run took >12 seconds (this was with 100 iterations). This sort of variance is enough to prevent real performance profiling... My first guess is IO blocking (even if I have an SSD so several seconds sounds like a lot). 
> 
> Would it make sense to add the timing inside the tool so the buffer loading could be discounted? 
> 
> Also, as an idea: if you start doing the timing in the tool you could filter any outliers (like X% furthest from the mean) in the tool, before calculating the final result. Further idea: Either output the variance as well or refuse to output a result if variance exceeds X.
> 
> I guess doing either of those would only make sense after figuring out where the variance actually is. I'll see what I can do about that.

3 sec. for IO? That's too much. It would be interesting to see real/user/sys values from this run.
If I run benchmark I try not to do anything in parallel on PC. Also, I used higher number of runs to minimize influence of IO and background processes. I've never seen variance bigger than 1%.
qtwebkit has this repo http://gitorious.org/qtwebkit/performance/ with a lot of useful things for benchmarking that could be borrowed, but I don't think it should go in this patch. I would submit it as it is and start making improvements on top of it.

[Brent Fulgham]

(In reply to comment #21)
> Hi, thanks for this. I've used it in bug 105405 already.
> 
> The results are already useful but I'm seeing some variance... mostly rare spikes in execution time: as an example 9 runs finished in 9.1-9.2 seconds each and then one run took >12 seconds (this was with 100 iterations). This sort of variance is enough to prevent real performance profiling... My first guess is IO blocking (even if I have an SSD so several seconds sounds like a lot). 
> 
> Would it make sense to add the timing inside the tool so the buffer loading could be discounted? 

That's a good idea. I must admit that I don't use this in its raw form.  I mainly use it inside of VTune to look at hotspots, so the I/O costs and so forth can easily be excluded.

[Anders Carlsson]

Comment on attachment 176779 [details]
Patch

Clearing review flag on patches from before 2014. If this patch is still relevant, please reset the r? flag.