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

Issue 9436044: Prototype for less memory intensive initializing of builtins. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
9 years ago by Alan Leung Chromium
Modified:
8 years, 11 months ago
CC:
angleproject-review_googlegroups.com
Base URL:
http://angleproject.googlecode.com/svn/trunk/
Visibility:
Public.

Description

Less memory intensive initializing of builtins. R=alokp@chromium.org Committed: https://code.google.com/p/angleproject/source/detail?r=2425

Patch Set 1 #

Patch Set 2 : #

Total comments: 12

Patch Set 3 : #

Total comments: 1

Patch Set 4 : #

Total comments: 1

Patch Set 5 : #

Patch Set 6 : #

Total comments: 6

Patch Set 7 : #

Total comments: 6

Patch Set 8 : #

Patch Set 9 : #

Patch Set 10 : #

Total comments: 8

Patch Set 11 : #

Patch Set 12 : #

Patch Set 13 : #

Patch Set 14 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+2166 lines, -344 lines) Patch
M src/build_angle.gypi View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +2 lines, -0 lines 0 comments Download
M src/compiler/Compiler.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 3 chunks +3 lines, -0 lines 0 comments Download
M src/compiler/Initialize.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 2 chunks +0 lines, -344 lines 0 comments Download
A src/compiler/builtin_symbol_table.h View 1 2 3 4 5 6 7 8 1 chunk +15 lines, -0 lines 0 comments Download
A src/compiler/builtin_symbol_table.cpp View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +279 lines, -0 lines 0 comments Download
A src/compiler/builtin_symbols.json View 1 2 3 4 5 6 1 chunk +1776 lines, -0 lines 0 comments Download
A src/compiler/generate_builtin_symbol_table.py View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +91 lines, -0 lines 0 comments Download

Messages

Total messages: 36
Alan Leung Chromium
Hey Alok I played around with the idea of not using the parser at all ...
9 years ago (2013-05-15 21:45:14 UTC) #1
Alok Priyadarshi
A couple of questions. I am not sure where the memory savings are coming from. ...
9 years ago (2013-05-16 04:19:37 UTC) #2
Alok Priyadarshi
A couple of questions. I am not sure where the memory savings are coming from. ...
9 years ago (2013-05-16 04:19:38 UTC) #3
nicolas%transgaming.com
Please note that this code is undergoing significant changes for ES3 support, since it defines ...
9 years ago (2013-05-16 14:13:13 UTC) #4
Alan Leung Chromium
Thanks for looking at this, Alok / Nicolas. As for nicolas's question: I thought about ...
9 years ago (2013-05-16 18:05:49 UTC) #5
Alok Priyadarshi
https://codereview.appspot.com/9436044/diff/1004/src/compiler/Initialize.cpp File src/compiler/Initialize.cpp (left): https://codereview.appspot.com/9436044/diff/1004/src/compiler/Initialize.cpp#oldcode24 src/compiler/Initialize.cpp:24: TString s; Can you try just using std::string instead ...
9 years ago (2013-05-16 22:51:11 UTC) #6
Alan Leung Chromium
https://codereview.appspot.com/9436044/diff/1004/src/compiler/Initialize.cpp File src/compiler/Initialize.cpp (left): https://codereview.appspot.com/9436044/diff/1004/src/compiler/Initialize.cpp#oldcode24 src/compiler/Initialize.cpp:24: TString s; On 2013/05/16 22:51:11, Alok Priyadarshi wrote: > ...
8 years, 12 months ago (2013-05-20 22:26:53 UTC) #7
Alok Priyadarshi
I like the idea of manually building the built-in symbol table. Lets figure out how ...
8 years, 12 months ago (2013-05-21 00:06:12 UTC) #8
Alan Leung Chromium
> combination of function dictionary and python script. > > Nicolas: Do you have any ...
8 years, 12 months ago (2013-05-23 21:53:50 UTC) #9
Alok Priyadarshi
Yes please port it to python if you can.
8 years, 12 months ago (2013-05-23 22:38:12 UTC) #10
Alan Leung Chromium
On 2013/05/23 22:38:12, Alok Priyadarshi wrote: > Yes please port it to python if you ...
8 years, 11 months ago (2013-05-28 18:18:42 UTC) #11
Alok Priyadarshi
If you use a text file, you would need to parse it. I was thinking ...
8 years, 11 months ago (2013-05-28 19:16:34 UTC) #12
Alan Leung Chromium
What do you think of something like this. It allows us to have finer control ...
8 years, 11 months ago (2013-05-28 21:43:05 UTC) #13
Alok Priyadarshi
https://codereview.appspot.com/9436044/diff/18002/src/compiler/Initialize.h File src/compiler/Initialize.h (right): https://codereview.appspot.com/9436044/diff/18002/src/compiler/Initialize.h#newcode37 src/compiler/Initialize.h:37: static TType* GetGlobalType(const std::string& name) { One of the ...
8 years, 11 months ago (2013-05-28 21:50:01 UTC) #14
Alan Leung Chromium
On 2013/05/28 21:50:01, Alok Priyadarshi wrote: > https://codereview.appspot.com/9436044/diff/18002/src/compiler/Initialize.h > File src/compiler/Initialize.h (right): > > https://codereview.appspot.com/9436044/diff/18002/src/compiler/Initialize.h#newcode37 ...
8 years, 11 months ago (2013-05-28 22:18:13 UTC) #15
Alok Priyadarshi
> But what do you think about the common.input file format? Does this sound like ...
8 years, 11 months ago (2013-05-28 22:29:09 UTC) #16
Alan Leung Chromium
On 2013/05/28 22:29:09, Alok Priyadarshi wrote: > > But what do you think about the ...
8 years, 11 months ago (2013-05-29 00:32:59 UTC) #17
Alok Priyadarshi
Getting closer! I think the following scheme would work well: 1. builtin_symbols.json will contain a ...
8 years, 11 months ago (2013-05-29 05:43:19 UTC) #18
Alan Leung Chromium
Thanks for the suggestions, Alok. Would you mind taking another look? On 2013/05/29 05:43:19, Alok ...
8 years, 11 months ago (2013-05-29 23:25:22 UTC) #19
Alok Priyadarshi
did you forget to include the json file? https://codereview.appspot.com/9436044/diff/27001/src/compiler/builtin_symbol_table.cpp File src/compiler/builtin_symbol_table.cpp (right): https://codereview.appspot.com/9436044/diff/27001/src/compiler/builtin_symbol_table.cpp#newcode1 src/compiler/builtin_symbol_table.cpp:1: // ...
8 years, 11 months ago (2013-05-30 18:49:47 UTC) #20
Alan Leung Chromium
I added the json file as well. https://codereview.appspot.com/9436044/diff/27001/src/compiler/builtin_symbol_table.cpp File src/compiler/builtin_symbol_table.cpp (right): https://codereview.appspot.com/9436044/diff/27001/src/compiler/builtin_symbol_table.cpp#newcode1 src/compiler/builtin_symbol_table.cpp:1: // Copyright ...
8 years, 11 months ago (2013-05-30 20:18:48 UTC) #21
Alok Priyadarshi
Almost there. Did you forget to include changes to avoid adding common-function string? https://codereview.appspot.com/9436044/diff/32001/src/compiler/builtin_symbol_table.h File ...
8 years, 11 months ago (2013-05-31 03:47:57 UTC) #22
Alan Leung Chromium
PTAL? > Did you forget to include changes to avoid adding common-function string? What do ...
8 years, 11 months ago (2013-06-03 20:40:51 UTC) #23
Alan Leung Chromium
PTAL? https://codereview.appspot.com/9436044/diff/32001/src/compiler/builtin_symbol_table.h File src/compiler/builtin_symbol_table.h (right): https://codereview.appspot.com/9436044/diff/32001/src/compiler/builtin_symbol_table.h#newcode9 src/compiler/builtin_symbol_table.h:9: #include "compiler/SymbolTable.h" On 2013/05/31 03:47:58, Alok Priyadarshi wrote: ...
8 years, 11 months ago (2013-06-03 20:41:57 UTC) #24
Alok Priyadarshi
On 2013/06/03 20:40:51, acleung1 wrote: > PTAL? > > > Did you forget to include ...
8 years, 11 months ago (2013-06-03 20:46:32 UTC) #25
nicolas
Please note that using a macro will create a lot of code. Also, this approach ...
8 years, 11 months ago (2013-06-04 19:51:32 UTC) #26
Alan Leung Chromium
On 2013/06/03 20:46:32, Alok Priyadarshi wrote: > On 2013/06/03 20:40:51, acleung1 wrote: > > PTAL? ...
8 years, 11 months ago (2013-06-04 23:32:57 UTC) #27
Alok Priyadarshi
On 2013/06/04 23:32:57, acleung1 wrote: > On 2013/06/03 20:46:32, Alok Priyadarshi wrote: > > On ...
8 years, 11 months ago (2013-06-05 21:16:00 UTC) #28
Alok Priyadarshi
On 2013/06/04 19:51:32, nicolas wrote: > Please note that using a macro will create a ...
8 years, 11 months ago (2013-06-05 21:21:30 UTC) #29
Alok Priyadarshi
https://codereview.appspot.com/9436044/diff/47001/src/compiler/Initialize.cpp File src/compiler/Initialize.cpp (left): https://codereview.appspot.com/9436044/diff/47001/src/compiler/Initialize.cpp#oldcode503 src/compiler/Initialize.cpp:503: builtInStrings.push_back(BuiltInFunctionsCommon(resources)); Could you also delete BuiltInFunctionsCommon if it is ...
8 years, 11 months ago (2013-06-05 21:26:04 UTC) #30
Alan Leung Chromium
PTAL. https://codereview.appspot.com/9436044/diff/47001/src/compiler/Initialize.cpp File src/compiler/Initialize.cpp (left): https://codereview.appspot.com/9436044/diff/47001/src/compiler/Initialize.cpp#oldcode503 src/compiler/Initialize.cpp:503: builtInStrings.push_back(BuiltInFunctionsCommon(resources)); On 2013/06/05 21:26:05, Alok Priyadarshi wrote: > ...
8 years, 11 months ago (2013-06-05 23:26:07 UTC) #31
Alok Priyadarshi
https://codereview.appspot.com/9436044/diff/47001/src/compiler/builtin_symbol_table.cpp File src/compiler/builtin_symbol_table.cpp (right): https://codereview.appspot.com/9436044/diff/47001/src/compiler/builtin_symbol_table.cpp#newcode12 src/compiler/builtin_symbol_table.cpp:12: #define BUILTIN1(t, rvalue, name, ptype1, pname1) { TFunction* f ...
8 years, 11 months ago (2013-06-05 23:55:42 UTC) #32
Alan Leung Chromium
On 2013/06/05 23:55:42, Alok Priyadarshi wrote: > https://codereview.appspot.com/9436044/diff/47001/src/compiler/builtin_symbol_table.cpp > File src/compiler/builtin_symbol_table.cpp (right): > > https://codereview.appspot.com/9436044/diff/47001/src/compiler/builtin_symbol_table.cpp#newcode12 ...
8 years, 11 months ago (2013-06-06 22:21:11 UTC) #33
Alan Leung Chromium
Forgot to send the comments as well. https://codereview.appspot.com/9436044/diff/47001/src/compiler/builtin_symbol_table.cpp File src/compiler/builtin_symbol_table.cpp (right): https://codereview.appspot.com/9436044/diff/47001/src/compiler/builtin_symbol_table.cpp#newcode12 src/compiler/builtin_symbol_table.cpp:12: #define BUILTIN1(t, ...
8 years, 11 months ago (2013-06-06 22:21:41 UTC) #34
Alok Priyadarshi
On 2013/06/06 22:21:41, acleung1 wrote: > Forgot to send the comments as well. > > ...
8 years, 11 months ago (2013-06-06 23:26:45 UTC) #35
Alan Leung Chromium
8 years, 11 months ago (2013-06-07 00:26:09 UTC) #36
Message was sent while issue was closed.
Committed patchset #14 manually as r2425 (presubmit successful).
Sign in to reply to this message.

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