Bug 59029 - Reduce size of GraphicsContext3D's compiled code
Summary: Reduce size of GraphicsContext3D's compiled code
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGL (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Kenneth Russell
URL:
Keywords:
Depends on:
Blocks: 59076
  Show dependency treegraph
 
Reported: 2011-04-20 15:41 PDT by Kenneth Russell
Modified: 2011-08-02 08:54 PDT (History)
9 users (show)

See Also:


Attachments
Patch (96.89 KB, patch)
2011-04-20 15:51 PDT, Kenneth Russell
jamesr: review+
kbr: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kenneth Russell 2011-04-20 15:41:13 PDT
GraphicsContext3D necessarily contains comprehensive image format conversion code in order to support all of the conversions that might be needed during texture uploading. Currently each format conversion is expressed as a function handling a single pixel, and these functions are passed as template parameters so they can be inlined. Unfortunately, this is causing excessive code bloat. In http://crbug.com/78976 thakis suggests making the conversion functions do their work line-by-line and passing them as function pointers instead of template parameters to reduce the code size.
Comment 1 Kenneth Russell 2011-04-20 15:51:11 PDT
Created attachment 90430 [details]
Patch
Comment 2 Zhenyao Mo 2011-04-20 16:06:19 PDT
LGTM.
Comment 3 James Robinson 2011-04-20 16:53:48 PDT
Comment on attachment 90430 [details]
Patch

Nifty.  What's the before/after size of GraphicsContext3D.o ?
Comment 4 Nico Weber 2011-04-20 16:57:25 PDT
jamesr: See ChangeLog
Comment 5 Kenneth Russell 2011-04-20 17:28:29 PDT
Committed r84441: <http://trac.webkit.org/changeset/84441>
Comment 6 James Robinson 2011-04-20 17:29:20 PDT
(In reply to comment #4)
> jamesr: See ChangeLog

Oh. I can reads good
Comment 7 Stephen White 2011-04-21 10:41:46 PDT
Comment on attachment 90430 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=90430&action=review

> Source/WebCore/platform/graphics/GraphicsContext3D.cpp:373
> +        destination[3] = convertColor16LittleTo8(source[3]);

Not really related to this work, but if performance is a consideration, doing these conversions an int at a time (or least, using an int pointer for the destination doing a single store) is often much faster than doing char-by-char.  SSE2+ would even better would be even better (but more work).
Comment 8 Nico Weber 2011-04-21 10:51:35 PDT
On the chrome bug I suggested jitting up the right conversion function, which is probably best wrt performance. But if anything needs to be color converted on the cpu on every frame, things won't be very fast anyhow.
Comment 9 Benoit Jacob 2011-08-02 08:54:02 PDT
Possibly related:
http://code.google.com/p/chromium/issues/detail?id=91208#c7

As I explain there, I disagree with unstripped object file size as a metric for code bloat. Here, using `nm` tells me that the code generated by these templates is just 25k.