LGTM

http://codereview.chromium.org/3311028/diff/1/2
File include/v8-profiler.h (right):

http://codereview.chromium.org/3311028/diff/1/2#newcode368
include/v8-profiler.h:368: class V8EXPORT OutputStream {
Maybe we should consider moving OutputStream to v8.h at the top level of
the v8 namespace, as it might be useful for other APIs not related to
profiling.

http://codereview.chromium.org/3311028/diff/1/2#newcode374
include/v8-profiler.h:374: virtual void WriteAsciiChunk(char* buffer,
int chars_written) = 0;
chars_written -> chars?

http://codereview.chromium.org/3311028/diff/1/2#newcode377
include/v8-profiler.h:377: void SerializeToJSON(OutputStream* stream,
int chunk_size) const;
How about just calling this method Serialize, and have an e Format enum
which contains kFormatJSON, which could be extended with other formats.

Also how about moving the selection of chunk_size to the OutputStream
(e.g. returned by a method GetChunkSize() which could have a default
implementation).

http://codereview.chromium.org/3311028/diff/1/4
File src/profile-generator.cc (right):

http://codereview.chromium.org/3311028/diff/1/4#newcode2190
src/profile-generator.cc:2190: void MaybeWriteChunk() {
Maybe have an assert here that chunk_pos_ <= chunk_size_.

http://codereview.chromium.org/3311028/show

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to