Bug 42272 - Canvas: Fast-path for assigning the same color string as before to fillStyle or strokeStyle
Summary: Canvas: Fast-path for assigning the same color string as before to fillStyle ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords: Performance
Depends on:
Blocks:
 
Reported: 2010-07-14 11:01 PDT by Andreas Kling
Modified: 2010-07-14 12:33 PDT (History)
4 users (show)

See Also:


Attachments
Proposed patch (5.83 KB, patch)
2010-07-14 11:07 PDT, Andreas Kling
no flags Details | Formatted Diff | Diff
Proposed patch v2 (5.33 KB, patch)
2010-07-14 11:30 PDT, Andreas Kling
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andreas Kling 2010-07-14 11:01:00 PDT
When assigning a color string to fillStyle or strokeStyle, we take a detour via a CanvasStyle object.

We could add an early return fast-path when assigning the same string as before that does the check without creating a CanvasStyle object.
Comment 1 Andreas Kling 2010-07-14 11:07:35 PDT
Created attachment 61541 [details]
Proposed patch

Benchmarking with Hixie's fillRect video test:
http://hixie.ch/tests/adhoc/perf/video/002.html

BEFORE:

Elapsed wall-clock time: 853ms (ideal: 640ms).
Elapsed non-idle time: 213ms (ideal: 0ms).
Speed: 17.58fps (ideal: 25.00fps).

AFTER:

Elapsed wall-clock time: 819ms (ideal: 640ms).
Elapsed non-idle time: 179ms (ideal: 0ms).
Speed: 18.32fps (ideal: 25.00fps).
Comment 2 Eric Seidel (no email) 2010-07-14 11:18:36 PDT
Attachment 61541 [details] did not build on mac:
Build output: http://webkit-commit-queue.appspot.com/results/3548050
Comment 3 Andreas Kling 2010-07-14 11:22:38 PDT
(In reply to comment #2)
> Attachment 61541 [details] did not build on mac:
> Build output: http://webkit-commit-queue.appspot.com/results/3548050

cc1plus: warnings being treated as errors
/Users/eseidel/Projects/MacEWS/WebCore/bindings/js/JSCanvasRenderingContext2DCustom.cpp:54: warning: unused parameter 'exec'
distcc[13899] ERROR: compile /Users/eseidel/Projects/MacEWS/WebCore/bindings/js/JSCanvasRenderingContext2DCustom.cpp on localhost failed
Comment 4 Andreas Kling 2010-07-14 11:30:19 PDT
Created attachment 61546 [details]
Proposed patch v2

Less kludgy version - since assigning an empty string to a CanvasRenderingContext2D property has no effect we don't need the bools in the previous patch.
Comment 5 Darin Adler 2010-07-14 12:19:25 PDT
Comment on attachment 61546 [details]
Proposed patch v2

>  JSValue JSCanvasRenderingContext2D::strokeStyle(ExecState* exec) const
>  {
>      CanvasRenderingContext2D* context = static_cast<CanvasRenderingContext2D*>(impl());
> +
>      return toJS(exec, context->strokeStyle());        
>  }

Stray change here.
Comment 6 Andreas Kling 2010-07-14 12:33:37 PDT
Committed r63344: <http://trac.webkit.org/changeset/63344>