http://codereview.chromium.org/6070009/diff/33001/SConstruct
File SConstruct (right):

http://codereview.chromium.org/6070009/diff/33001/SConstruct#newcode129
SConstruct:129: 'CPPDEFINES':   ['DEBUG_THREAD_NAMES'],
Why do you want to enable/disable this using conditional compilation? It
does not seem to add any overhead except for the 16 char name for each
thread. If there are other penalties then I suggest that a flag
--threadnames is added to skip the OS call to actually set the name for
the thread.

http://codereview.chromium.org/6070009/diff/33001/src/platform-freebsd.cc
File src/platform-freebsd.cc (right):

http://codereview.chromium.org/6070009/diff/33001/src/platform-freebsd.cc#newcode446
src/platform-freebsd.cc:446: name_[sizeof(name_)-1] = '\0';
Please add spaces on both sides of '-'. In all the other platform files
as well.

http://codereview.chromium.org/6070009/diff/33001/src/platform-macos.cc
File src/platform-macos.cc (right):

http://codereview.chromium.org/6070009/diff/33001/src/platform-macos.cc#newcode435
src/platform-macos.cc:435: // one) so we initialize it here too.
Is this not supported on MacOS somehow?

http://codereview.chromium.org/6070009/diff/33001/src/platform.h
File src/platform.h (right):

http://codereview.chromium.org/6070009/diff/33001/src/platform.h#newcode402
src/platform.h:402: inline const char* Name() const {
Please use all lower-case for this type of accessor, see
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Function_Names#Function_Names

http://codereview.chromium.org/6070009/diff/33001/src/platform.h#newcode430
src/platform.h:430: void SetName(const char *name);
This setter should be called set_name, see
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Function_Names#Function_Names

http://codereview.chromium.org/6070009/diff/33001/src/platform.h#newcode436
src/platform.h:436: char name_[16];
I think that this constant deserves a comment. E.g. "The thread name
length limit on Linux is 16.". Actually you should also make a constant
for it

static const int kMaxThreadNamelength = 16;

http://codereview.chromium.org/6070009/

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

Reply via email to