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

Issue 4358051: Add vertex declaration cache (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 years ago by John Bauman
Modified:
3 years 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
3 years ago #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 ...
3 years ago #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 ...
3 years ago #3
John Bauman
3 years ago #4
apatrick1
LGTM
3 years ago #5
nicolas
The code looks good to me. I assume it has been/will be extensively tested against ...
3 years ago #6
dgkoch
Other than making the LRU counter unsigned it looks good. A couple questions though: 1) ...
3 years ago #7
John Bauman
I don't have actual framerate numbers, but before JSGamebench was spending somewhere around 20% of ...
3 years ago #8
dgkoch
On 2011/04/06 16:36:02, John Bauman wrote: > I don't have actual framerate numbers, but before ...
3 years ago #9
apatrick1
3 years ago #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 1278:e6ce13d99bf5