Replaced std::string argument storage in gpu_trace_event with faster TraceAnyType

Replaced std::string argument storage in gpu_trace_event with faster TraceAnyType

BUG=80172
TEST=

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=82620

[jbates, 2011-04-21 16:21:38]

(added scheib and jbauman)

[scheib, 2011-04-21 16:44:35]

I like where it's going. ;)

Look into Value - values.s to see if we can use that code some?

Also, good form to have an issue for things and mark it in the BUG= line.

http://codereview.chromium.org/6877101/diff/1/gpu/common/gpu_trace_event.cc
File gpu/common/gpu_trace_event.cc (right):

http://codereview.chromium.org/6877101/diff/1/gpu/common/gpu_trace_event.cc#n...
gpu/common/gpu_trace_event.cc:103: *out += "'";
Should check for ' characters in the string and escape them or change them.

[jbates, 2011-04-21 21:11:50]

Added comment to explain why base::Value is not used.

http://codereview.chromium.org/6877101/diff/1/gpu/common/gpu_trace_event.cc
File gpu/common/gpu_trace_event.cc (right):

http://codereview.chromium.org/6877101/diff/1/gpu/common/gpu_trace_event.cc#n...
gpu/common/gpu_trace_event.cc:103: *out += "'";
On 2011/04/21 16:44:35, scheib wrote:
> Should check for ' characters in the string and escape them or change them.

Done.

[scheib, 2011-04-21 21:41:39]

LGTM

[nduca, 2011-04-21 22:15:43]

can you also modify webkit/glue/webkitclient_impl.cc to use this new magic?

http://codereview.chromium.org/6877101/diff/1005/gpu/common/gpu_trace_event.cc
File gpu/common/gpu_trace_event.cc (left):

http://codereview.chromium.org/6877101/diff/1005/gpu/common/gpu_trace_event.c...
gpu/common/gpu_trace_event.cc:110: *out += argNames[i];
check with @scheib/bauman --- might need to quote the argName

http://codereview.chromium.org/6877101/diff/1005/gpu/common/gpu_trace_event.cc
File gpu/common/gpu_trace_event.cc (right):

http://codereview.chromium.org/6877101/diff/1005/gpu/common/gpu_trace_event.c...
gpu/common/gpu_trace_event.cc:104: *out += "'";
double quotes

http://codereview.chromium.org/6877101/diff/1005/gpu/common/gpu_trace_event.c...
gpu/common/gpu_trace_event.cc:112: *out += "''";
double quotes

http://codereview.chromium.org/6877101/diff/1005/gpu/common/gpu_trace_event.h
File gpu/common/gpu_trace_event.h (right):

http://codereview.chromium.org/6877101/diff/1005/gpu/common/gpu_trace_event.h...
gpu/common/gpu_trace_event.h:186: class TraceAnyType {
TraceVar or TraceVariadic?

Very similar to PP_Var (chrome/ppapi)

http://codereview.chromium.org/6877101/diff/1005/gpu/common/gpu_trace_event.h...
gpu/common/gpu_trace_event.h:190: BOOLEAN,
/me wonders if we usually prefix with TYPE_? pp_var and base/values.h might be
good reference points. And, sorry if you've already normalized to a convention..
:)

[nduca, 2011-04-21 22:15:52]

LGTM less nits

[scheib, 2011-04-21 22:30:23]

Right, so I'm just discovering a few things we were failing on to have
strictly conforming JSON:
- must use "
- must wrap dictionary keys in "
- no hex numbers

[jbates, 2011-04-21 23:57:53]

Thanks guys. I think I fixed the JSON format as well.

http://codereview.chromium.org/6877101/diff/1005/gpu/common/gpu_trace_event.cc
File gpu/common/gpu_trace_event.cc (left):

http://codereview.chromium.org/6877101/diff/1005/gpu/common/gpu_trace_event.c...
gpu/common/gpu_trace_event.cc:110: *out += argNames[i];
On 2011/04/21 22:15:44, nduca wrote:
> check with @scheib/bauman --- might need to quote the argName

Done.

http://codereview.chromium.org/6877101/diff/1005/gpu/common/gpu_trace_event.cc
File gpu/common/gpu_trace_event.cc (right):

http://codereview.chromium.org/6877101/diff/1005/gpu/common/gpu_trace_event.c...
gpu/common/gpu_trace_event.cc:104: *out += "'";
On 2011/04/21 22:15:44, nduca wrote:
> double quotes

Done.

http://codereview.chromium.org/6877101/diff/1005/gpu/common/gpu_trace_event.c...
gpu/common/gpu_trace_event.cc:112: *out += "''";
On 2011/04/21 22:15:44, nduca wrote:
> double quotes

Done.

http://codereview.chromium.org/6877101/diff/1005/gpu/common/gpu_trace_event.h
File gpu/common/gpu_trace_event.h (right):

http://codereview.chromium.org/6877101/diff/1005/gpu/common/gpu_trace_event.h...
gpu/common/gpu_trace_event.h:186: class TraceAnyType {
On 2011/04/21 22:15:44, nduca wrote:
> TraceVar or TraceVariadic?
> 
> Very similar to PP_Var (chrome/ppapi)

Done.

http://codereview.chromium.org/6877101/diff/1005/gpu/common/gpu_trace_event.h...
gpu/common/gpu_trace_event.h:190: BOOLEAN,
On 2011/04/21 22:15:44, nduca wrote:
> /me wonders if we usually prefix with TYPE_? pp_var and base/values.h might be
> good reference points. And, sorry if you've already normalized to a
convention..
> :)

Done.