[Webkit-unassigned] [Bug 88271] initial rendering microbenchmark

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=88271





--- Comment #2 from Nat Duca <nduca at chromium.org>  2012-06-04 19:10:05 PST ---
(From update of attachment 145662)
View in context: https://bugs.webkit.org/attachment.cgi?id=145662&action=review

Good first pass. Lets start gluing it all together.

> Source/WebKit/chromium/public/RenderingBenchmarkResults.h:1
> +/*

copy the style of other headers in that folder.

> Source/WebKit/chromium/public/RenderingBenchmarkResults.h:10
> +#pragma once

follow the style in the rest of the public/ folder

> Source/WebKit/chromium/public/RenderingBenchmarkResults.h:12
> +class RenderingBenchmarkResults {

We put "web" in the front of public wk apis. See other things in the folder, e.g. WebCompositorInputHandler.h

> Source/WebKit/chromium/public/RenderingBenchmarkResults.h:15
> +    virtual void addResult(const char *resultName, double result) = 0;

A const char* for the unit is probably also a good thing.

> Source/WebKit/chromium/public/WebView.h:486
> +    // Rendering benchmarks ------------------------------------------------------

Lets put this entry point with the 'GPU acceleration support' group of methods, e.g. just above visibility after transferActiveWheelBlahblah

Take a peek through this file and see if we typically pass by pointer. Pass-by-ref seems to be used only for pass-by-reference, though I might be missing something.

> Source/WebKit/chromium/src/WebViewImpl.cpp:1614
> +void WebViewImpl::runRenderingBenchmarks(RenderingBenchmarkResults& results)

Lets get this factored into a class like we described...

RenderingBenchmark {
  const char* name() const = 0;
  void setUp(WebViewImpl* webViewImpl) {}
  void run(RenderingBenchmarkResults& results, WebViewImpl* webViewImpl) = 0;
  void tearDown(WebViewImpl* webViewImpl) { }
}

Then have another class called

AllRenderingBenchmarks : RenderingBenchmark {
   AllRenderingBenchmarks() {
      push instances benchmark subclasses of interest into m_benchmarks;
   }
   run() { run each of the instances; }
};
This runRenderingBenchamrks function should just construct a benchmark and run it. Ideally the test code will be in a bunch of classes in a cpp file --- the only thing exported by the benchmark.h would be the AllRenderingBenchmarks.

> Source/WebKit/chromium/src/WebViewImpl.cpp:1623
> +    this->paint(&canvas, screenRect);

This works if you're in software mode. You need to handle accelerated mode as well. That is, you need to get the root graphics layer and iterate over each graphics layer painting it.

> Source/WebKit/chromium/src/WebViewImpl.cpp:1625
> +    results.addResult("paint to bitmap time", afterPaint - beforePaint);

methinks the benchmark names should be camelcased words. E.g. paintTime

> Source/WebKit/chromium/src/WebViewImpl.cpp:1632
> +    results.addResult("paint to null canvas time", afterPaint - beforePaint);

paintTime_NullCanvas

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.