Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(710)

Issue 4358051: Add vertex declaration cache (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
12 years, 12 months ago by John Bauman
Modified:
12 years, 12 months ago
Reviewers:
dgkoch, nicolas, apatrick1
CC:
angleproject-review_googlegroups.com
Base URL:
https://angleproject.googlecode.com/svn/trunk
Visibility:
Public.

Description

Profiling shows that creating and destroying vertex declarations is extremely expensive, so we can keep a 16-element cache around to speed that up. BUG= TEST=JSGameBench Committed: http://code.google.com/p/angleproject/source/detail?r=609

Patch Set 1 #

Total comments: 7

Patch Set 2 : Fix various issues #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+57 lines, -7 lines) Patch
M src/libGLESv2/VertexDataManager.h View 1 1 chunk +11 lines, -0 lines 2 comments Download
M src/libGLESv2/VertexDataManager.cpp View 1 4 chunks +46 lines, -7 lines 0 comments Download

Messages

Total messages: 10
John Bauman
12 years, 12 months ago (2011-04-05 01:42:14 UTC) #1
nicolas
Just my two cents. http://codereview.appspot.com/4358051/diff/1/src/libGLESv2/VertexDataManager.cpp File src/libGLESv2/VertexDataManager.cpp (right): http://codereview.appspot.com/4358051/diff/1/src/libGLESv2/VertexDataManager.cpp#newcode522 src/libGLESv2/VertexDataManager.cpp:522: int maxLru; This should be ...
12 years, 12 months ago (2011-04-05 11:50:27 UTC) #2
apatrick1
http://codereview.appspot.com/4358051/diff/1/src/libGLESv2/VertexDataManager.cpp File src/libGLESv2/VertexDataManager.cpp (right): http://codereview.appspot.com/4358051/diff/1/src/libGLESv2/VertexDataManager.cpp#newcode546 src/libGLESv2/VertexDataManager.cpp:546: *element = end; This comment logically follows the one ...
12 years, 12 months ago (2011-04-05 18:05:22 UTC) #3
John Bauman
12 years, 12 months ago (2011-04-06 02:18:24 UTC) #4
apatrick1
LGTM
12 years, 12 months ago (2011-04-06 07:32:32 UTC) #5
nicolas
The code looks good to me. I assume it has been/will be extensively tested against ...
12 years, 12 months ago (2011-04-06 07:40:46 UTC) #6
dgkoch
Other than making the LRU counter unsigned it looks good. A couple questions though: 1) ...
12 years, 12 months ago (2011-04-06 14:15:00 UTC) #7
John Bauman
I don't have actual framerate numbers, but before JSGamebench was spending somewhere around 20% of ...
12 years, 12 months ago (2011-04-06 16:36:02 UTC) #8
dgkoch
On 2011/04/06 16:36:02, John Bauman wrote: > I don't have actual framerate numbers, but before ...
12 years, 12 months ago (2011-04-06 18:06:41 UTC) #9
apatrick1
12 years, 12 months ago (2011-04-06 18:35:09 UTC) #10
Awesome work John. When you check this in, can you roll DEPS to pull this into
Chromium? Let's get this in Canary for testing. Ping me if you have trouble.

Tip: a common DEPS roll error is to not check whether the shader translator has
warnings as errors on linux and mac. It's worth sending the patch to the trybots
on all platforms.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b