FYI, the changes for SConstruct is now removed because we're no longer
doing a
conditional flag to set DEBUG_THREAD_NAME.
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'],
On 2011/01/03 07:48:34, Søren Gjesse wrote:
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.
I didn't know how strict we want to be about not incurring any cost that
may not yield any benefits for the various platforms. I agree that the
cost here is minimal, and I'm fine with removing the conditional nature
of it. I'll make the appropriate change for making this unconditional.
Thanks.
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';
On 2011/01/03 07:48:34, Søren Gjesse wrote:
Please add spaces on both sides of '-'. In all the other platform
files as well.
Done.
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.
On 2011/01/03 07:48:34, Søren Gjesse wrote:
Is this not supported on MacOS somehow?
There is a pthread_setname_np() but it is not supported before macos
10.6. I've adapted an implementation from the chromium sources here:
http://src.chromium.org/viewvc/chrome/trunk/src/base/platform_thread_mac.mm?r1=49212&r2=49211&pathrev=49212
Is this acceptable?
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 {
On 2011/01/03 07:48:34, Søren Gjesse wrote:
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
Done.
http://codereview.chromium.org/6070009/diff/33001/src/platform.h#newcode430
src/platform.h:430: void SetName(const char *name);
On 2011/01/03 07:48:34, Søren Gjesse wrote:
This setter should be called set_name, see
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Function_Names#Function_Names
Done.
http://codereview.chromium.org/6070009/diff/33001/src/platform.h#newcode436
src/platform.h:436: char name_[16];
On 2011/01/03 07:48:34, Søren Gjesse wrote:
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;
Done.
http://codereview.chromium.org/6070009/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
