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

Issue 4358051: Add vertex declaration cache (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 years, 1 month ago by John Bauman
Modified:
4 years, 1 month 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
4 years, 1 month 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 ...
4 years, 1 month 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 ...
4 years, 1 month ago (2011-04-05 18:05:22 UTC) #3
John Bauman
4 years, 1 month ago (2011-04-06 02:18:24 UTC) #4
apatrick1
LGTM
4 years, 1 month 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 ...
4 years, 1 month 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) ...
4 years, 1 month 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 ...
4 years, 1 month 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 ...
4 years, 1 month ago (2011-04-06 18:06:41 UTC) #9
apatrick1
4 years, 1 month 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 e6eadc5